From 271cda8722da644beaa0439ef45914e9fe3bef67 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 07:40:24 +0000 Subject: [PATCH 1/3] Initial plan From e19b2bfebf8889774c2d195509d5b56239e7cf8c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 07:53:11 +0000 Subject: [PATCH 2/3] Fix: Default error scenarios to enabled when configured The 'enabled' field now defaults to true when configuring error scenarios. This resolves the UX issue where external tests couldn't trigger error scenarios without explicitly setting "enabled": true. Changes: - Changed 'enabled' field to *bool (pointer) to distinguish nil from false - Default 'enabled' to true when error scenario is configured (nil) - Allow explicit "enabled": false to disable scenarios - Updated README documentation to clarify the enabled field behavior - Added comprehensive test coverage for default enabled behavior - Listed all supported OAuth2 error codes in documentation Co-authored-by: chrisw-dev <68233262+chrisw-dev@users.noreply.github.com> --- README.md | 29 ++++++++- internal/handlers/config.go | 16 +++-- internal/handlers/config_test.go | 101 ++++++++++++++++++++++++++++--- 3 files changed, 132 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index eedf7a8..29d62a4 100644 --- a/README.md +++ b/README.md @@ -376,11 +376,14 @@ Allows test code to dynamically configure the responses returned by the OAuth2 e "error_scenario": { "endpoint": "token", "error": "invalid_grant", - "error_description": "Custom error for testing" + "error_description": "Custom error for testing", + "enabled": true } } ``` +**Note**: The `enabled` field is optional and defaults to `true` when `endpoint` and `error` are provided. To explicitly disable an error scenario, set `"enabled": false`. + **Response**: ```json @@ -392,6 +395,30 @@ Allows test code to dynamically configure the responses returned by the OAuth2 e **IMPORTANT**: To update user information that will be returned by the `/userinfo` endpoint, you must include the user profile data inside the `tokens.user_info` object, not in the top-level `user_info` field. The top-level `user_info` field updates a different user object that is not used by the `/userinfo` endpoint. +**Error Scenario Configuration**: + +The `error_scenario` object supports the following OAuth2 error codes for testing: + +**Authorize endpoint errors:** +- `access_denied` - User denied access +- `unauthorized_client` - Client not authorized for this grant type +- `invalid_scope` - Requested scope is invalid or unknown +- `temporarily_unavailable` - Server is temporarily unavailable +- `invalid_request` - Request is missing a required parameter or malformed +- `unsupported_response_type` - Response type is not supported +- `server_error` - Internal server error occurred + +**Token endpoint errors:** +- `invalid_grant` - Invalid authorization code or credentials +- `invalid_client` - Client authentication failed +- `unsupported_grant_type` - Grant type is not supported +- `invalid_request` - Request is malformed + +**Userinfo endpoint errors:** +- `invalid_token` - Access token is invalid or expired +- `insufficient_scope` - Token lacks required scope +- `server_error` - Internal server error occurred + This enables testing scenarios like: - Testing how your application handles different user profiles diff --git a/internal/handlers/config.go b/internal/handlers/config.go index 88dc82b..59fb74c 100644 --- a/internal/handlers/config.go +++ b/internal/handlers/config.go @@ -34,9 +34,9 @@ type ConfigRequest struct { // ErrorScenario defines an error condition to simulate type ErrorScenario struct { - Enabled bool `json:"enabled"` // Whether the error scenario is enabled - Endpoint string `json:"endpoint"` // Which endpoint should return an error (authorize, token, userinfo) - Error string `json:"error"` // OAuth2 error code + Enabled *bool `json:"enabled,omitempty"` // Whether the error scenario is enabled (defaults to true if not specified) + Endpoint string `json:"endpoint"` // Which endpoint should return an error (authorize, token, userinfo) + Error string `json:"error"` // OAuth2 error code ErrorDescription string `json:"error_description,omitempty"` } @@ -109,9 +109,17 @@ func (h *ConfigHandler) storeTokenConfig(tokenConfig map[string]interface{}) { // storeErrorScenario saves error scenario configuration to the store func (h *ConfigHandler) storeErrorScenario(scenario ErrorScenario) { + // Default enabled to true when an error scenario is being configured + // If Enabled is nil (not provided), default to true + // If Enabled is explicitly set (true or false), use that value + enabled := true + if scenario.Enabled != nil { + enabled = *scenario.Enabled + } + // Convert from handlers.ErrorScenario to types.ErrorScenario storeScenario := types.ErrorScenario{ - Enabled: scenario.Enabled, + Enabled: enabled, Endpoint: scenario.Endpoint, StatusCode: determineStatusCode(scenario.Error), ErrorCode: scenario.Error, diff --git a/internal/handlers/config_test.go b/internal/handlers/config_test.go index d94979e..ee5c89f 100644 --- a/internal/handlers/config_test.go +++ b/internal/handlers/config_test.go @@ -28,6 +28,11 @@ func newMockStore() *mockStore { } } +// Helper function to create a bool pointer +func boolPtr(b bool) *bool { + return &b +} + func (s *mockStore) StoreAuthCode(code string, request *models.AuthRequest) { s.authCodes[code] = request } @@ -255,7 +260,7 @@ func TestConfigHandler_UpdateErrorScenario(t *testing.T) { { name: "invalid_request", errorScenario: ErrorScenario{ - Enabled: true, + Enabled: boolPtr(true), Endpoint: "token", Error: "invalid_request", ErrorDescription: "Test invalid request", @@ -265,7 +270,7 @@ func TestConfigHandler_UpdateErrorScenario(t *testing.T) { { name: "invalid_client", errorScenario: ErrorScenario{ - Enabled: true, + Enabled: boolPtr(true), Endpoint: "token", Error: "invalid_client", ErrorDescription: "Test invalid client", @@ -275,7 +280,7 @@ func TestConfigHandler_UpdateErrorScenario(t *testing.T) { { name: "server_error", errorScenario: ErrorScenario{ - Enabled: true, + Enabled: boolPtr(true), Endpoint: "userinfo", Error: "server_error", ErrorDescription: "Test server error", @@ -285,7 +290,7 @@ func TestConfigHandler_UpdateErrorScenario(t *testing.T) { { name: "unknown_error", errorScenario: ErrorScenario{ - Enabled: true, + Enabled: boolPtr(true), Endpoint: "authorize", Error: "unknown_error", ErrorDescription: "Test unknown error", @@ -295,7 +300,7 @@ func TestConfigHandler_UpdateErrorScenario(t *testing.T) { { name: "unsupported_response_type", errorScenario: ErrorScenario{ - Enabled: true, + Enabled: boolPtr(true), Endpoint: "authorize", Error: "unsupported_response_type", ErrorDescription: "Response type not supported", @@ -305,7 +310,7 @@ func TestConfigHandler_UpdateErrorScenario(t *testing.T) { { name: "temporarily_unavailable", errorScenario: ErrorScenario{ - Enabled: true, + Enabled: boolPtr(true), Endpoint: "authorize", Error: "temporarily_unavailable", ErrorDescription: "Server is under maintenance", @@ -340,8 +345,9 @@ func TestConfigHandler_UpdateErrorScenario(t *testing.T) { } // Check the fields were stored correctly - if scenario.Enabled != tc.errorScenario.Enabled { - t.Errorf("wrong enabled status stored: got %v want %v", scenario.Enabled, tc.errorScenario.Enabled) + if scenario.Enabled != (tc.errorScenario.Enabled != nil && *tc.errorScenario.Enabled) { + expectedEnabled := tc.errorScenario.Enabled != nil && *tc.errorScenario.Enabled + t.Errorf("wrong enabled status stored: got %v want %v", scenario.Enabled, expectedEnabled) } if scenario.Endpoint != tc.errorScenario.Endpoint { t.Errorf("wrong endpoint stored: got %v want %v", scenario.Endpoint, tc.errorScenario.Endpoint) @@ -376,7 +382,7 @@ func TestConfigHandler_CombinedUpdate(t *testing.T) { "expires_in": 2400, }, ErrorScenario: &ErrorScenario{ - Enabled: true, + Enabled: boolPtr(true), Endpoint: "token", Error: "invalid_grant", ErrorDescription: "Combined test error", @@ -476,3 +482,80 @@ func TestDetermineStatusCode(t *testing.T) { }) } } + +func TestConfigHandler_ErrorScenarioDefaultEnabled(t *testing.T) { + // Setup + mockStore := newMockStore() + defaultUser := models.NewDefaultUser() + handler := NewConfigHandler(mockStore, defaultUser) + + // Test case where enabled field is not set (defaults to false in Go) + // but should be enabled when endpoint and error are provided + configReq := ConfigRequest{ + ErrorScenario: &ErrorScenario{ + // Enabled field not set, defaults to false + Endpoint: "authorize", + Error: "unauthorized_client", + ErrorDescription: "Client not authorized", + }, + } + reqBody, _ := json.Marshal(configReq) + req := httptest.NewRequest("POST", "/config", bytes.NewBuffer(reqBody)) + rr := httptest.NewRecorder() + + // Call handler + handler.ServeHTTP(rr, req) + + // Check response code + if status := rr.Code; status != http.StatusOK { + t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK) + } + + // Verify error scenario was stored and ENABLED by default + scenario, exists := mockStore.GetErrorScenario("authorize") + if !exists { + t.Errorf("error scenario not found for authorize endpoint") + return + } + + if !scenario.Enabled { + t.Errorf("error scenario should be enabled by default when endpoint and error are provided, got enabled=%v", scenario.Enabled) + } + if scenario.ErrorCode != "unauthorized_client" { + t.Errorf("wrong error code stored: got %v want %v", scenario.ErrorCode, "unauthorized_client") + } +} + +func TestConfigHandler_ErrorScenarioExplicitlyDisabled(t *testing.T) { + // Setup + mockStore := newMockStore() + defaultUser := models.NewDefaultUser() + handler := NewConfigHandler(mockStore, defaultUser) + + // Test case where enabled is explicitly set to false + configReq := ConfigRequest{ + ErrorScenario: &ErrorScenario{ + Enabled: boolPtr(false), // Explicitly disabled + Endpoint: "authorize", + Error: "access_denied", + ErrorDescription: "Should not be active", + }, + } + reqBody, _ := json.Marshal(configReq) + req := httptest.NewRequest("POST", "/config", bytes.NewBuffer(reqBody)) + rr := httptest.NewRecorder() + + // Call handler + handler.ServeHTTP(rr, req) + + // Check response code + if status := rr.Code; status != http.StatusOK { + t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK) + } + + // Verify error scenario is stored but NOT enabled + scenario, exists := mockStore.GetErrorScenario("authorize") + if exists { + t.Errorf("error scenario should not be returned when disabled, but got: %+v", scenario) + } +} From 617ce928a51aadcfba3d644713c0d340342ba9d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 07:55:00 +0000 Subject: [PATCH 3/3] Refactor: Address code review feedback - Extract duplicated boolean expression to variable for readability - Update misleading comment about default nil behavior Co-authored-by: chrisw-dev <68233262+chrisw-dev@users.noreply.github.com> --- internal/handlers/config_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/handlers/config_test.go b/internal/handlers/config_test.go index ee5c89f..6a37971 100644 --- a/internal/handlers/config_test.go +++ b/internal/handlers/config_test.go @@ -345,8 +345,8 @@ func TestConfigHandler_UpdateErrorScenario(t *testing.T) { } // Check the fields were stored correctly - if scenario.Enabled != (tc.errorScenario.Enabled != nil && *tc.errorScenario.Enabled) { - expectedEnabled := tc.errorScenario.Enabled != nil && *tc.errorScenario.Enabled + expectedEnabled := tc.errorScenario.Enabled != nil && *tc.errorScenario.Enabled + if scenario.Enabled != expectedEnabled { t.Errorf("wrong enabled status stored: got %v want %v", scenario.Enabled, expectedEnabled) } if scenario.Endpoint != tc.errorScenario.Endpoint { @@ -489,11 +489,11 @@ func TestConfigHandler_ErrorScenarioDefaultEnabled(t *testing.T) { defaultUser := models.NewDefaultUser() handler := NewConfigHandler(mockStore, defaultUser) - // Test case where enabled field is not set (defaults to false in Go) - // but should be enabled when endpoint and error are provided + // Test case where enabled field is not set (nil pointer) + // Should default to true when endpoint and error are provided configReq := ConfigRequest{ ErrorScenario: &ErrorScenario{ - // Enabled field not set, defaults to false + // Enabled field not set (nil), will default to true Endpoint: "authorize", Error: "unauthorized_client", ErrorDescription: "Client not authorized",