Skip to content

Commit

Permalink
feat: add idempotent refresh token algorithm (supabase#1278)
Browse files Browse the repository at this point in the history
Modifies the refresh token algorithm to support a limited form of
idempotency. The lack of this behavior is documented to cause loss of
session.

**Problem**

GoTrue, so far, assumes that clients calling the `POST
/token?grant_type=refresh_token` endpoint are guaranteed to at least
save the result of the response. Like all networked software, there are
no guarantees that the sender of a request will receive the response, or
act on it. This problem is exacerbated by network appliances like CDNs
and reverse proxies which mask the TCP stream semantics from GoTrue. A
properly closed TCP stream does not mean that the receiver of the
response received the stream, but rather that a proxy in the chain
buffered the response.

Furthermore, even if the receiver is able to receive _and parse_ the
response, usually there are no guarantees that it will continue
processing the response. With refresh tokens, it's incredbily important
that the receiver successfully persists the new refresh token to durable
storage. There are no guarantees of this as browsers and mobile apps
(and the computers they run on) can die, go offline or just malfunction
between sending a request and processing its response.

**Solution**

There are really only two solutions to this problem:

1. Idempotency. Where for the same inputs the same output is generated.
2. Double-commit. Where the receipt of the response needs to be
acknowledged before the state changes.

We considered a double-commit protocol, but decided against it at this
time as it introduces quite a bit of complexity. We may turn to it if
the limited idempotency solution does not cover a sufficient number of
the cases in real-world testing.

**Changes**

The refresh token algorithm is changed to offer a limited form of
idempotency, such that:

1. An **active refresh token** is the last non-revoked refresh token in
a session.
This is the token that should have been saved by the client. It can
generally be only used once to generate a new active refresh token at
which point it looses its status.
2. A non-active refresh token can sometimes be used again to issue a
valid token:
2.1. If the non-active token is being _reused_ close to the time it was
used again.
2.2. **NEW** If the non-active token is the parent of the currently
active token.
This case adds limited idempotency by always returning the active token,
and does not create a new active refresh token.
  • Loading branch information
hf authored Oct 23, 2023
1 parent 206fc09 commit b0426c6
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 26 deletions.
68 changes: 43 additions & 25 deletions internal/api/token_refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
var newTokenResponse *AccessTokenResponse

err = db.Transaction(func(tx *storage.Connection) error {
user, token, _, terr := models.FindUserWithRefreshToken(tx, params.RefreshToken, true /* forUpdate */)
user, token, session, terr := models.FindUserWithRefreshToken(tx, params.RefreshToken, true /* forUpdate */)
if terr != nil {
if models.IsNotFoundError(terr) {
// because forUpdate was set, and the
Expand All @@ -106,42 +106,60 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
// refresh token row and session are locked at this
// point, cannot be concurrently refreshed

if token.Revoked {
a.clearCookieTokens(config, w)
// For a revoked refresh token to be reused, it
// has to fall within the reuse interval.

reuseUntil := token.UpdatedAt.Add(
time.Second * time.Duration(config.Security.RefreshTokenReuseInterval))
var issuedToken *models.RefreshToken

if time.Now().After(reuseUntil) {
// not OK to reuse this token
if token.Revoked {
activeRefreshToken, terr := session.FindCurrentlyActiveRefreshToken(tx)
if terr != nil {
return internalServerError(terr.Error())
}

if config.Security.RefreshTokenRotationEnabled {
// Revoke all tokens in token family
if err := models.RevokeTokenFamily(tx, token); err != nil {
return internalServerError(err.Error())
if activeRefreshToken != nil && activeRefreshToken.Parent.String() == token.Token {
// Token was revoked, but it's the
// parent of the currently active one.
// This indicates that the client was
// not able to store the result when it
// refreshed token. This case is
// allowed, provided we return back the
// active refresh token instead of
// creating a new one.
issuedToken = activeRefreshToken
} else {
// For a revoked refresh token to be reused, it
// has to fall within the reuse interval.
reuseUntil := token.UpdatedAt.Add(
time.Second * time.Duration(config.Security.RefreshTokenReuseInterval))

if time.Now().After(reuseUntil) {
a.clearCookieTokens(config, w)
// not OK to reuse this token

if config.Security.RefreshTokenRotationEnabled {
// Revoke all tokens in token family
if err := models.RevokeTokenFamily(tx, token); err != nil {
return internalServerError(err.Error())
}
}
}

return oauthError("invalid_grant", "Invalid Refresh Token: Already Used").WithInternalMessage("Possible abuse attempt: %v", token.ID)
return oauthError("invalid_grant", "Invalid Refresh Token: Already Used").WithInternalMessage("Possible abuse attempt: %v", token.ID)
}
}
}

if terr = models.NewAuditLogEntry(r, tx, user, models.TokenRefreshedAction, "", nil); terr != nil {
return terr
}

// a new refresh token is generated and explicitly not reusing
// a previous one as it could have already been revoked while
// this handler was running
newToken, terr := models.GrantRefreshTokenSwap(r, tx, user, token)
if terr != nil {
return terr
}
if issuedToken == nil {
newToken, terr := models.GrantRefreshTokenSwap(r, tx, user, token)
if terr != nil {
return terr
}

tokenString, expiresAt, terr = generateAccessToken(tx, user, newToken.SessionId, &config.JWT)
issuedToken = newToken
}

tokenString, expiresAt, terr = generateAccessToken(tx, user, issuedToken.SessionId, &config.JWT)
if terr != nil {
return internalServerError("error generating jwt token").WithInternalError(terr)
}
Expand All @@ -151,7 +169,7 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
TokenType: "bearer",
ExpiresIn: config.JWT.Exp,
ExpiresAt: expiresAt,
RefreshToken: newToken.Token,
RefreshToken: issuedToken.Token,
User: user,
}
if terr = a.setCookieTokens(config, newTokenResponse, false, w); terr != nil {
Expand Down
49 changes: 48 additions & 1 deletion internal/api/token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,53 @@ func (ts *TokenTestSuite) SetupTest() {
require.NoError(ts.T(), err, "Error creating refresh token")
}

func (ts *TokenTestSuite) TestFailedToSaveRefreshTokenResultCase() {
var buffer bytes.Buffer

// first refresh
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"refresh_token": ts.RefreshToken.Token,
}))

req := httptest.NewRequest(http.MethodPost, "http://localhost/token?grant_type=refresh_token", &buffer)
req.Header.Set("Content-Type", "application/json")

w := httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
assert.Equal(ts.T(), http.StatusOK, w.Code)

var firstResult struct {
RefreshToken string `json:"refresh_token"`
}

assert.NoError(ts.T(), json.NewDecoder(w.Result().Body).Decode(&firstResult))
assert.NotEmpty(ts.T(), firstResult.RefreshToken)

// pretend that the browser wasn't able to save the firstResult,
// run again with the first refresh token
buffer = bytes.Buffer{}

// second refresh with the reused refresh token
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"refresh_token": ts.RefreshToken.Token,
}))

w = httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
assert.Equal(ts.T(), http.StatusOK, w.Code)

var secondResult struct {
RefreshToken string `json:"refresh_token"`
}

assert.NoError(ts.T(), json.NewDecoder(w.Result().Body).Decode(&secondResult))
assert.NotEmpty(ts.T(), secondResult.RefreshToken)

// new refresh token is not being issued but the active one from
// the first refresh that failed to save is stored
assert.Equal(ts.T(), firstResult.RefreshToken, secondResult.RefreshToken)
}

func (ts *TokenTestSuite) TestRateLimitTokenRefresh() {
var buffer bytes.Buffer
req := httptest.NewRequest(http.MethodPost, "http://localhost/token", &buffer)
Expand Down Expand Up @@ -248,7 +295,7 @@ func (ts *TokenTestSuite) TestTokenRefreshTokenRotation() {
desc: "Invalid refresh, revoke third token",
refreshTokenRotationEnabled: true,
reuseInterval: 0,
refreshToken: second.Token,
refreshToken: first.Token,
expectedCode: http.StatusBadRequest,
expectedBody: map[string]interface{}{
"error": "invalid_grant",
Expand Down
13 changes: 13 additions & 0 deletions internal/models/sessions.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,16 @@ func (s *Session) GetAAL() string {
func (s *Session) IsAAL2() bool {
return s.GetAAL() == AAL2.String()
}

// FindCurrentlyActiveRefreshToken returns the currently active refresh
// token in the session. This is the last created (ordered by the serial
// primary key) non-revoked refresh token for the session.
func (s *Session) FindCurrentlyActiveRefreshToken(tx *storage.Connection) (*RefreshToken, error) {
var activeRefreshToken RefreshToken

if err := tx.Q().Where("session_id = ? and revoked is false", s.ID).Order("id desc").First(&activeRefreshToken); err != nil {
return nil, err
}

return &activeRefreshToken, nil
}
4 changes: 4 additions & 0 deletions internal/storage/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,7 @@ func (s NullString) Value() (driver.Value, error) {
}
return string(s), nil
}

func (s NullString) String() string {
return string(s)
}

0 comments on commit b0426c6

Please sign in to comment.