diff --git a/handler/openid/flow_explicit_token.go b/handler/openid/flow_explicit_token.go index d952da94..be0f30c9 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(oauth2.ErrUnknownRequest) } - authorize, err := c.OpenIDConnectRequestStorage.GetOpenIDConnectSession(ctx, requester.GetRequestForm().Get(consts.FormParameterAuthorizationCode), requester) + authorizeCode := requester.GetRequestForm().Get(consts.FormParameterAuthorizationCode) + + authorize, err := c.OpenIDConnectRequestStorage.GetOpenIDConnectSession(ctx, authorizeCode, requester) if errors.Is(err, ErrNoSessionFound) { return errorsx.WithStack(oauth2.ErrUnknownRequest.WithWrap(err).WithDebugError(err)) } else if err != nil { @@ -42,6 +44,10 @@ func (c *OpenIDConnectExplicitHandler) PopulateTokenEndpointResponse(ctx context return errorsx.WithStack(oauth2.ErrServerError.WithDebug("Failed to generate id token because session must be of type oauth2/handler/openid.Session.")) } + if err = c.OpenIDConnectRequestStorage.DeleteOpenIDConnectSession(ctx, authorizeCode); err != nil { + return errorsx.WithStack(oauth2.ErrServerError.WithWrap(err).WithDebugError(err)) + } + claims := sess.IDTokenClaims() if claims.Subject == "" { return errorsx.WithStack(oauth2.ErrServerError.WithDebug("Failed to generate id token because subject is an empty string.")) diff --git a/handler/openid/flow_explicit_token_test.go b/handler/openid/flow_explicit_token_test.go index 5edbde08..c5f8c14e 100644 --- a/handler/openid/flow_explicit_token_test.go +++ b/handler/openid/flow_explicit_token_test.go @@ -103,6 +103,7 @@ func TestExplicit_PopulateTokenEndpointResponse(t *testing.T) { storedReq.GrantedScope = oauth2.Arguments{consts.ScopeOpenID} storedReq.Form.Set(consts.FormParameterNonce, "1111111111111111") store.EXPECT().GetOpenIDConnectSession(context.TODO(), "foobar", req).Return(storedReq, nil) + store.EXPECT().DeleteOpenIDConnectSession(gomock.Any(), "foobar").Return(nil) }, check: func(t *testing.T, aresp *oauth2.AccessResponse) { assert.NotEmpty(t, aresp.GetExtra(consts.AccessResponseIDToken)) @@ -133,6 +134,7 @@ func TestExplicit_PopulateTokenEndpointResponse(t *testing.T) { storedReq.GrantedScope = oauth2.Arguments{consts.ScopeOpenID} storedReq.Form.Set("nonce", "1111111111111111") store.EXPECT().GetOpenIDConnectSession(context.TODO(), "foobar", req).Return(storedReq, nil) + store.EXPECT().DeleteOpenIDConnectSession(gomock.Any(), "foobar").Return(nil) }, check: func(t *testing.T, aresp *oauth2.AccessResponse) { assert.NotEmpty(t, aresp.GetExtra("id_token")) @@ -159,6 +161,7 @@ func TestExplicit_PopulateTokenEndpointResponse(t *testing.T) { storedReq.Session = storedSession storedReq.GrantedScope = oauth2.Arguments{consts.ScopeOpenID} store.EXPECT().GetOpenIDConnectSession(context.TODO(), "foobar", req).Return(storedReq, nil) + store.EXPECT().DeleteOpenIDConnectSession(gomock.Any(), "foobar").Return(nil) }, expectErr: oauth2.ErrServerError, }, @@ -174,6 +177,25 @@ func TestExplicit_PopulateTokenEndpointResponse(t *testing.T) { }, expectErr: oauth2.ErrServerError, }, + { + description: "should fail because storage returns error when deleting openid session", + setup: func(store *internal.MockOpenIDConnectRequestStorage, req *oauth2.AccessRequest) { + req.Client = &oauth2.DefaultClient{ + GrantTypes: oauth2.Arguments{"authorization_code"}, + } + req.GrantTypes = oauth2.Arguments{"authorization_code"} + req.Form.Set("code", "foobar") + storedSession := &DefaultSession{ + Claims: &jwt.IDTokenClaims{Subject: "peter"}, + } + storedReq := oauth2.NewAuthorizeRequest() + storedReq.Session = storedSession + storedReq.GrantedScope = oauth2.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: oauth2.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 1c1119b9..36dab24f 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 oauth2.Requester) (oauth2.Requester, error) - // Deprecated: DeleteOpenIDConnectSession is not called from anywhere. - // Originally, it should remove an open id connect session from the store. + // DeleteOpenIDConnectSession deletes the OpenID Connect 1.0 session from storage. DeleteOpenIDConnectSession(ctx context.Context, authorizeCode string) error }