Skip to content

Commit

Permalink
fix: add error codes to refresh token flow (#1824)
Browse files Browse the repository at this point in the history 
## What kind of change does this PR introduce?
* Updates the refresh token flow to use error codes
* The `oauthError` originally returns a bad request error (400) so we're
keeping the status codes the same in this PR.
* Once this is merged, we can update the client lib to start picking up
on these new error codes and removing the session locally.

## What is the current behavior?

Please link any relevant issues here.

## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
  • Loading branch information
@kangmingtay
kangmingtay authored Nov 1, 2024
1 parent a7129df commit 4614dc5
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 28 deletions.
5 changes: 4 additions & 1 deletion internal/api/errorcodes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ const (
ErrorCodeNoAuthorization ErrorCode = "no_authorization"
ErrorCodeUserNotFound ErrorCode = "user_not_found"
ErrorCodeSessionNotFound ErrorCode = "session_not_found"
ErrorCodeSessionExpired ErrorCode = "session_expired"
ErrorCodeRefreshTokenNotFound ErrorCode = "refresh_token_not_found"
ErrorCodeRefreshTokenAlreadyUsed ErrorCode = "refresh_token_already_used"
ErrorCodeFlowStateNotFound ErrorCode = "flow_state_not_found"
ErrorCodeFlowStateExpired ErrorCode = "flow_state_expired"
ErrorCodeSignupDisabled ErrorCode = "signup_disabled"
Expand Down Expand Up @@ -71,7 +74,7 @@ const (
ErrorCodeOverRequestRateLimit ErrorCode = "over_request_rate_limit"
ErrorCodeOverEmailSendRateLimit ErrorCode = "over_email_send_rate_limit"
ErrorCodeOverSMSSendRateLimit ErrorCode = "over_sms_send_rate_limit"
ErrorBadCodeVerifier ErrorCode = "bad_code_verifier"
ErrorCodeBadCodeVerifier ErrorCode = "bad_code_verifier"
ErrorCodeAnonymousProviderDisabled ErrorCode = "anonymous_provider_disabled"
ErrorCodeHookTimeout ErrorCode = "hook_timeout"
ErrorCodeHookTimeoutAfterRetry ErrorCode = "hook_timeout_after_retry"
Expand Down
2 changes: 1 addition & 1 deletion internal/api/token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ func (a *API) PKCE(ctx context.Context, w http.ResponseWriter, r *http.Request)
return err
}
if err := flowState.VerifyPKCE(params.CodeVerifier); err != nil {
return badRequestError(ErrorBadCodeVerifier, err.Error())
return badRequestError(ErrorCodeBadCodeVerifier, err.Error())
}

var token *AccessTokenResponse
Expand Down
12 changes: 6 additions & 6 deletions internal/api/token_refresh.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
user, token, session, err := models.FindUserWithRefreshToken(db, params.RefreshToken, false)
if err != nil {
if models.IsNotFoundError(err) {
return oauthError("invalid_grant", "Invalid Refresh Token: Refresh Token Not Found")
return badRequestError(ErrorCodeRefreshTokenNotFound, "Invalid Refresh Token: Refresh Token Not Found")
}
return internalServerError(err.Error())
}

if user.IsBanned() {
return oauthError("invalid_grant", "Invalid Refresh Token: User Banned")
return badRequestError(ErrorCodeUserBanned, "Invalid Refresh Token: User Banned")
}

if session == nil {
Expand All @@ -71,10 +71,10 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
// do nothing

case models.SessionTimedOut:
return oauthError("invalid_grant", "Invalid Refresh Token: Session Expired (Inactivity)")
return badRequestError(ErrorCodeSessionExpired, "Invalid Refresh Token: Session Expired (Inactivity)")

default:
return oauthError("invalid_grant", "Invalid Refresh Token: Session Expired")
return badRequestError(ErrorCodeSessionExpired, "Invalid Refresh Token: Session Expired")
}

// Basic checks above passed, now we need to serialize access
Expand Down Expand Up @@ -153,7 +153,7 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
if s.LastRefreshedAt(nil).After(session.LastRefreshedAt(&token.UpdatedAt)) {
// session is not the most
// recently active one
return oauthError("invalid_grant", "Invalid Refresh Token: Session Expired (Revoked by Newer Login)")
return badRequestError(ErrorCodeSessionExpired, "Invalid Refresh Token: Session Expired (Revoked by Newer Login)")
}
}

Expand Down Expand Up @@ -196,7 +196,7 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
}
}

