fix: do not validate request when creating response

canonical · Mar 28, 2024 · 5c0e1dd · 5c0e1dd
1 parent f5008d6
commit 5c0e1dd
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 65 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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{