diff --git a/internal/api/apikey_constraints_roundtrip_test.go b/internal/api/apikey_constraints_roundtrip_test.go new file mode 100644 index 000000000..b1154602b --- /dev/null +++ b/internal/api/apikey_constraints_roundtrip_test.go @@ -0,0 +1,212 @@ +package api //nolint:revive // var-naming: package name "api" is intentional for handler package + +import ( + "context" + "encoding/base64" + "encoding/json" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// Issue #951: the create and list API-key responses must return permission +// constraints in the documented OpenAPI shape (constraints.accounts / +// constraints.max_amount), the same shape POST /api/api-keys accepts. These +// tests drive the real HTTP handler into the real auth.Service (backed by +// auth.MockStore, so no database). That the create path stores the +// constraints is covered at the service and adapter level by #957's tests. + +const ( + roundtripUserID = "user-951" + roundtripToken = "session-951" + roundtripCSRF = "csrf-951" + roundtripAccount = "2f1c8e40-0000-4000-8000-000000000951" + roundtripOwnerPw = "Owner-951-pass" +) + +// realAPIKeyAuth answers session and permission checks from MockAuthService +// but sends the API-key create and list calls to a real auth.Service, so the +// handler-to-service conversion under test is the production one. +type realAPIKeyAuth struct { + *MockAuthService + service *auth.Service +} + +func (a *realAPIKeyAuth) CreateAPIKeyAPI(ctx context.Context, userID string, req any) (any, error) { + return a.service.CreateAPIKeyAPI(ctx, userID, req) +} + +func (a *realAPIKeyAuth) ListUserAPIKeysAPI(ctx context.Context, userID string) (any, error) { + return a.service.ListUserAPIKeysAPI(ctx, userID) +} + +// newAPIKeyRoundtripHandler returns a handler wired to a real auth.Service, +// the store behind it, and a pointer that receives the key the service hands +// to the store. +func newAPIKeyRoundtripHandler(t *testing.T) (*Handler, *auth.MockStore, **auth.UserAPIKey) { + t.Helper() + store := new(auth.MockStore) + user := &auth.User{ + ID: roundtripUserID, + Email: "owner@example.com", + Active: true, + PasswordHash: auth.TestPasswordHash(t, roundtripOwnerPw), + GroupIDs: []string{auth.DefaultAdminGroupID}, + } + store.On("GetUserByID", mock.Anything, roundtripUserID).Return(user, nil) + store.On("GetGroup", mock.Anything, auth.DefaultAdminGroupID).Return(&auth.Group{ + ID: auth.DefaultAdminGroupID, + Permissions: []auth.Permission{{Action: auth.ActionAdmin, Resource: auth.ResourceAll}}, + }, nil) + + stored := new(*auth.UserAPIKey) + store.On("CreateAPIKey", mock.Anything, mock.AnythingOfType("*auth.UserAPIKey")). + Run(func(args mock.Arguments) { *stored = args.Get(1).(*auth.UserAPIKey) }). + Return(nil) + + mockAuth := new(MockAuthService) + mockAuth.On("ValidateSession", mock.Anything, roundtripToken). + Return(&Session{UserID: roundtripUserID, Email: user.Email}, nil) + mockAuth.On("ValidateCSRFToken", mock.Anything, roundtripToken, roundtripCSRF).Return(nil).Maybe() + mockAuth.grantAdmin() + + service := auth.NewService(auth.ServiceConfig{Store: store, CSRFKey: auth.TestCSRFKey()}) + return &Handler{auth: &realAPIKeyAuth{MockAuthService: mockAuth, service: service}}, store, stored +} + +func apiKeyRoundtripRequest(t *testing.T, h *Handler, method, body string) *events.LambdaFunctionURLResponse { + t.Helper() + resp, err := h.HandleRequest(context.Background(), &events.LambdaFunctionURLRequest{ + Headers: map[string]string{ + "Authorization": "Bearer " + roundtripToken, + "Content-Type": "application/json", + "X-CSRF-Token": roundtripCSRF, + }, + Body: body, + RequestContext: events.LambdaFunctionURLRequestContext{ + HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: method, Path: "/api/api-keys"}, + }, + }) + require.NoError(t, err) + require.NotNil(t, resp) + return resp +} + +func constrainedAPIKeyBody(t *testing.T, maxAmount float64) string { + t.Helper() + body, err := json.Marshal(map[string]any{ + "name": "scoped key", + "password": base64.StdEncoding.EncodeToString([]byte(roundtripOwnerPw)), + "expires_at": time.Now().Add(24 * time.Hour).UTC().Format(time.RFC3339), + "permissions": []map[string]any{{ + "action": auth.ActionView, + "resource": auth.ResourceRecommendations, + "constraints": map[string]any{ + "accounts": []string{roundtripAccount}, + "providers": []string{"aws"}, + "services": []string{"ec2"}, + "regions": []string{"us-east-1"}, + "max_amount": maxAmount, + }, + }}, + }) + require.NoError(t, err) + return string(body) +} + +// requireDocumentedConstraints asserts a serialized constraints object uses +// the OpenAPI field names and carries every dimension that was requested. +func requireDocumentedConstraints(t *testing.T, constraints map[string]any) { + t.Helper() + assert.Equal(t, []any{roundtripAccount}, constraints["accounts"]) + assert.Equal(t, 500.5, constraints["max_amount"]) + assert.Equal(t, []any{"aws"}, constraints["providers"]) + assert.Equal(t, []any{"ec2"}, constraints["services"]) + assert.Equal(t, []any{"us-east-1"}, constraints["regions"]) + assert.NotContains(t, constraints, "account_ids", "response must use the documented field name") + assert.NotContains(t, constraints, "max_purchase_amount", "response must use the documented field name") +} + +func firstPermissionConstraints(t *testing.T, perms any) map[string]any { + t.Helper() + list, ok := perms.([]any) + require.True(t, ok, "permissions = %#v", perms) + require.Len(t, list, 1) + perm, ok := list[0].(map[string]any) + require.True(t, ok) + constraints, ok := perm["constraints"].(map[string]any) + require.True(t, ok, "constraints missing from %#v", perm) + return constraints +} + +func TestHandleRequest_CreateAPIKey_ResponseUsesDocumentedConstraintShape(t *testing.T) { + h, _, _ := newAPIKeyRoundtripHandler(t) + + resp := apiKeyRoundtripRequest(t, h, "POST", constrainedAPIKeyBody(t, 500.5)) + require.Equal(t, 200, resp.StatusCode, "body: %s", resp.Body) + + var created struct { + Info struct { + Permissions any `json:"permissions"` + } `json:"info"` + } + require.NoError(t, json.Unmarshal([]byte(resp.Body), &created)) + requireDocumentedConstraints(t, firstPermissionConstraints(t, created.Info.Permissions)) +} + +func TestHandleRequest_ListAPIKeys_ReturnsDocumentedConstraintShape(t *testing.T) { + h, store, _ := newAPIKeyRoundtripHandler(t) + + // Seeded directly in the stored (auth) shape so this checks the listing + // response independently of the create path. + store.On("ListAPIKeysByUser", mock.Anything, roundtripUserID).Return([]*auth.UserAPIKey{{ + ID: "key-951", + UserID: roundtripUserID, + Name: "scoped key", + IsActive: true, + Permissions: []auth.Permission{{ + Action: auth.ActionView, + Resource: auth.ResourceRecommendations, + Constraints: &auth.PermissionConstraints{ + AccountIDs: []string{roundtripAccount}, + Providers: []string{"aws"}, + Services: []string{"ec2"}, + Regions: []string{"us-east-1"}, + MaxPurchaseAmount: 500.5, + }, + }}, + }}, nil) + + resp := apiKeyRoundtripRequest(t, h, "GET", "") + require.Equal(t, 200, resp.StatusCode, "body: %s", resp.Body) + + var listed struct { + APIKeys []struct { + Permissions any `json:"permissions"` + } `json:"api_keys"` + } + require.NoError(t, json.Unmarshal([]byte(resp.Body), &listed)) + require.Len(t, listed.APIKeys, 1) + requireDocumentedConstraints(t, firstPermissionConstraints(t, listed.APIKeys[0].Permissions)) +} + +// The negative spend-cap check from issue #83 runs on the converted +// permissions, so it must be reachable over HTTP, not only at the service +// level. Before #957 the negative max_amount was dropped during decoding and +// the key was minted with no cap at all. +func TestHandleRequest_CreateAPIKey_RejectsNegativeMaxAmount(t *testing.T) { + h, _, stored := newAPIKeyRoundtripHandler(t) + + resp := apiKeyRoundtripRequest(t, h, "POST", constrainedAPIKeyBody(t, -1)) + + assert.Equal(t, 400, resp.StatusCode, "body: %s", resp.Body) + assert.Contains(t, resp.Body, "invalid max purchase amount") + // stored is set by the store's CreateAPIKey mock, so nil means the + // service never wrote the key. + assert.Nil(t, *stored, "a key with a negative cap must not be stored") +} diff --git a/internal/auth/service_apikeys_api.go b/internal/auth/service_apikeys_api.go index 2f5dc4cc5..b86d74252 100644 --- a/internal/auth/service_apikeys_api.go +++ b/internal/auth/service_apikeys_api.go @@ -72,19 +72,22 @@ func effectiveLifetimeUsage(key *UserAPIKey) *int64 { return &total } -// normalizeAPIKeyPermissions returns perms unchanged, or []Permission{} when -// nil. CreateAPIKey now rejects new keys with zero permissions (issue #61), +// normalizeAPIKeyPermissions converts a stored key's permissions to the +// documented wire shape (APIPermission: constraints.accounts / +// constraints.max_amount, issue #951), returning []APIPermission{} when perms +// is nil. CreateAPIKey now rejects new keys with zero permissions (issue #61), // but a legacy key minted before that fix can still have a NULL permissions // column, which the JSONB scan surfaces as a nil slice. Since // APIKeyInfo.Permissions is no longer omitempty (issue #61), a raw nil would // serialize as "permissions":null instead of the intended "permissions":[] // -- both mean "no scope", but a client that expects the field to always be // an array (never null) would otherwise have to special-case it. -func normalizeAPIKeyPermissions(perms []Permission) []Permission { - if perms == nil { - return []Permission{} +func normalizeAPIKeyPermissions(perms []Permission) []APIPermission { + out := make([]APIPermission, len(perms)) + for i, p := range perms { + out[i] = permissionToAPIPermission(p) } - return perms + return out } // API wrapper methods for API key operations @@ -118,8 +121,11 @@ type APIKeyInfo struct { // simply has no info populated. Hiding it behind omitempty made a // zero-scope key indistinguishable from a fully-scoped one in // listAPIKeys, which was itself part of the audit gap the issue raised. - Permissions []Permission `json:"permissions"` - IsActive bool `json:"is_active"` + // It uses the documented wire shape (constraints.accounts / + // constraints.max_amount), the same one the create request accepts + // (issue #951). + Permissions []APIPermission `json:"permissions"` + IsActive bool `json:"is_active"` // Usage counters (issue #340/#344 deferred sub-task). RequestCountWindow // is a fixed/tumbling window count, not a true rolling 24h total, and is // zero once that window has closed -- see effectiveWindowUsage, diff --git a/internal/auth/service_apikeys_api_test.go b/internal/auth/service_apikeys_api_test.go index e16d11b0d..413e2f934 100644 --- a/internal/auth/service_apikeys_api_test.go +++ b/internal/auth/service_apikeys_api_test.go @@ -27,7 +27,7 @@ func TestService_CreateAPIKeyAPI(t *testing.T) { GroupIDs: []string{DefaultAdminGroupID}, } - permissions := []Permission{ + permissions := []APIPermission{ {Action: ActionView, Resource: ResourceRecommendations}, }