diff --git a/handler/openid/flow_explicit_token.go b/handler/openid/flow_explicit_token.go index 0b416d2c2..5988913e6 100644 --- a/handler/openid/flow_explicit_token.go +++ b/handler/openid/flow_explicit_token.go @@ -22,7 +22,9 @@ func (c *OpenIDConnectExplicitHandler) PopulateTokenEndpointResponse(ctx context return errorsx.WithStack(fosite.ErrUnknownRequest) } - authorize, err := c.OpenIDConnectRequestStorage.GetOpenIDConnectSession(ctx, requester.GetRequestForm().Get("code"), requester) + authorizeCode := requester.GetRequestForm().Get("code") + + authorize, err := c.OpenIDConnectRequestStorage.GetOpenIDConnectSession(ctx, authorizeCode, requester) if errors.Is(err, ErrNoSessionFound) { return errorsx.WithStack(fosite.ErrUnknownRequest.WithWrap(err).WithDebug(err.Error())) } else if err != nil { @@ -47,6 +49,11 @@ func (c *OpenIDConnectExplicitHandler) PopulateTokenEndpointResponse(ctx context return errorsx.WithStack(fosite.ErrServerError.WithDebug("Failed to generate id token because subject is an empty string.")) } + err = c.OpenIDConnectRequestStorage.DeleteOpenIDConnectSession(ctx, authorizeCode) + if err != nil { + return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error())) + } + claims.AccessTokenHash = c.GetAccessTokenHash(ctx, requester, responder) // The response type `id_token` is only required when performing the implicit or hybrid flow, see: diff --git a/handler/openid/flow_explicit_token_test.go b/handler/openid/flow_explicit_token_test.go index 56325e5d9..8c6b571ab 100644 --- a/handler/openid/flow_explicit_token_test.go +++ b/handler/openid/flow_explicit_token_test.go @@ -102,6 +102,7 @@ func TestExplicit_PopulateTokenEndpointResponse(t *testing.T) { storedReq.GrantedScope = fosite.Arguments{"openid"} storedReq.Form.Set("nonce", "1111111111111111") store.EXPECT().GetOpenIDConnectSession(gomock.Any(), "foobar", req).Return(storedReq, nil) + store.EXPECT().DeleteOpenIDConnectSession(gomock.Any(), "foobar").Return(nil) }, check: func(t *testing.T, aresp *fosite.AccessResponse) { assert.NotEmpty(t, aresp.GetExtra("id_token")) @@ -132,6 +133,7 @@ func TestExplicit_PopulateTokenEndpointResponse(t *testing.T) { storedReq.GrantedScope = fosite.Arguments{"openid"} storedReq.Form.Set("nonce", "1111111111111111") store.EXPECT().GetOpenIDConnectSession(gomock.Any(), "foobar", req).Return(storedReq, nil) + store.EXPECT().DeleteOpenIDConnectSession(gomock.Any(), "foobar").Return(nil) }, check: func(t *testing.T, aresp *fosite.AccessResponse) { assert.NotEmpty(t, aresp.GetExtra("id_token")) @@ -173,6 +175,25 @@ func TestExplicit_PopulateTokenEndpointResponse(t *testing.T) { }, expectErr: fosite.ErrServerError, }, + { + description: "should fail because storage returns error when deleting openid session", + setup: func(store *internal.MockOpenIDConnectRequestStorage, req *fosite.AccessRequest) { + req.Client = &fosite.DefaultClient{ + GrantTypes: fosite.Arguments{"authorization_code"}, + } + req.GrantTypes = fosite.Arguments{"authorization_code"} + req.Form.Set("code", "foobar") + storedSession := &DefaultSession{ + Claims: &jwt.IDTokenClaims{Subject: "peter"}, + } + storedReq := fosite.NewAuthorizeRequest() + storedReq.Session = storedSession + storedReq.GrantedScope = fosite.Arguments{"openid"} + store.EXPECT().GetOpenIDConnectSession(gomock.Any(), "foobar", req).Return(storedReq, nil) + store.EXPECT().DeleteOpenIDConnectSession(gomock.Any(), "foobar").Return(errors.New("delete openid session err")) + }, + expectErr: fosite.ErrServerError, + }, } { t.Run(fmt.Sprintf("case=%d/description=%s", k, c.description), func(t *testing.T) { ctrl := gomock.NewController(t) diff --git a/handler/openid/storage.go b/handler/openid/storage.go index d29f73717..2b9dae9ba 100644 --- a/handler/openid/storage.go +++ b/handler/openid/storage.go @@ -22,7 +22,6 @@ type OpenIDConnectRequestStorage interface { // - or an arbitrary error if an error occurred. GetOpenIDConnectSession(ctx context.Context, authorizeCode string, requester fosite.Requester) (fosite.Requester, error) - // Deprecated: DeleteOpenIDConnectSession is not called from anywhere. - // Originally, it should remove an open id connect session from the store. + // DeleteOpenIDConnectSession removes an open id connect session from the store. DeleteOpenIDConnectSession(ctx context.Context, authorizeCode string) error } diff --git a/storage/memory.go b/storage/memory.go index 3c6a5ad45..079b80beb 100644 --- a/storage/memory.go +++ b/storage/memory.go @@ -165,7 +165,6 @@ func (s *MemoryStore) GetOpenIDConnectSession(_ context.Context, authorizeCode s return cl, nil } -// DeleteOpenIDConnectSession is not really called from anywhere and it is deprecated. func (s *MemoryStore) DeleteOpenIDConnectSession(_ context.Context, authorizeCode string) error { s.idSessionsMutex.Lock() defer s.idSessionsMutex.Unlock()