diff --git a/internal/api/handler_config.go b/internal/api/handler_config.go index 1909046f..fda9a122 100644 --- a/internal/api/handler_config.go +++ b/internal/api/handler_config.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/aws/aws-lambda-go/events" ) @@ -60,8 +61,9 @@ func (h *Handler) getConfig(ctx context.Context, req *events.LambdaFunctionURLRe func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionURLRequest) (*StatusResponse, error) { // Require update:config permission - if _, err := h.requirePermission(ctx, req, "update", "config"); err != nil { - return nil, err + session, permErr := h.requirePermission(ctx, req, "update", "config") + if permErr != nil { + return nil, permErr } // Reject a malformed body before any DB work so a bad request fails fast @@ -80,6 +82,9 @@ func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionUR return nil, NewClientError(400, "invalid request body") } _, gracePresent := present["grace_period_days"] + if err := h.requireGlobalConfigScope(ctx, session); err != nil { + return nil, err + } // Serialized read-modify-write: the store loads the stored config and // applies this closure under an advisory-locked transaction, then upserts @@ -130,6 +135,18 @@ func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionUR return &StatusResponse{Status: "updated"}, nil } +func (h *Handler) requireGlobalConfigScope(ctx context.Context, session *Session) error { + scope, err := h.getAccountScope(ctx, session) + if err != nil { + return err + } + if !scope.AllowsAll() { + return NewClientError(403, "permission denied: global configuration requires unrestricted account access") + } + return h.requirePermissionConstraints(ctx, session, auth.ActionUpdate, auth.ResourceConfig, + []auth.PermissionConstraints{{StrictScope: true}}) +} + // anyKeyPresent reports whether any of keys is present in m. func anyKeyPresent(m map[string]json.RawMessage, keys ...string) bool { for _, k := range keys { diff --git a/internal/api/handler_ladder.go b/internal/api/handler_ladder.go index aa43a37f..ac8aa7a6 100644 --- a/internal/api/handler_ladder.go +++ b/internal/api/handler_ladder.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/aws/aws-lambda-go/events" ) @@ -156,7 +157,9 @@ func (h *Handler) requireLadderAccountAccess(ctx context.Context, session *Sessi if account.Provider != cfg.Provider { return NewClientError(400, fmt.Sprintf("provider %q does not match cloud account provider %q", cfg.Provider, account.Provider)) } - return nil + return h.requirePermissionConstraints(ctx, session, auth.ActionUpdate, auth.ResourceConfig, []auth.PermissionConstraints{{ + AccountIDs: []string{cfg.CloudAccountID}, Providers: []string{account.Provider}, StrictScope: true, + }}) } // applyLadderConfigNumericDefaults fills a numeric field with its default ONLY diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index 3b493621..198b5a4b 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -2055,7 +2055,8 @@ func (h *Handler) getRIExchangeConfig(ctx context.Context, req *events.LambdaFun // updateRIExchangeConfig updates the RI exchange automation settings. func (h *Handler) updateRIExchangeConfig(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) { - if _, err := h.requirePermission(ctx, req, "update", "config"); err != nil { + session, err := h.requirePermission(ctx, req, "update", "config") + if err != nil { return nil, err } @@ -2067,6 +2068,9 @@ func (h *Handler) updateRIExchangeConfig(ctx context.Context, req *events.Lambda if err := body.validate(); err != nil { return nil, err } + if err := h.requireGlobalConfigScope(ctx, session); err != nil { + return nil, err + } // Route through the serialized read-modify-write so this partial update // (only the ri_exchange_* columns) shares the advisory lock with other diff --git a/internal/api/permission_constraints_config_integration_test.go b/internal/api/permission_constraints_config_integration_test.go new file mode 100644 index 00000000..8c92c776 --- /dev/null +++ b/internal/api/permission_constraints_config_integration_test.go @@ -0,0 +1,82 @@ +//go:build integration + +package api + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" + "github.com/aws/aws-lambda-go/events" + "github.com/google/uuid" + "github.com/stretchr/testify/require" +) + +func TestConfigInfrastructureKeyHTTP(t *testing.T) { + ctx := context.Background() + pg, err := testhelpers.SetupPostgresContainer(ctx, t) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, pg.Cleanup(ctx)) }) + require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + store := config.NewPostgresStore(pg.DB) + authService := new(MockAuthService) + handler := NewHandler(HandlerConfig{ConfigStore: store, AuthService: authService}) + // Preload the infrastructure key; this verifies authorization, not secret loading. + handler.apiKey = uuid.NewString() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, readErr := io.ReadAll(r.Body) + if readErr != nil { + http.Error(w, readErr.Error(), http.StatusBadRequest) + return + } + resp, handleErr := handler.HandleRequest(r.Context(), &events.LambdaFunctionURLRequest{ + Body: string(body), Headers: map[string]string{"x-api-key": r.Header.Get("X-API-Key"), "content-type": "application/json"}, + RequestContext: events.LambdaFunctionURLRequestContext{HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: r.Method, Path: r.URL.Path}}, + }) + if handleErr != nil { + http.Error(w, handleErr.Error(), http.StatusInternalServerError) + return + } + w.WriteHeader(resp.StatusCode) + _, _ = io.WriteString(w, resp.Body) + })) + t.Cleanup(server.Close) + account := uuid.NewString() + require.NoError(t, store.CreateCloudAccount(ctx, &config.CloudAccount{ID: account, Name: account, Provider: "aws", ExternalID: "123456789012", Enabled: true})) + initial, err := store.GetGlobalConfig(ctx) + require.NoError(t, err) + initial.LadderingEnabled, initial.RIExchangeEnabled, initial.RIExchangeMode = false, false, "manual" + require.NoError(t, store.SaveGlobalConfig(ctx, initial)) + for _, route := range []struct{ path, body string }{ + {"/api/ladder/configs", fmt.Sprintf(`{"cloud_account_id":%q,"provider":"aws","enabled":true,"mode":"email_approval","cadence":"daily","ramp_schedule":{"steps":[{"after_days":0,"fraction":1}]}}`, account)}, + {"/api/config", `{"laddering_enabled":true}`}, + {"/api/ri-exchange/config", `{"auto_exchange_enabled":true,"mode":"manual","utilization_threshold":50,"lookback_days":30}`}, + } { + req, reqErr := http.NewRequestWithContext(ctx, http.MethodPut, server.URL+route.path, strings.NewReader(route.body)) + require.NoError(t, reqErr) + req.Header.Set("X-API-Key", handler.apiKey) + resp, reqErr := server.Client().Do(req) + require.NoError(t, reqErr) + body, readErr := io.ReadAll(resp.Body) + require.NoError(t, resp.Body.Close()) + require.NoError(t, readErr) + require.Equal(t, http.StatusOK, resp.StatusCode, "%s: %s", route.path, body) + } + ladders, err := store.GetLadderConfigs(ctx) + require.NoError(t, err) + require.Len(t, ladders, 1) + require.True(t, ladders[0].Enabled) + after, err := store.GetGlobalConfig(ctx) + require.NoError(t, err) + require.True(t, after.LadderingEnabled) + require.True(t, after.RIExchangeEnabled) + require.Empty(t, authService.Calls) + require.Empty(t, authService.UsageBookings()) +} diff --git a/internal/api/ri_exchange_automode_gate_test.go b/internal/api/ri_exchange_automode_gate_test.go index e222326e..e14517a9 100644 --- a/internal/api/ri_exchange_automode_gate_test.go +++ b/internal/api/ri_exchange_automode_gate_test.go @@ -119,6 +119,13 @@ func autoGateSessionRouter(t *testing.T, perms []auth.Permission, stored config. func autoGateKeyRouter(t *testing.T, keyPerms, userPerms []auth.Permission, stored config.GlobalConfig) (*Router, *mocks.MockConfigStore) { t.Helper() mockAuth := new(MockAuthService) + sets := []auth.PermissionConstraints{{StrictScope: true}} + keyAllowed, err := auth.PermissionsAllowForConstraintSets(keyPerms, auth.ActionUpdate, auth.ResourceConfig, sets) + require.NoError(t, err) + ownerAllowed, err := auth.PermissionsAllowForConstraintSets(userPerms, auth.ActionUpdate, auth.ResourceConfig, sets) + require.NoError(t, err) + mockAuth.On("HasAPIKeyPermissionForConstraintsAPI", mock.Anything, autoGateKeyID, autoGateUserID, + auth.ActionUpdate, auth.ResourceConfig, sets).Return(keyAllowed && ownerAllowed, nil).Maybe() mockAuth.On("ValidateUserAPIKeyAPI", mock.Anything, autoGateAPIKey). Return(&auth.UserAPIKey{ID: autoGateKeyID}, &auth.User{ID: autoGateUserID}, nil).Maybe() diff --git a/internal/auth/permission_strict_scope_test.go b/internal/auth/permission_strict_scope_test.go new file mode 100644 index 00000000..dc57ffa4 --- /dev/null +++ b/internal/auth/permission_strict_scope_test.go @@ -0,0 +1,61 @@ +package auth + +import ( + "encoding/json" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPermissionStrictScope(t *testing.T) { + t.Parallel() + for _, admin := range []bool{false, true} { + permission := Permission{Action: ActionUpdate, Resource: ResourceConfig} + if admin { + permission.Action, permission.Resource = ActionAdmin, ResourceAll + } + for _, tc := range []struct { + name string + held *PermissionConstraints + req PermissionConstraints + want bool + }{ + {"unrestricted", nil, PermissionConstraints{StrictScope: true}, true}, + {"unknown-account", &PermissionConstraints{AccountIDs: []string{"a"}}, PermissionConstraints{StrictScope: true}, false}, + {"unknown-provider", &PermissionConstraints{Providers: []string{"aws"}}, PermissionConstraints{StrictScope: true}, false}, + {"unknown-service", &PermissionConstraints{Services: []string{"ec2"}}, PermissionConstraints{StrictScope: true}, false}, + {"unknown-region", &PermissionConstraints{Regions: []string{"eastus"}}, PermissionConstraints{StrictScope: true}, false}, + {"known-normalized-region", &PermissionConstraints{Regions: []string{"EastUS "}}, PermissionConstraints{StrictScope: true, Regions: []string{" eastus"}}, true}, + {"mixed-regions", &PermissionConstraints{Regions: []string{"eastus"}}, PermissionConstraints{StrictScope: true, Regions: []string{"eastus", "westus"}}, false}, + {"known-scope", &PermissionConstraints{AccountIDs: []string{"a"}, Providers: []string{"aws"}}, PermissionConstraints{StrictScope: true, AccountIDs: []string{"a"}, Providers: []string{"aws"}}, true}, + {"legacy-absent-dimension", &PermissionConstraints{AccountIDs: []string{"a"}}, PermissionConstraints{}, true}, + {"amount-unaffected", &PermissionConstraints{MaxPurchaseAmount: 1}, PermissionConstraints{StrictScope: true}, true}, + } { + t.Run(tc.name, func(t *testing.T) { + permission.Constraints = tc.held + allowed, err := PermissionsAllowForConstraintSets([]Permission{permission}, ActionUpdate, ResourceConfig, []PermissionConstraints{tc.req}) + require.NoError(t, err) + assert.Equal(t, tc.want, allowed, "admin=%t", admin) + }) + } + } + assert.False(t, permissionsAllow([]Permission{{Action: ActionAdmin, Resource: ResourceAll}}, ActionExecute, ResourceRIExchange, &PermissionConstraints{StrictScope: true})) +} + +func TestStrictScopeIsRequestOnly(t *testing.T) { + t.Parallel() + original := PermissionConstraints{StrictScope: true, Providers: []string{"aws"}} + encoded, err := json.Marshal(original) + require.NoError(t, err) + var decoded PermissionConstraints + require.NoError(t, json.Unmarshal(encoded, &decoded)) + assert.False(t, decoded.StrictScope) + require.NoError(t, json.Unmarshal([]byte(`{"StrictScope":true,"strict_scope":true}`), &decoded)) + assert.False(t, decoded.StrictScope) + field, found := reflect.TypeFor[PermissionConstraints]().FieldByName("StrictScope") + require.True(t, found) + assert.Equal(t, "-", field.Tag.Get("dynamodbav")) + assert.Equal(t, original.Providers, decoded.Providers) +} diff --git a/internal/auth/service_group.go b/internal/auth/service_group.go index 2f27227c..f8be7e9b 100644 --- a/internal/auth/service_group.go +++ b/internal/auth/service_group.go @@ -332,6 +332,9 @@ func permissionsAllow(permissions []Permission, action, resource string, constra if coversCarvedOut(Permission{Action: action, Resource: resource}) { continue } + if constraints != nil && constraints.StrictScope && !checkPermissionConstraints(perm, constraints) { + continue + } return true } @@ -391,9 +394,16 @@ func checkPermissionConstraints(perm Permission, constraints *PermissionConstrai // matchConstraints checks if permission constraints match request constraints. func matchConstraints(permConstraints, reqConstraints *PermissionConstraints) bool { - return matchStringListConstraints(permConstraints.AccountIDs, reqConstraints.AccountIDs) && - matchStringListConstraints(permConstraints.Providers, reqConstraints.Providers) && - matchStringListConstraints(permConstraints.Services, reqConstraints.Services) && + if reqConstraints.StrictScope && len(permConstraints.Regions) > 0 && len(reqConstraints.Regions) == 0 { + return false + } + matchList := matchStringListConstraints + if reqConstraints.StrictScope { + matchList = listCovers + } + return matchList(permConstraints.AccountIDs, reqConstraints.AccountIDs) && + matchList(permConstraints.Providers, reqConstraints.Providers) && + matchList(permConstraints.Services, reqConstraints.Services) && matchAllRegionsConstraint(permConstraints.Regions, reqConstraints.Regions) && matchPurchaseAmountConstraint(permConstraints.MaxPurchaseAmount, reqConstraints.MaxPurchaseAmount) } diff --git a/internal/auth/types.go b/internal/auth/types.go index 7be67b3c..f3cd809d 100644 --- a/internal/auth/types.go +++ b/internal/auth/types.go @@ -54,6 +54,9 @@ type Permission struct { // PermissionConstraints limit permissions to specific accounts, providers, or services. type PermissionConstraints struct { + // StrictScope is request-only: unknown or unbounded dimensions require an unrestricted grant. + StrictScope bool `json:"-" dynamodbav:"-"` + // AccountIDs limits to specific AWS/Azure/GCP accounts AccountIDs []string `json:"account_ids,omitempty" dynamodbav:"AccountIDs"` diff --git a/internal/server/permission_constraints_config_integration_test.go b/internal/server/permission_constraints_config_integration_test.go new file mode 100644 index 00000000..7be1d797 --- /dev/null +++ b/internal/server/permission_constraints_config_integration_test.go @@ -0,0 +1,155 @@ +//go:build integration + +package server + +import ( + "context" + "crypto/hmac" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/api" + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConfigPermissionConstraintsHTTP(t *testing.T) { + ctx := context.Background() + pg, err := testhelpers.SetupPostgresContainer(ctx, t) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, pg.Cleanup(ctx)) }) + require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + store, authStore := config.NewPostgresStore(pg.DB), auth.NewPostgresStore(pg.DB) + csrfKey := []byte(strings.Repeat("c", 32)) + authSvc := auth.NewService(auth.ServiceConfig{Store: authStore, CSRFKey: csrfKey}) + handler := api.NewHandler(api.HandlerConfig{ConfigStore: store, AuthService: newAuthServiceAdapter(authSvc)}) + app := &Application{API: handler, DB: pg.DB, dbConnected: true} + server := httptest.NewServer(CreateHTTPServer(app, 0).Handler) + t.Cleanup(server.Close) + accounts := []string{uuid.NewString(), uuid.NewString()} + for i, id := range accounts { + require.NoError(t, store.CreateCloudAccount(ctx, &config.CloudAccount{ID: id, Name: id, Provider: "aws", ExternalID: fmt.Sprintf("%012d", i+1), Enabled: true})) + } + initial, err := store.GetGlobalConfig(ctx) + require.NoError(t, err) + initial.LadderingEnabled, initial.RIExchangeEnabled, initial.RIExchangeMode = false, false, "manual" + ladderBody := fmt.Sprintf(`{"cloud_account_id":%q,"provider":"aws","enabled":true,"mode":"email_approval","cadence":"daily","ramp_schedule":{"steps":[{"after_days":0,"fraction":1}]}}`, accounts[0]) + for _, route := range []struct{ name, path, body string }{ + {"ladder", "/api/ladder/configs", ladderBody}, + {"global", "/api/config", `{"laddering_enabled":true}`}, + {"exchange", "/api/ri-exchange/config", `{"auto_exchange_enabled":true,"mode":"manual","utilization_threshold":50,"lookback_days":30}`}, + } { + for _, tc := range []struct { + name string + constraints *auth.PermissionConstraints + allowed []string + key bool + want int + }{ + {"unrestricted", nil, []string{"*"}, false, 200}, + {"account-mismatch", &auth.PermissionConstraints{AccountIDs: accounts[1:]}, []string{"*"}, false, 403}, + {"provider-mismatch", &auth.PermissionConstraints{Providers: []string{"azure"}}, []string{"*"}, false, 403}, + {"service-bounded", &auth.PermissionConstraints{Services: []string{"ec2"}}, []string{"*"}, false, 403}, + {"region-bounded", &auth.PermissionConstraints{Regions: []string{"us-east-1"}}, []string{"*"}, false, 403}, + {"all-current-accounts", &auth.PermissionConstraints{AccountIDs: accounts}, []string{"*"}, false, 403}, + {"matching-account", &auth.PermissionConstraints{AccountIDs: accounts[:1], Providers: []string{"aws"}}, accounts[:1], false, 403}, + {"independent-account-scope", nil, accounts[1:], false, 403}, + {"key-restricted", &auth.PermissionConstraints{Providers: []string{"azure"}}, []string{"*"}, true, 403}, + {"key-owner-restricted", &auth.PermissionConstraints{Providers: []string{"azure"}}, []string{"*"}, true, 403}, + {"key-unrestricted", nil, []string{"*"}, true, 200}, + {"amount-not-purchase", &auth.PermissionConstraints{MaxPurchaseAmount: 1}, []string{"*"}, false, 200}, + {"non-admin", nil, []string{"*"}, false, 200}, + } { + t.Run(route.name+"/"+tc.name, func(t *testing.T) { + want := tc.want + if route.name == "ladder" && (tc.name == "all-current-accounts" || tc.name == "matching-account") { + want = http.StatusOK + } + if route.name == "global" && tc.name == "non-admin" { + want = http.StatusForbidden + } + if route.name == "ladder" && tc.name == "independent-account-scope" { + want = http.StatusNotFound + } + require.NoError(t, store.SaveGlobalConfig(ctx, initial)) + before, loadErr := store.GetLadderConfigs(ctx) + require.NoError(t, loadErr) + if route.name == "ladder" && len(before) > 0 { + before[0].Enabled = false + _, loadErr = store.UpsertLadderConfig(ctx, &before[0]) + require.NoError(t, loadErr) + before, loadErr = store.GetLadderConfigs(ctx) + require.NoError(t, loadErr) + } + permission := auth.Permission{Action: auth.ActionUpdate, Resource: auth.ResourceConfig, Constraints: tc.constraints} + if route.name == "global" && tc.name != "non-admin" { + permission.Action, permission.Resource = auth.ActionAdmin, auth.ResourceAll + } + group := &auth.Group{Name: uuid.NewString(), Permissions: []auth.Permission{permission}, AllowedAccounts: tc.allowed} + if tc.key && tc.name != "key-owner-restricted" { + group.Permissions[0].Constraints = nil + } + require.NoError(t, authStore.CreateGroup(ctx, group)) + user := &auth.User{Email: uuid.NewString() + "@example.test", Active: true, GroupIDs: []string{group.ID}} + require.NoError(t, authStore.CreateUser(ctx, user)) + token := uuid.NewString() + hash := sha256.Sum256([]byte(token)) + require.NoError(t, authStore.CreateSession(ctx, &auth.Session{Token: hex.EncodeToString(hash[:]), UserID: user.ID, Email: user.Email, CreatedAt: time.Now(), ExpiresAt: time.Now().Add(time.Hour)})) + mac := hmac.New(sha256.New, csrfKey) + _, err = mac.Write([]byte(token)) + require.NoError(t, err) + req, reqErr := http.NewRequestWithContext(ctx, http.MethodPut, server.URL+route.path, strings.NewReader(route.body)) + require.NoError(t, reqErr) + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("X-CSRF-Token", hex.EncodeToString(mac.Sum(nil))) + req.Header.Set("Content-Type", "application/json") + if tc.key { + if tc.name == "key-owner-restricted" { + permission.Constraints = nil + } + key := "cudly_" + uuid.NewString() + keyHash := sha256.Sum256([]byte(key)) + require.NoError(t, authStore.CreateAPIKey(ctx, &auth.UserAPIKey{UserID: user.ID, Name: "test", KeyPrefix: key[:12], KeyHash: base64.RawURLEncoding.EncodeToString(keyHash[:]), IsActive: true, Permissions: []auth.Permission{permission}})) + req.Header.Set("X-API-Key", key) + } + resp, reqErr := server.Client().Do(req) + require.NoError(t, reqErr) + body, readErr := io.ReadAll(resp.Body) + require.NoError(t, resp.Body.Close()) + require.NoError(t, readErr) + assert.Equal(t, want, resp.StatusCode, "%s", body) + after, loadErr := store.GetGlobalConfig(ctx) + require.NoError(t, loadErr) + ladders, loadErr := store.GetLadderConfigs(ctx) + require.NoError(t, loadErr) + if want != http.StatusOK { + assert.Equal(t, initial, after) + assert.Equal(t, before, ladders) + } else { + switch route.name { + case "ladder": + require.Len(t, ladders, 1) + assert.True(t, ladders[0].Enabled) + case "global": + assert.True(t, after.LadderingEnabled) + case "exchange": + assert.True(t, after.RIExchangeEnabled) + } + } + }) + } + } +}