refactor: refresh token rotation (#838)

Previously, the refresh token handler was using a combination of delete/update storage primitives. This made optimizing and implementing the refresh token handling difficult. Going forward, the RefreshTokenStorage must implement `RotateRefreshToken`. Token creation continues to be separated.

BREAKING CHANGES:

Method `RevokeRefreshTokenMaybeGracePeriod` was removed from `handler/fosite/TokenRevocationStorage`.

Interface `handler/fosite/RefreshTokenStorage` has changed:

- `CreateRefreshToken` now takes an additional argument `accessSignature` to keep track of refresh/access token pairs:
- A new method `RotateRefreshToken` was added, which revokes old refresh tokens and associated access tokens:

```diff
// handler/fosite/storage.go
type RefreshTokenStorage interface {
-	CreateRefreshTokenSession(ctx context.Context, signature string, request fosite.Requester) (err error)
+	CreateRefreshTokenSession(ctx context.Context, signature string, accessSignature string, request fosite.Requester) (err error)

	GetRefreshTokenSession(ctx context.Context, signature string, session fosite.Session) (request fosite.Requester, err error)
	DeleteRefreshTokenSession(ctx context.Context, signature string) (err error)

+	RotateRefreshToken(ctx context.Context, requestID string, refreshTokenSignature string) (err error)
}
```

Author: aeneasr

.github/workflows/oidc-conformity.yml

@@ -14,7 +14,7 @@ jobs:
         with:
           fetch-depth: 2
           repository: ory/hydra
-          ref: 2866a0499d02341ed0603601cfe4e63b24506fcb
+          ref: a35e78e364a26c4f87f37d9f545ef10b3ffa468a
       - uses: actions/setup-go@v2
         with:
           go-version: "1.21"

Makefile

@@ -1,3 +1,5 @@
+export PATH 					:= .bin:${PATH}
+
 format: .bin/goimports .bin/ory node_modules  # formats the source code
 	.bin/ory dev headers copyright --type=open-source
 	.bin/goimports -w .
@@ -18,6 +20,9 @@ test:  # runs all tests
 .bin/licenses: Makefile
 	curl https://raw.githubusercontent.com/ory/ci/master/licenses/install | sh
 
+.bin/mockgen:
+	go build -o .bin/mockgen github.com/golang/mock/mockgen
+
 .bin/ory: Makefile
 	curl https://raw.githubusercontent.com/ory/meta/master/install.sh | bash -s -- -b .bin ory v0.1.48
 	touch .bin/ory
@@ -26,4 +31,7 @@ node_modules: package-lock.json
 	npm ci
 	touch node_modules
 
+gen: .bin/goimports .bin/mockgen  # generates mocks
+	./generate-mocks.sh
+
 .DEFAULT_GOAL := help

handler/oauth2/flow_authorize_code_token.go

@@ -169,7 +169,7 @@ func (c *AuthorizeExplicitGrantHandler) PopulateTokenEndpointResponse(ctx contex
 	} else if err = c.CoreStorage.CreateAccessTokenSession(ctx, accessSignature, requester.Sanitize([]string{})); err != nil {
 		return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error()))
 	} else if refreshSignature != "" {
-		if err = c.CoreStorage.CreateRefreshTokenSession(ctx, refreshSignature, requester.Sanitize([]string{})); err != nil {
+		if err = c.CoreStorage.CreateRefreshTokenSession(ctx, refreshSignature, accessSignature, requester.Sanitize([]string{})); err != nil {
 			return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error()))
 		}
 	}

handler/oauth2/flow_authorize_code_token_test.go

