diff --git a/internal/api/handler_accounts.go b/internal/api/handler_accounts.go index b23802413..4f151dfb3 100644 --- a/internal/api/handler_accounts.go +++ b/internal/api/handler_accounts.go @@ -751,11 +751,14 @@ func awsAmbientCredResult(acct *config.CloudAccount) (AccountTestResult, bool) { } // testAccountCredentials handles POST /api/accounts/:id/test. +// This operation performs live outbound cloud API calls / credential probing, +// which is a write-class side effect. Gate on update:accounts (not view:accounts) +// so the handler is self-protecting if the route Auth level is ever relaxed (02-M5). func (h *Handler) testAccountCredentials(ctx context.Context, req *events.LambdaFunctionURLRequest, id string) (any, error) { if err := validateUUID(id); err != nil { return nil, err } - session, err := h.requirePermission(ctx, req, "view", "accounts") + session, err := h.requirePermission(ctx, req, "update", "accounts") if err != nil { return nil, err } diff --git a/internal/api/handler_accounts_test.go b/internal/api/handler_accounts_test.go index 3561f65b0..e3fdab2ed 100644 --- a/internal/api/handler_accounts_test.go +++ b/internal/api/handler_accounts_test.go @@ -1915,3 +1915,32 @@ func (m *mockConfigStoreAccounts) SavePurchaseExecutionTx(ctx context.Context, _ func (m *mockConfigStoreAccounts) WithTx(_ context.Context, fn func(tx pgx.Tx) error) error { return fn(nil) } + +// Regression test for 02-M5: testAccountCredentials must gate on update:accounts +// (not view:accounts) to match its write-class side effect. A caller holding only +// view:accounts must be rejected with 403. +func TestTestAccountCredentials_RequiresUpdateAccounts(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + + viewOnlySession := &Session{ + UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd", + Email: "viewonly@example.com", + } + + // The caller has a valid session and view:accounts, but NOT update:accounts. + mockAuth.On("ValidateSession", ctx, "view-token").Return(viewOnlySession, nil) + mockAuth.On("HasPermissionAPI", ctx, viewOnlySession.UserID, "update", "accounts").Return(false, nil) + + handler := &Handler{auth: mockAuth, config: setupAdminMock(ctx)} + + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer view-token"}, + } + result, err := handler.testAccountCredentials(ctx, req, "11111111-1111-1111-1111-111111111111") + require.Error(t, err, "view-only caller must be rejected") + assert.Nil(t, result) + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 403, ce.code, "expected 403, got %d", ce.code) +} diff --git a/internal/api/handler_config.go b/internal/api/handler_config.go index d589f235b..db591f235 100644 --- a/internal/api/handler_config.go +++ b/internal/api/handler_config.go @@ -24,6 +24,13 @@ func sourceCloud() string { // Configuration handlers func (h *Handler) getConfig(ctx context.Context, req *events.LambdaFunctionURLRequest) (*ConfigResponse, error) { + // Require view:config permission. Every other read handler in the package + // pairs the route-level AuthUser gate with this explicit permission check; + // config GET must be consistent (02-M4). + if _, err := h.requirePermission(ctx, req, "view", "config"); err != nil { + return nil, err + } + globalCfg, err := h.config.GetGlobalConfig(ctx) if err != nil { return nil, err @@ -179,7 +186,12 @@ func mergeServiceConfig(ctx context.Context, store config.StoreInterface, cfg co return cfg, nil } -func (h *Handler) getServiceConfig(ctx context.Context, service string) (any, error) { +func (h *Handler) getServiceConfig(ctx context.Context, req *events.LambdaFunctionURLRequest, service string) (any, error) { + // Require view:config permission. Consistent with getConfig (02-M4). + if _, err := h.requirePermission(ctx, req, "view", "config"); err != nil { + return nil, err + } + // Validate for path traversal attacks if err := validateServicePath(service); err != nil { return nil, err diff --git a/internal/api/handler_config_test.go b/internal/api/handler_config_test.go index 132e01a41..b47a791a7 100644 --- a/internal/api/handler_config_test.go +++ b/internal/api/handler_config_test.go @@ -117,6 +117,12 @@ func TestHandler_updateConfig_InvalidBody(t *testing.T) { func TestHandler_getServiceConfig(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + + adminSession := &Session{ + UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + Email: "admin@example.com", + } serviceCfg := &config.ServiceConfig{ Provider: "aws", @@ -126,11 +132,16 @@ func TestHandler_getServiceConfig(t *testing.T) { Coverage: 80, } + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdmin() mockStore.On("GetServiceConfig", ctx, "aws", "rds").Return(serviceCfg, nil) - handler := &Handler{config: mockStore} + handler := &Handler{config: mockStore, auth: mockAuth} - result, err := handler.getServiceConfig(ctx, "aws/rds") + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } + result, err := handler.getServiceConfig(ctx, req, "aws/rds") require.NoError(t, err) cfg := result.(*config.ServiceConfig) @@ -140,9 +151,17 @@ func TestHandler_getServiceConfig(t *testing.T) { func TestHandler_getServiceConfig_InvalidFormat(t *testing.T) { ctx := context.Background() - handler := &Handler{corsAllowedOrigin: "*"} + mockAuth := new(MockAuthService) + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdmin() + + handler := &Handler{corsAllowedOrigin: "*", auth: mockAuth} - result, err := handler.getServiceConfig(ctx, "invalid-format") + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } + result, err := handler.getServiceConfig(ctx, req, "invalid-format") assert.Error(t, err) assert.Nil(t, result) assert.Contains(t, err.Error(), "invalid service path") @@ -151,12 +170,19 @@ func TestHandler_getServiceConfig_InvalidFormat(t *testing.T) { func TestHandler_getServiceConfig_NotFound(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdmin() mockStore.On("GetServiceConfig", ctx, "aws", "unknown").Return(nil, nil) - handler := &Handler{config: mockStore} + handler := &Handler{config: mockStore, auth: mockAuth} - result, err := handler.getServiceConfig(ctx, "aws/unknown") + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } + result, err := handler.getServiceConfig(ctx, req, "aws/unknown") require.NoError(t, err) // Returns empty response for not found @@ -333,12 +359,18 @@ func TestHandler_updateServiceConfig_NoSlash(t *testing.T) { func TestHandler_getConfig_GlobalConfigError(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdmin() mockStore.On("GetGlobalConfig", ctx).Return(nil, assert.AnError) - handler := &Handler{config: mockStore} + handler := &Handler{config: mockStore, auth: mockAuth} - req := &events.LambdaFunctionURLRequest{} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } result, err := handler.getConfig(ctx, req) assert.Error(t, err) assert.Nil(t, result) @@ -347,6 +379,11 @@ func TestHandler_getConfig_GlobalConfigError(t *testing.T) { func TestHandler_getConfig_ListServiceConfigsError(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdmin() globalCfg := &config.GlobalConfig{ EnabledProviders: []string{"aws"}, @@ -355,9 +392,11 @@ func TestHandler_getConfig_ListServiceConfigsError(t *testing.T) { mockStore.On("GetGlobalConfig", ctx).Return(globalCfg, nil) mockStore.On("ListServiceConfigs", ctx).Return(nil, assert.AnError) - handler := &Handler{config: mockStore} + handler := &Handler{config: mockStore, auth: mockAuth} - req := &events.LambdaFunctionURLRequest{} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } result, err := handler.getConfig(ctx, req) assert.Error(t, err) assert.Nil(t, result) @@ -396,7 +435,12 @@ func TestHandler_getConfig_SourceIdentity_AdminOnly(t *testing.T) { }) t.Run("regression #407: non-admin does not receive SourceIdentity", func(t *testing.T) { - mockAuth.On("HasPermissionAPI", ctx, "regular-user", mock.Anything, mock.Anything).Return(false, nil) + // The regular user has view:config permission (passes requirePermission) + // but does not hold admin:* (requireAdmin fails), so SourceIdentity is + // withheld. We match the specific permission verbs so the admin:* call + // (action="admin", resource="*") still returns false. + mockAuth.On("HasPermissionAPI", ctx, "regular-user", "view", "config").Return(true, nil) + mockAuth.On("HasPermissionAPI", ctx, "regular-user", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(false, nil).Maybe() req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{"Authorization": "Bearer user-token"}, } @@ -410,12 +454,19 @@ func TestHandler_getConfig_SourceIdentity_AdminOnly(t *testing.T) { func TestHandler_getServiceConfig_Error(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdmin() mockStore.On("GetServiceConfig", ctx, "aws", "rds").Return(nil, assert.AnError) - handler := &Handler{config: mockStore} + handler := &Handler{config: mockStore, auth: mockAuth} - result, err := handler.getServiceConfig(ctx, "aws/rds") + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } + result, err := handler.getServiceConfig(ctx, req, "aws/rds") assert.Error(t, err) assert.Nil(t, result) } @@ -521,9 +572,17 @@ func TestHandler_updateServiceConfig_SaveError(t *testing.T) { func TestHandler_getServiceConfig_InvalidProvider(t *testing.T) { ctx := context.Background() - handler := &Handler{} + mockAuth := new(MockAuthService) + adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"} + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockAuth.grantAdmin() + + handler := &Handler{auth: mockAuth} - result, err := handler.getServiceConfig(ctx, "invalid/rds") + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } + result, err := handler.getServiceConfig(ctx, req, "invalid/rds") assert.Error(t, err) assert.Nil(t, result) assert.Contains(t, err.Error(), "invalid provider") @@ -651,3 +710,61 @@ func TestHandler_updateConfig_PropagationListError(t *testing.T) { require.NoError(t, err) assert.Equal(t, "updated", result.Status) } + +// Regression tests for 02-M4: GET /api/config and GET /api/config/service/* +// must enforce requirePermission("view","config") and return 403 to callers +// who lack that permission, even though the route is only AuthUser-gated. + +func TestHandler_getConfig_ViewConfigPermission_Enforced(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + + userSession := &Session{ + UserID: "cccccccc-cccc-cccc-cccc-cccccccccccc", + Email: "viewer@example.com", + } + // User has a valid session but view:config is revoked. + mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil) + mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "view", "config").Return(false, nil) + + handler := &Handler{config: mockStore, auth: mockAuth} + + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer user-token"}, + } + result, err := handler.getConfig(ctx, req) + require.Error(t, err, "must be rejected when view:config is revoked") + assert.Nil(t, result) + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 403, ce.code, "expected 403, got %d", ce.code) + mockStore.AssertNotCalled(t, "GetGlobalConfig", mock.Anything) +} + +func TestHandler_getServiceConfig_ViewConfigPermission_Enforced(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + + userSession := &Session{ + UserID: "cccccccc-cccc-cccc-cccc-cccccccccccc", + Email: "viewer@example.com", + } + // User has a valid session but view:config is revoked. + mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil) + mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "view", "config").Return(false, nil) + + handler := &Handler{config: mockStore, auth: mockAuth} + + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer user-token"}, + } + result, err := handler.getServiceConfig(ctx, req, "aws/rds") + require.Error(t, err, "must be rejected when view:config is revoked") + assert.Nil(t, result) + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 403, ce.code, "expected 403, got %d", ce.code) + mockStore.AssertNotCalled(t, "GetServiceConfig", mock.Anything, mock.Anything, mock.Anything) +} diff --git a/internal/api/router.go b/internal/api/router.go index 9e3adfcf3..f0bcb7c1e 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -437,7 +437,7 @@ func (r *Router) updateConfigHandler(ctx context.Context, req *events.LambdaFunc } func (r *Router) getServiceConfigHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { - return r.h.getServiceConfig(ctx, params["id"]) + return r.h.getServiceConfig(ctx, req, params["id"]) } func (r *Router) updateServiceConfigHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) {