From 5c0e1ddc0018955545336df4a4d0f875b82d970e Mon Sep 17 00:00:00 2001 From: Nikos Date: Thu, 28 Mar 2024 18:19:40 +0200 Subject: [PATCH] fix: do not validate request when creating response --- .../oauth2/flow_authorize_code_token_test.go | 22 --------------- handler/oauth2/flow_generic_code_token.go | 8 ++---- handler/rfc8628/token_handler.go | 18 ++++++------ handler/rfc8628/token_handler_test.go | 28 ------------------- 4 files changed, 11 insertions(+), 65 deletions(-) diff --git a/handler/oauth2/flow_authorize_code_token_test.go b/handler/oauth2/flow_authorize_code_token_test.go index aac54dfe..e7349627 100644 --- a/handler/oauth2/flow_authorize_code_token_test.go +++ b/handler/oauth2/flow_authorize_code_token_test.go @@ -64,28 +64,6 @@ func TestAuthorizeCode_PopulateTokenEndpointResponse(t *testing.T) { }, expectErr: fosite.ErrServerError, }, - { - description: "should fail because authorization code is expired", - areq: &fosite.AccessRequest{ - GrantTypes: fosite.Arguments{"authorization_code"}, - Request: fosite.Request{ - Form: url.Values{"code": []string{"foo.bar"}}, - Client: &fosite.DefaultClient{ - GrantTypes: fosite.Arguments{"authorization_code"}, - }, - Session: &fosite.DefaultSession{ - ExpiresAt: map[fosite.TokenType]time.Time{ - fosite.AuthorizeCode: time.Now().Add(-time.Hour).UTC(), - }, - }, - RequestedAt: time.Now().Add(-2 * time.Hour).UTC(), - }, - }, - setup: func(t *testing.T, areq *fosite.AccessRequest, config *fosite.Config) { - require.NoError(t, store.CreateAuthorizeCodeSession(context.Background(), "bar", areq)) - }, - expectErr: fosite.ErrInvalidRequest, - }, { description: "should pass with offline scope and refresh token", areq: &fosite.AccessRequest{ diff --git a/handler/oauth2/flow_generic_code_token.go b/handler/oauth2/flow_generic_code_token.go index 756507c2..c7781eac 100644 --- a/handler/oauth2/flow_generic_code_token.go +++ b/handler/oauth2/flow_generic_code_token.go @@ -72,9 +72,9 @@ func (c *GenericCodeTokenEndpointHandler) PopulateTokenEndpointResponse(ctx cont return errorsx.WithStack(fosite.ErrUnknownRequest) } - var code, signature string + var signature string var err error - if code, signature, err = c.Code(ctx, requester); err != nil { + if _, signature, err = c.Code(ctx, requester); err != nil { return err } @@ -83,10 +83,6 @@ func (c *GenericCodeTokenEndpointHandler) PopulateTokenEndpointResponse(ctx cont return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error())) } - if err = c.ValidateCode(ctx, requester, code); err != nil { - return errorsx.WithStack(fosite.ErrInvalidRequest.WithWrap(err).WithDebug(err.Error())) - } - for _, scope := range ar.GetRequestedScopes() { requester.GrantScope(scope) } diff --git a/handler/rfc8628/token_handler.go b/handler/rfc8628/token_handler.go index a8d0f8bb..c21c07a3 100644 --- a/handler/rfc8628/token_handler.go +++ b/handler/rfc8628/token_handler.go @@ -22,15 +22,6 @@ type DeviceCodeHandler struct { func (c DeviceCodeHandler) Code(ctx context.Context, requester fosite.AccessRequester) (code string, signature string, err error) { code = requester.GetRequestForm().Get("device_code") - shouldRateLimit, err := c.DeviceRateLimitStrategy.ShouldRateLimit(ctx, code) - // TODO(nsklikas) : should we error out or just silently log it? - if err != nil { - return "", "", err - } - if shouldRateLimit { - return "", "", errorsx.WithStack(fosite.ErrPollingRateLimited) - } - signature, err = c.DeviceCodeStrategy.DeviceCodeSignature(ctx, code) if err != nil { return "", "", errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error())) @@ -40,6 +31,15 @@ func (c DeviceCodeHandler) Code(ctx context.Context, requester fosite.AccessRequ } func (c DeviceCodeHandler) ValidateCode(ctx context.Context, requester fosite.Requester, code string) error { + shouldRateLimit, err := c.DeviceRateLimitStrategy.ShouldRateLimit(ctx, code) + // TODO(nsklikas) : should we error out or just silently log it? + if err != nil { + return err + } + if shouldRateLimit { + return errorsx.WithStack(fosite.ErrPollingRateLimited) + } + return c.DeviceCodeStrategy.ValidateDeviceCode(ctx, requester, code) } diff --git a/handler/rfc8628/token_handler_test.go b/handler/rfc8628/token_handler_test.go index 01d55d8b..6e29d187 100644 --- a/handler/rfc8628/token_handler_test.go +++ b/handler/rfc8628/token_handler_test.go @@ -376,33 +376,6 @@ func TestDeviceUserCode_PopulateTokenEndpointResponse(t *testing.T) { }, expectErr: fosite.ErrServerError, }, - { - description: "should fail because device code is expired", - areq: &fosite.AccessRequest{ - GrantTypes: fosite.Arguments{string(fosite.GrantTypeDeviceCode)}, - Request: fosite.Request{ - Form: url.Values{}, - Client: &fosite.DefaultClient{ - GrantTypes: fosite.Arguments{string(fosite.GrantTypeDeviceCode)}, - }, - Session: &DefaultDeviceFlowSession{ - ExpiresAt: map[fosite.TokenType]time.Time{ - fosite.DeviceCode: time.Now().Add(-time.Hour).UTC(), - }, - BrowserFlowCompleted: true, - }, - RequestedAt: time.Now().Add(-2 * time.Hour).UTC(), - }, - }, - setup: func(t *testing.T, areq *fosite.AccessRequest, config *fosite.Config) { - code, signature, err := strategy.GenerateDeviceCode(context.TODO()) - require.NoError(t, err) - areq.Form.Add("device_code", code) - - require.NoError(t, store.CreateDeviceCodeSession(context.Background(), signature, areq)) - }, - expectErr: fosite.ErrInvalidRequest, - }, { description: "should pass with offline scope and refresh token", areq: &fosite.AccessRequest{ @@ -767,7 +740,6 @@ func TestDeviceUserCodeTransactional_HandleTokenEndpointRequest(t *testing.T) { mockCoreStore = internal.NewMockCoreStorage(ctrl) mockDeviceCodeStore = internal.NewMockDeviceCodeStorage(ctrl) mockDeviceRateLimitStrategy = internal.NewMockDeviceRateLimitStrategy(ctrl) - mockDeviceRateLimitStrategy.EXPECT().ShouldRateLimit(gomock.Any(), gomock.Any()).Return(false, nil).Times(1) testCase.setup() handler := oauth2.GenericCodeTokenEndpointHandler{