@@ -498,7 +498,7 @@ func TestAuthorizeCodeTransactional_HandleTokenEndpointRequest(t *testing.T) {
 					Times(1)
 				mockCoreStore.
 					EXPECT().
-					CreateRefreshTokenSession(propagatedContext, gomock.Any(), gomock.Any()).
+					CreateRefreshTokenSession(propagatedContext, gomock.Any(), gomock.Any(), gomock.Any()).
 					Return(nil).
 					Times(1)
 				mockTransactional.
@@ -627,7 +627,7 @@ func TestAuthorizeCodeTransactional_HandleTokenEndpointRequest(t *testing.T) {
 					Times(1)
 				mockCoreStore.
 					EXPECT().
-					CreateRefreshTokenSession(propagatedContext, gomock.Any(), gomock.Any()).
+					CreateRefreshTokenSession(propagatedContext, gomock.Any(), gomock.Any(), gomock.Any()).
 					Return(nil).
 					Times(1)
 				mockTransactional.

handler/oauth2/flow_client_credentials.go

@@ -64,7 +64,8 @@ func (c *ClientCredentialsGrantHandler) PopulateTokenEndpointResponse(ctx contex
 	}
 
 	atLifespan := fosite.GetEffectiveLifespan(request.GetClient(), fosite.GrantTypeClientCredentials, fosite.AccessToken, c.Config.GetAccessTokenLifespan(ctx))
-	return c.IssueAccessToken(ctx, atLifespan, request, response)
+	_, err := c.IssueAccessToken(ctx, atLifespan, request, response)
+	return err
 }
 
 func (c *ClientCredentialsGrantHandler) CanSkipClientAuth(ctx context.Context, requester fosite.AccessRequester) bool {

handler/oauth2/flow_refresh.go

@@ -51,25 +51,31 @@ func (c *RefreshTokenGrantHandler) HandleTokenEndpointRequest(ctx context.Contex
 			return errorsx.WithStack(rErr)
 		}
 
-		return errorsx.WithStack(fosite.ErrInactiveToken.WithWrap(err).WithDebug(err.Error()))
+		return fosite.ErrInvalidGrant.WithWrap(err).
+			WithHint("The refresh token was already used.").
+			WithDebugf("Refresh token re-use was detected. All related tokens have been revoked.")
 	} 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 fosite.ErrInvalidGrant.WithWrap(err).
+			WithHint("The refresh token is malformed or not valid.").
+			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 {
+		return fosite.ErrServerError.WithWrap(err).WithDebug(err.Error())
+	}
+
+	if err := c.RefreshTokenStrategy.ValidateRefreshToken(ctx, originalRequest, refresh); err != nil {
 		// The authorization server MUST ... validate the refresh token.
 		// This needs to happen after store retrieval for the session to be hydrated properly
 		if errors.Is(err, fosite.ErrTokenExpired) {
-			return errorsx.WithStack(fosite.ErrInvalidGrant.WithWrap(err).WithDebug(err.Error()))
+			return fosite.ErrInvalidGrant.WithWrap(err).
+				WithHint("The refresh token expired.")
 		}
-		return errorsx.WithStack(fosite.ErrInvalidRequest.WithWrap(err).WithDebug(err.Error()))
+		return fosite.ErrInvalidRequest.WithWrap(err).WithDebug(err.Error())
 	}
 
 	if !(len(c.Config.GetRefreshTokenScopes(ctx)) == 0 || originalRequest.GetGrantedScopes().HasOneOf(c.Config.GetRefreshTokenScopes(ctx)...)) {
 		scopeNames := strings.Join(c.Config.GetRefreshTokenScopes(ctx), " or ")
 		hint := fmt.Sprintf("The OAuth 2.0 Client was not granted scope %s and may thus not perform the 'refresh_token' authorization grant.", scopeNames)
 		return errorsx.WithStack(fosite.ErrScopeNotGranted.WithHint(hint))
-
 	}
 
 	// The authorization server MUST ... and ensure that the refresh token was issued to the authenticated client
@@ -130,30 +136,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)
-	}()
 
-	ts, err := c.TokenRevocationStorage.GetRefreshTokenSession(ctx, signature, nil)
-	if err != nil {
-		return err
-	} else if err := c.TokenRevocationStorage.RevokeAccessToken(ctx, ts.GetID()); err != nil {
-		return err
-	}
+	storeReq := requester.Sanitize([]string{})
+	storeReq.SetID(requester.GetID())
 
-	if err := c.TokenRevocationStorage.RevokeRefreshTokenMaybeGracePeriod(ctx, ts.GetID(), signature); err != nil {
-		return err
+	if err = c.TokenRevocationStorage.RotateRefreshToken(ctx, requester.GetID(), signature); err != nil {
+		return c.handleRefreshTokenEndpointStorageError(ctx, err)
 	}
 
-	storeReq := requester.Sanitize([]string{})
-	storeReq.SetID(ts.GetID())
-
 	if err = c.TokenRevocationStorage.CreateAccessTokenSession(ctx, accessSignature, storeReq); err != nil {
-		return err
+		return c.handleRefreshTokenEndpointStorageError(ctx, err)
 	}
 
-	if err = c.TokenRevocationStorage.CreateRefreshTokenSession(ctx, refreshSignature, storeReq); err != nil {
-		return err
+	if err = c.TokenRevocationStorage.CreateRefreshTokenSession(ctx, refreshSignature, accessSignature, storeReq); err != nil {
+		return c.handleRefreshTokenEndpointStorageError(ctx, err)
 	}
 
 	responder.SetAccessToken(accessToken)
@@ -164,7 +160,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
@@ -222,14 +218,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()))