diff --git a/frontend/src/__tests__/permissions.test.ts b/frontend/src/__tests__/permissions.test.ts index 2e679df0..3052d961 100644 --- a/frontend/src/__tests__/permissions.test.ts +++ b/frontend/src/__tests__/permissions.test.ts @@ -132,6 +132,7 @@ describe('permissions', () => { expect(canAccess('view', 'users')).toBe(true); expect(canAccess('delete', 'plans')).toBe(true); expect(canAccess('execute', 'purchases')).toBe(true); + expect(canAccess('execute', 'ri-exchange')).toBe(true); expect(canAccess('view', 'accounts')).toBe(true); }); @@ -193,6 +194,31 @@ describe('permissions', () => { expect(canAccess('view', 'users')).toBe(false); }); + test('execute:purchases in effective set does NOT imply execute:ri-exchange (issue #660 isolation)', () => { + // RI exchanges are financially irreversible (no AWS rollback). The + // permission split intentionally makes execute:ri-exchange disjoint + // from execute:purchases so granting one does not transitively grant + // the other. This guards the frontend gating mirror of the backend + // gate exercised in router_660_permission_flips_test.go. + const perms: PermissionEntry[] = [ + { action: 'execute', resource: 'purchases' }, + ]; + mockUserWithGroups([STD_GID], perms); + expect(canAccess('execute', 'purchases')).toBe(true); + expect(canAccess('execute', 'ri-exchange')).toBe(false); + }); + + test('execute:ri-exchange in effective set grants only ri-exchange, not purchases', () => { + // Inverse of the previous test: holding execute:ri-exchange does NOT + // imply execute:purchases either. The two permissions are disjoint. + const perms: PermissionEntry[] = [ + { action: 'execute', resource: 'ri-exchange' }, + ]; + mockUserWithGroups([STD_GID], perms); + expect(canAccess('execute', 'ri-exchange')).toBe(true); + expect(canAccess('execute', 'purchases')).toBe(false); + }); + test('admin wildcard in effectivePermissions grants everything', () => { const perms: PermissionEntry[] = [ { action: 'admin', resource: '*' }, diff --git a/frontend/src/permissions.ts b/frontend/src/permissions.ts index c7157e48..998ccba9 100644 --- a/frontend/src/permissions.ts +++ b/frontend/src/permissions.ts @@ -60,6 +60,11 @@ export type Resource = | 'users' | 'groups' | 'api-keys' + // ri-exchange is a separate resource from purchases so that execute:ri-exchange + // can be granted independently. RI exchanges are financially irreversible; + // keeping the permission disjoint prevents execute:purchases from implicitly + // covering the exchange path (issue #660). + | 'ri-exchange' | '*'; /** diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index e7046cbc..cbd762be 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -576,8 +576,10 @@ func validateExecuteExchangeBody(body ExchangeExecuteRequestBody) error { } // executeExchange executes an RI exchange with a spend-cap guardrail. +// Requires execute:ri-exchange (deliberately separate from execute:purchases) +// because RI exchanges are financially irreversible once submitted to AWS. func (h *Handler) executeExchange(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) { - if _, err := h.requirePermission(ctx, req, "execute", "purchases"); err != nil { + if _, err := h.requirePermission(ctx, req, "execute", "ri-exchange"); err != nil { return nil, err } diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index ec83a448..68ad1a19 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -616,6 +616,275 @@ paths: '404': $ref: '#/components/responses/NotFound' + # ---- RI Exchange -------------------------------------------------------- + /api/ri-exchange/instances: + get: + operationId: listConvertibleRIs + tags: [RIExchange] + summary: List convertible Reserved Instances eligible for exchange + parameters: + - $ref: '#/components/parameters/AccountID' + responses: + '200': + description: Convertible RI list + content: + application/json: + schema: + type: object + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/azure-instances: + get: + operationId: listExchangeableAzureRIs + tags: [RIExchange] + summary: List Azure Reserved Instances eligible for exchange + parameters: + - $ref: '#/components/parameters/AccountID' + responses: + '200': + description: Azure exchangeable RI list + content: + application/json: + schema: + type: object + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/target-offerings: + get: + operationId: listTargetOfferings + tags: [RIExchange] + summary: List available RI offerings to exchange into + parameters: + - $ref: '#/components/parameters/AccountID' + responses: + '200': + description: Target offering list + content: + application/json: + schema: + type: object + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/utilization: + get: + operationId: getRIUtilization + tags: [RIExchange] + summary: Get utilization metrics for Reserved Instances + responses: + '200': + description: RI utilization data + content: + application/json: + schema: + type: object + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/reshape-recommendations: + get: + operationId: getReshapeRecommendations + tags: [RIExchange] + summary: Get reshape recommendations for convertible RIs + responses: + '200': + description: Reshape recommendation list + content: + application/json: + schema: + type: object + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/quote: + post: + operationId: getExchangeQuote + tags: [RIExchange] + summary: Get a quote for an RI exchange before executing + description: > + Requires `view:purchases` permission. Returns the exchange valuation + without committing any financial transaction. + parameters: + - $ref: '#/components/parameters/CSRFToken' + requestBody: + required: true + content: + application/json: + schema: + type: object + responses: + '200': + description: Exchange quote + content: + application/json: + schema: + type: object + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/execute: + post: + operationId: executeRIExchange + tags: [RIExchange] + summary: Execute an RI exchange (irreversible) + description: > + Requires `execute:ri-exchange` permission. This is intentionally a + separate, narrower permission from `execute:purchases` because RI + exchanges submitted to AWS cannot be rolled back. Non-admin users must + be explicitly granted `execute:ri-exchange` via a custom group; there + is no default user-role grant. + parameters: + - $ref: '#/components/parameters/CSRFToken' + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [ri_ids, targets, max_payment_due_usd] + properties: + ri_ids: + type: array + items: + type: string + targets: + type: array + items: + type: object + max_payment_due_usd: + type: string + description: Spend-cap guardrail (decimal string, e.g. "1000.00") + responses: + '200': + description: Exchange submitted + content: + application/json: + schema: + type: object + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/config: + get: + operationId: getRIExchangeConfig + tags: [RIExchange] + summary: Get RI exchange configuration + description: Requires `view:config` permission. + responses: + '200': + description: RI exchange configuration + content: + application/json: + schema: + type: object + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + put: + operationId: updateRIExchangeConfig + tags: [RIExchange] + summary: Update RI exchange configuration + description: Requires `update:config` permission. + parameters: + - $ref: '#/components/parameters/CSRFToken' + requestBody: + required: true + content: + application/json: + schema: + type: object + responses: + '200': + description: Updated configuration + content: + application/json: + schema: + type: object + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/history: + get: + operationId: getRIExchangeHistory + tags: [RIExchange] + summary: Get RI exchange history + responses: + '200': + description: RI exchange history list + content: + application/json: + schema: + type: object + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + + /api/ri-exchange/approve/{id}: + parameters: + - $ref: '#/components/parameters/ResourceID' + post: + operationId: approveRIExchange + tags: [RIExchange] + summary: Approve a pending RI exchange (token-based, no session required) + security: [] + responses: + '200': + description: Exchange approved + content: + application/json: + schema: + $ref: '#/components/schemas/StatusResponse' + '400': + $ref: '#/components/responses/BadRequest' + '404': + $ref: '#/components/responses/NotFound' + + /api/ri-exchange/reject/{id}: + parameters: + - $ref: '#/components/parameters/ResourceID' + post: + operationId: rejectRIExchange + tags: [RIExchange] + summary: Reject a pending RI exchange (token-based, no session required) + security: [] + responses: + '200': + description: Exchange rejected + content: + application/json: + schema: + $ref: '#/components/schemas/StatusResponse' + '400': + $ref: '#/components/responses/BadRequest' + '404': + $ref: '#/components/responses/NotFound' + # ---- History ------------------------------------------------------------ /api/history: get: diff --git a/internal/api/router_660_permission_flips_test.go b/internal/api/router_660_permission_flips_test.go index aad023e3..b435fd98 100644 --- a/internal/api/router_660_permission_flips_test.go +++ b/internal/api/router_660_permission_flips_test.go @@ -264,13 +264,15 @@ func TestDeletePlannedPurchase_PermissionGate(t *testing.T) { // ---- RI Exchange ---------------------------------------------------------- // TestExecuteExchange_PermissionGate covers POST /api/ri-exchange/execute. -// The handler requires execute:purchases. +// The handler requires execute:ri-exchange (not execute:purchases) because +// RI exchanges are financially irreversible; the two permissions are +// intentionally disjoint so granting one does not implicitly grant the other. func TestExecuteExchange_PermissionGate(t *testing.T) { ctx := context.Background() const userID = "55555555-5555-5555-5555-555555555555" - t.Run("user without execute:purchases is rejected with 403", func(t *testing.T) { - mockAuth := authForUserWith(ctx, t, userID, "execute", "purchases", false) + t.Run("user without execute:ri-exchange is rejected with 403", func(t *testing.T) { + mockAuth := authForUserWith(ctx, t, userID, "execute", "ri-exchange", false) h := &Handler{auth: mockAuth} body := `{"ri_ids":["ri-abc"],"targets":[{"offering_id":"of-1"}],"max_payment_due_usd":"1000"}` _, err := h.executeExchange(ctx, reqWithBearerAndBody("user-token", body)) @@ -281,14 +283,33 @@ func TestExecuteExchange_PermissionGate(t *testing.T) { // the real AWS exchange client. We verify the permission gate passes by // checking the error is NOT a 403 when the permission is granted; the // AWS SDK call will fail with a non-403 (connection refused / 500). - t.Run("user with execute:purchases clears 403 gate", func(t *testing.T) { - mockAuth := authForUserWith(ctx, t, userID, "execute", "purchases", true) + t.Run("user with execute:ri-exchange clears 403 gate", func(t *testing.T) { + mockAuth := authForUserWith(ctx, t, userID, "execute", "ri-exchange", true) h := &Handler{auth: mockAuth} body := `{"ri_ids":["ri-abc"],"targets":[{"offering_id":"of-1"}],"max_payment_due_usd":"1000"}` _, err := h.executeExchange(ctx, reqWithBearerAndBody("user-token", body)) // The error will be from the AWS SDK (not a 403), proving the gate passed. assertNotForbidden(t, err) }) + + // Isolation test: execute:purchases does NOT grant access to RI exchange. + // This is the key invariant of the permission split: the two resources are + // disjoint; holding one does not imply the other. + t.Run("user with execute:purchases cannot execute RI exchange (403)", func(t *testing.T) { + // The mock grants execute:purchases (true) but the handler asks for + // execute:ri-exchange. HasPermissionAPI will be called with the + // ri-exchange resource and must return false. + mockAuth := new(MockAuthService) + mockAuth.On("ValidateSession", ctx, "user-token").Return(userSessionFixture(userID), nil) + // Handler asks for "execute":"ri-exchange"; this user does NOT have it. + mockAuth.On("HasPermissionAPI", ctx, userID, "execute", "ri-exchange").Return(false, nil) + t.Cleanup(func() { mockAuth.AssertExpectations(t) }) + + h := &Handler{auth: mockAuth} + body := `{"ri_ids":["ri-abc"],"targets":[{"offering_id":"of-1"}],"max_payment_due_usd":"1000"}` + _, err := h.executeExchange(ctx, reqWithBearerAndBody("user-token", body)) + assert403(t, err) + }) } // TestUpdateRIExchangeConfig_PermissionGate covers PUT /api/ri-exchange/config. diff --git a/internal/auth/types.go b/internal/auth/types.go index 5f8fc76d..0601637d 100644 --- a/internal/auth/types.go +++ b/internal/auth/types.go @@ -380,7 +380,15 @@ const ( ResourceUsers = "users" ResourceGroups = "groups" ResourceAPIKeys = "api-keys" - ResourceAll = "*" + // ResourceRIExchange gates RI-exchange-specific operations. The execute + // verb on this resource is deliberately separate from execute:purchases + // because RI exchanges are financially irreversible (the AWS API does + // not have a rollback path once an exchange is submitted). Admins carry + // implicit access via {ActionAdmin, ResourceAll}. Non-admin roles must + // be explicitly granted execute:ri-exchange by a custom group; there is + // no default user-role grant. + ResourceRIExchange = "ri-exchange" + ResourceAll = "*" ) // DefaultAdminPermissions returns full admin permissions