fix: refresh token error wrappers return InactiveToken

Fixes an issue where the incorrect error code was returned when rotating refresh tokens.
ory · Dec 4, 2024 · 9a18e70 · 9a18e70
1 parent c4fe21c
commit 9a18e70
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 12 deletions.
diff --git a/handler/oauth2/flow_refresh.go b/handler/oauth2/flow_refresh.go
@@ -53,7 +53,7 @@ func (c *RefreshTokenGrantHandler) HandleTokenEndpointRequest(ctx context.Contex
 
 		return errorsx.WithStack(fosite.ErrInactiveToken.WithWrap(err).WithDebug(err.Error()))
 	} else if errors.Is(err, fosite.ErrNotFound) {
-		return errorsx.WithStack(fosite.ErrInvalidGrant.WithWrap(err).WithDebugf("The refresh token has not been found: %s", err.Error()))
+		return errorsx.WithStack(fosite.ErrInactiveToken.WithWrap(err).WithDebug("The refresh token can not be found."))
 	} else if err != nil {
 		return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error()))
 	} else if err := c.RefreshTokenStrategy.ValidateRefreshToken(ctx, originalRequest, refresh); err != nil {
@@ -129,23 +129,20 @@ func (c *RefreshTokenGrantHandler) PopulateTokenEndpointResponse(ctx context.Con
 	if err != nil {
 		return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error()))
 	}
-	defer func() {
-		err = c.handleRefreshTokenEndpointStorageError(ctx, err)
-	}()
 
 	storeReq := requester.Sanitize([]string{})
 	storeReq.SetID(requester.GetID())
 
 	if err = c.TokenRevocationStorage.RotateRefreshToken(ctx, requester.GetID(), signature); err != nil {
-		return err
+		return c.handleRefreshTokenEndpointStorageError(ctx, err)
 	}
 
 	if err = c.TokenRevocationStorage.CreateAccessTokenSession(ctx, accessSignature, storeReq); err != nil {
-		return err
+		return c.handleRefreshTokenEndpointStorageError(ctx, err)
 	}
 
 	if err = c.TokenRevocationStorage.CreateRefreshTokenSession(ctx, refreshSignature, accessSignature, storeReq); err != nil {
-		return err
+		return c.handleRefreshTokenEndpointStorageError(ctx, err)
 	}
 
 	responder.SetAccessToken(accessToken)
@@ -156,7 +153,7 @@ func (c *RefreshTokenGrantHandler) PopulateTokenEndpointResponse(ctx context.Con
 	responder.SetExtra("refresh_token", refreshToken)
 
 	if err = storage.MaybeCommitTx(ctx, c.TokenRevocationStorage); err != nil {
-		return err
+		return c.handleRefreshTokenEndpointStorageError(ctx, err)
 	}
 
 	return nil
@@ -214,14 +211,14 @@ func (c *RefreshTokenGrantHandler) handleRefreshTokenEndpointStorageError(ctx co
 		return errorsx.WithStack(fosite.ErrInvalidRequest.
 			WithDebugf(storageErr.Error()).
 			WithWrap(storageErr).
-			WithHint("Failed to refresh token because of multiple concurrent requests using the same token which is not allowed."))
+			WithHint("Failed to refresh token because of multiple concurrent requests using the same token. Please retry the request."))
 	}
 
 	if errors.Is(storageErr, fosite.ErrNotFound) || errors.Is(storageErr, fosite.ErrInactiveToken) {
 		return errorsx.WithStack(fosite.ErrInvalidRequest.
 			WithDebugf(storageErr.Error()).
 			WithWrap(storageErr).
-			WithHint("Failed to refresh token because of multiple concurrent requests using the same token which is not allowed."))
+			WithHint("Failed to refresh token. Please retry the request."))
 	}
 
 	return errorsx.WithStack(fosite.ErrServerError.WithWrap(storageErr).WithDebug(storageErr.Error()))

diff --git a/handler/oauth2/flow_refresh_test.go b/handler/oauth2/flow_refresh_test.go
@@ -58,7 +58,7 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {
 
 						areq.Form.Add("refresh_token", "some.refreshtokensig")
 					},
-					expectErr: fosite.ErrInvalidGrant,
+					expectErr: fosite.ErrInactiveToken,
 				},
 				{
 					description: "should fail because token is valid but does not exist",
@@ -70,7 +70,7 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {
 						require.NoError(t, err)
 						areq.Form.Add("refresh_token", token)
 					},
-					expectErr: fosite.ErrInvalidGrant,
+					expectErr: fosite.ErrInactiveToken,
 				},
 				{
 					description: "should fail because client mismatches",