Skip to content

feat(api/auth): split write into per-resource permissions (closes #660) #891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 1 commit into from
Jun 3, 2026
Merged

feat(api/auth): split write into per-resource permissions (closes #660) #891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions frontend/src/__tests__/permissions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down Expand Up @@ -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: '*' },
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/permissions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
| '*';

/**
Expand Down
4 changes: 3 additions & 1 deletion internal/api/handler_ri_exchange.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
269 changes: 269 additions & 0 deletions internal/api/openapi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
31 changes: 26 additions & 5 deletions internal/api/router_660_permission_flips_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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.
Expand Down
Loading
Loading