Skip to content

Commit

Permalink
refactor: refresh token rotation interfaces
Browse files Browse the repository at this point in the history
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:

```patch
// 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)
}
```
  • Loading branch information
aeneasr committed Dec 3, 2024
1 parent e285999 commit 4cc28f4
Show file tree
Hide file tree
Showing 37 changed files with 129 additions and 316 deletions.
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -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 .
Expand All @@ -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
Expand All @@ -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
2 changes: 1 addition & 1 deletion handler/oauth2/flow_authorize_code_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
}
Expand Down
4 changes: 2 additions & 2 deletions handler/oauth2/flow_authorize_code_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion handler/oauth2/flow_client_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 6 additions & 12 deletions handler/oauth2/flow_refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ func (c *RefreshTokenGrantHandler) HandleTokenEndpointRequest(ctx context.Contex
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
Expand Down Expand Up @@ -134,25 +133,18 @@ func (c *RefreshTokenGrantHandler) PopulateTokenEndpointResponse(ctx context.Con
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 {
if err = c.TokenRevocationStorage.RotateRefreshToken(ctx, requester.GetID(), signature); err != nil {
return err
}

storeReq := requester.Sanitize([]string{})
storeReq.SetID(ts.GetID())

if err = c.TokenRevocationStorage.CreateAccessTokenSession(ctx, accessSignature, storeReq); err != nil {
return err
}

if err = c.TokenRevocationStorage.CreateRefreshTokenSession(ctx, refreshSignature, storeReq); err != nil {
if err = c.TokenRevocationStorage.CreateRefreshTokenSession(ctx, refreshSignature, accessSignature, storeReq); err != nil {
return err
}

Expand Down Expand Up @@ -221,12 +213,14 @@ func (c *RefreshTokenGrantHandler) handleRefreshTokenEndpointStorageError(ctx co
if errors.Is(storageErr, fosite.ErrSerializationFailure) {
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."))
}

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."))
}

Expand Down
Loading

0 comments on commit 4cc28f4

Please sign in to comment.