return storage.NewCommitWithError(oauthError("invalid_grant", "Invalid Refresh Token: Already Used").WithInternalMessage("Possible abuse attempt: %v", token.ID))
return storage.NewCommitWithError(badRequestError(ErrorCodeRefreshTokenAlreadyUsed, "Invalid Refresh Token: Already Used").WithInternalMessage("Possible abuse attempt: %v", token.ID))
}
}
}
Expand Down
40 changes: 20 additions & 20 deletions internal/api/token_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,13 @@ func (ts *TokenTestSuite) TestSessionTimebox() {
assert.Equal(ts.T(), http.StatusBadRequest, w.Code)

var firstResult struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description"`
ErrorCode string `json:"error_code"`
Message string `json:"msg"`
}

assert.NoError(ts.T(), json.NewDecoder(w.Result().Body).Decode(&firstResult))
assert.Equal(ts.T(), "invalid_grant", firstResult.Error)
assert.Equal(ts.T(), "Invalid Refresh Token: Session Expired", firstResult.ErrorDescription)
assert.Equal(ts.T(), ErrorCodeSessionExpired, firstResult.ErrorCode)
assert.Equal(ts.T(), "Invalid Refresh Token: Session Expired", firstResult.Message)
}

func (ts *TokenTestSuite) TestSessionInactivityTimeout() {
Expand Down Expand Up @@ -124,13 +124,13 @@ func (ts *TokenTestSuite) TestSessionInactivityTimeout() {
assert.Equal(ts.T(), http.StatusBadRequest, w.Code)

var firstResult struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description"`
ErrorCode string `json:"error_code"`
Message string `json:"msg"`
}

assert.NoError(ts.T(), json.NewDecoder(w.Result().Body).Decode(&firstResult))
assert.Equal(ts.T(), "invalid_grant", firstResult.Error)
assert.Equal(ts.T(), "Invalid Refresh Token: Session Expired (Inactivity)", firstResult.ErrorDescription)
assert.Equal(ts.T(), ErrorCodeSessionExpired, firstResult.ErrorCode)
assert.Equal(ts.T(), "Invalid Refresh Token: Session Expired (Inactivity)", firstResult.Message)
}

func (ts *TokenTestSuite) TestFailedToSaveRefreshTokenResultCase() {
Expand Down Expand Up @@ -213,13 +213,13 @@ func (ts *TokenTestSuite) TestSingleSessionPerUserNoTags() {
assert.True(ts.T(), ts.API.config.Sessions.SinglePerUser)

var firstResult struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description"`
ErrorCode string `json:"error_code"`
Message string `json:"msg"`
}

assert.NoError(ts.T(), json.NewDecoder(w.Result().Body).Decode(&firstResult))
assert.Equal(ts.T(), "invalid_grant", firstResult.Error)
assert.Equal(ts.T(), "Invalid Refresh Token: Session Expired (Revoked by Newer Login)", firstResult.ErrorDescription)
assert.Equal(ts.T(), ErrorCodeSessionExpired, firstResult.ErrorCode)
assert.Equal(ts.T(), "Invalid Refresh Token: Session Expired (Revoked by Newer Login)", firstResult.Message)
}

func (ts *TokenTestSuite) TestRateLimitTokenRefresh() {
Expand Down Expand Up @@ -428,13 +428,13 @@ func (ts *TokenTestSuite) TestRefreshTokenReuseRevocation() {
assert.Equal(ts.T(), http.StatusBadRequest, w.Code)

var response struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description"`
ErrorCode string `json:"error_code"`
Message string `json:"msg"`
}

require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&response))
require.Equal(ts.T(), response.Error, "invalid_grant")
require.Equal(ts.T(), response.ErrorDescription, "Invalid Refresh Token: Already Used")
require.Equal(ts.T(), ErrorCodeRefreshTokenAlreadyUsed, response.ErrorCode)
require.Equal(ts.T(), "Invalid Refresh Token: Already Used", response.Message)

// ensure that the refresh tokens are marked as revoked in the database
for _, refreshToken := range refreshTokens {
Expand All @@ -461,13 +461,13 @@ func (ts *TokenTestSuite) TestRefreshTokenReuseRevocation() {
assert.Equal(ts.T(), http.StatusBadRequest, w.Code, "For refresh token %d", i)

var response struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description"`
ErrorCode string `json:"error_code"`
Message string `json:"msg"`
}

require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&response))
require.Equal(ts.T(), response.Error, "invalid_grant", "For refresh token %d", i)
require.Equal(ts.T(), response.ErrorDescription, "Invalid Refresh Token: Already Used", "For refresh token %d", i)
require.Equal(ts.T(), ErrorCodeRefreshTokenAlreadyUsed, response.ErrorCode, "For refresh token %d", i)
require.Equal(ts.T(), "Invalid Refresh Token: Already Used", response.Message, "For refresh token %d", i)
}
}

Expand Down

0 comments on commit 4614dc5

Please sign in to comment.