Skip to content

Commit eb85a50

Browse files
authored
feat(api/auth): split execute:ri-exchange from execute:purchases (closes #660) (#891)
RI exchanges are financially irreversible once submitted to AWS. Keeping execute:ri-exchange disjoint from execute:purchases ensures a user who can initiate regular purchases cannot inadvertently (or intentionally) trigger an RI exchange without an explicit additional grant. Changes: - Add ResourceRIExchange = "ri-exchange" constant to internal/auth/types.go - Update executeExchange handler to require execute:ri-exchange (was execute:purchases) - Update TestExecuteExchange_PermissionGate to assert the new permission verb; add isolation sub-test proving execute:purchases does not grant ri-exchange access - Add /api/ri-exchange/* routes to openapi.yaml with 403 responses and permission documentation on the execute endpoint - Add 'ri-exchange' to the frontend Resource closed-union type and extend canAccess tests: admin true, user false (no default grant); add isolation sub-tests proving execute:purchases does not imply execute:ri-exchange (and vice versa) on the frontend gating mirror Rebased onto feat/multicloud-web-frontend post #922 (effectivePermissions migration). The conflict in frontend/src/__tests__/permissions.test.ts was resolved by re-expressing the user-default-deny invariant in the new effective-permissions model: a non-admin holding execute:purchases is still denied execute:ri-exchange. Em-dashes scrubbed from added prose. No DB migration needed: role defaults are computed at runtime from Go constants; no DB table stores the permission set. No conflict with PR #884 (fix/476): that PR adds 403 to plans/purchases routes; this PR adds a new ri-exchange section that #884 does not touch.
1 parent f3920e4 commit eb85a50

6 files changed

Lines changed: 338 additions & 7 deletions

File tree

‎frontend/src/__tests__/permissions.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ describe('permissions', () => {
132132
expect(canAccess('view', 'users')).toBe(true);
133133
expect(canAccess('delete', 'plans')).toBe(true);
134134
expect(canAccess('execute', 'purchases')).toBe(true);
135+
expect(canAccess('execute', 'ri-exchange')).toBe(true);
135136
expect(canAccess('view', 'accounts')).toBe(true);
136137
});
137138

@@ -193,6 +194,31 @@ describe('permissions', () => {
193194
expect(canAccess('view', 'users')).toBe(false);
194195
});
195196

197+
test('execute:purchases in effective set does NOT imply execute:ri-exchange (issue #660 isolation)', () => {
198+
// RI exchanges are financially irreversible (no AWS rollback). The
199+
// permission split intentionally makes execute:ri-exchange disjoint
200+
// from execute:purchases so granting one does not transitively grant
201+
// the other. This guards the frontend gating mirror of the backend
202+
// gate exercised in router_660_permission_flips_test.go.
203+
const perms: PermissionEntry[] = [
204+
{ action: 'execute', resource: 'purchases' },
205+
];
206+
mockUserWithGroups([STD_GID], perms);
207+
expect(canAccess('execute', 'purchases')).toBe(true);
208+
expect(canAccess('execute', 'ri-exchange')).toBe(false);
209+
});
210+
211+
test('execute:ri-exchange in effective set grants only ri-exchange, not purchases', () => {
212+
// Inverse of the previous test: holding execute:ri-exchange does NOT
213+
// imply execute:purchases either. The two permissions are disjoint.
214+
const perms: PermissionEntry[] = [
215+
{ action: 'execute', resource: 'ri-exchange' },
216+
];
217+
mockUserWithGroups([STD_GID], perms);
218+
expect(canAccess('execute', 'ri-exchange')).toBe(true);
219+
expect(canAccess('execute', 'purchases')).toBe(false);
220+
});
221+
196222
test('admin wildcard in effectivePermissions grants everything', () => {
197223
const perms: PermissionEntry[] = [
198224
{ action: 'admin', resource: '*' },

‎frontend/src/permissions.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ export type Resource =
6060
| 'users'
6161
| 'groups'
6262
| 'api-keys'
63+
// ri-exchange is a separate resource from purchases so that execute:ri-exchange
64+
// can be granted independently. RI exchanges are financially irreversible;
65+
// keeping the permission disjoint prevents execute:purchases from implicitly
66+
// covering the exchange path (issue #660).
67+
| 'ri-exchange'
6368
| '*';
6469

6570
/**

‎internal/api/handler_ri_exchange.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,8 +576,10 @@ func validateExecuteExchangeBody(body ExchangeExecuteRequestBody) error {
576576
}
577577

578578
// executeExchange executes an RI exchange with a spend-cap guardrail.
579+
// Requires execute:ri-exchange (deliberately separate from execute:purchases)
580+
// because RI exchanges are financially irreversible once submitted to AWS.
579581
func (h *Handler) executeExchange(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) {
580-
if _, err := h.requirePermission(ctx, req, "execute", "purchases"); err != nil {
582+
if _, err := h.requirePermission(ctx, req, "execute", "ri-exchange"); err != nil {
581583
return nil, err
582584
}
583585

‎internal/api/openapi.yaml‎

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,275 @@ paths:
616616
'404':
617617
$ref: '#/components/responses/NotFound'
618618

619+
# ---- RI Exchange --------------------------------------------------------
620+
/api/ri-exchange/instances:
621+
get:
622+
operationId: listConvertibleRIs
623+
tags: [RIExchange]
624+
summary: List convertible Reserved Instances eligible for exchange
625+
parameters:
626+
- $ref: '#/components/parameters/AccountID'
627+
responses:
628+
'200':
629+
description: Convertible RI list
630+
content:
631+
application/json:
632+
schema:
633+
type: object
634+
'401':
635+
$ref: '#/components/responses/Unauthorized'
636+
'403':
637+
$ref: '#/components/responses/Forbidden'
638+
639+
/api/ri-exchange/azure-instances:
640+
get:
641+
operationId: listExchangeableAzureRIs
642+
tags: [RIExchange]
643+
summary: List Azure Reserved Instances eligible for exchange
644+
parameters:
645+
- $ref: '#/components/parameters/AccountID'
646+
responses:
647+
'200':
648+
description: Azure exchangeable RI list
649+
content:
650+
application/json:
651+
schema:
652+
type: object
653+
'401':
654+
$ref: '#/components/responses/Unauthorized'
655+
'403':
656+
$ref: '#/components/responses/Forbidden'
657+
658+
/api/ri-exchange/target-offerings:
659+
get:
660+
operationId: listTargetOfferings
661+
tags: [RIExchange]
662+
summary: List available RI offerings to exchange into
663+
parameters:
664+
- $ref: '#/components/parameters/AccountID'
665+
responses:
666+
'200':
667+
description: Target offering list
668+
content:
669+
application/json:
670+
schema:
671+
type: object
672+
'401':
673+
$ref: '#/components/responses/Unauthorized'
674+
'403':
675+
$ref: '#/components/responses/Forbidden'
676+
677+
/api/ri-exchange/utilization:
678+
get:
679+
operationId: getRIUtilization
680+
tags: [RIExchange]
681+
summary: Get utilization metrics for Reserved Instances
682+
responses:
683+
'200':
684+
description: RI utilization data
685+
content:
686+
application/json:
687+
schema:
688+
type: object
689+
'401':
690+
$ref: '#/components/responses/Unauthorized'
691+
'403':
692+
$ref: '#/components/responses/Forbidden'
693+
694+
/api/ri-exchange/reshape-recommendations:
695+
get:
696+
operationId: getReshapeRecommendations
697+
tags: [RIExchange]
698+
summary: Get reshape recommendations for convertible RIs
699+
responses:
700+
'200':
701+
description: Reshape recommendation list
702+
content:
703+
application/json:
704+
schema:
705+
type: object
706+
'401':
707+
$ref: '#/components/responses/Unauthorized'
708+
'403':
709+
$ref: '#/components/responses/Forbidden'
710+
711+
/api/ri-exchange/quote:
712+
post:
713+
operationId: getExchangeQuote
714+
tags: [RIExchange]
715+
summary: Get a quote for an RI exchange before executing
716+
description: >
717+
Requires `view:purchases` permission. Returns the exchange valuation
718+
without committing any financial transaction.
719+
parameters:
720+
- $ref: '#/components/parameters/CSRFToken'
721+
requestBody:
722+
required: true
723+
content:
724+
application/json:
725+
schema:
726+
type: object
727+
responses:
728+
'200':
729+
description: Exchange quote
730+
content:
731+
application/json:
732+
schema:
733+
type: object
734+
'400':
735+
$ref: '#/components/responses/BadRequest'
736+
'401':
737+
$ref: '#/components/responses/Unauthorized'
738+
'403':
739+
$ref: '#/components/responses/Forbidden'
740+
741+
/api/ri-exchange/execute:
742+
post:
743+
operationId: executeRIExchange
744+
tags: [RIExchange]
745+
summary: Execute an RI exchange (irreversible)
746+
description: >
747+
Requires `execute:ri-exchange` permission. This is intentionally a
748+
separate, narrower permission from `execute:purchases` because RI
749+
exchanges submitted to AWS cannot be rolled back. Non-admin users must
750+
be explicitly granted `execute:ri-exchange` via a custom group; there
751+
is no default user-role grant.
752+
parameters:
753+
- $ref: '#/components/parameters/CSRFToken'
754+
requestBody:
755+
required: true
756+
content:
757+
application/json:
758+
schema:
759+
type: object
760+
required: [ri_ids, targets, max_payment_due_usd]
761+
properties:
762+
ri_ids:
763+
type: array
764+
items:
765+
type: string
766+
targets:
767+
type: array
768+
items:
769+
type: object
770+
max_payment_due_usd:
771+
type: string
772+
description: Spend-cap guardrail (decimal string, e.g. "1000.00")
773+
responses:
774+
'200':
775+
description: Exchange submitted
776+
content:
777+
application/json:
778+
schema:
779+
type: object
780+
'400':
781+
$ref: '#/components/responses/BadRequest'
782+
'401':
783+
$ref: '#/components/responses/Unauthorized'
784+
'403':
785+
$ref: '#/components/responses/Forbidden'
786+
787+
/api/ri-exchange/config:
788+
get:
789+
operationId: getRIExchangeConfig
790+
tags: [RIExchange]
791+
summary: Get RI exchange configuration
792+
description: Requires `view:config` permission.
793+
responses:
794+
'200':
795+
description: RI exchange configuration
796+
content:
797+
application/json:
798+
schema:
799+
type: object
800+
'401':
801+
$ref: '#/components/responses/Unauthorized'
802+
'403':
803+
$ref: '#/components/responses/Forbidden'
804+
put:
805+
operationId: updateRIExchangeConfig
806+
tags: [RIExchange]
807+
summary: Update RI exchange configuration
808+
description: Requires `update:config` permission.
809+
parameters:
810+
- $ref: '#/components/parameters/CSRFToken'
811+
requestBody:
812+
required: true
813+
content:
814+
application/json:
815+
schema:
816+
type: object
817+
responses:
818+
'200':
819+
description: Updated configuration
820+
content:
821+
application/json:
822+
schema:
823+
type: object
824+
'400':
825+
$ref: '#/components/responses/BadRequest'
826+
'401':
827+
$ref: '#/components/responses/Unauthorized'
828+
'403':
829+
$ref: '#/components/responses/Forbidden'
830+
831+
/api/ri-exchange/history:
832+
get:
833+
operationId: getRIExchangeHistory
834+
tags: [RIExchange]
835+
summary: Get RI exchange history
836+
responses:
837+
'200':
838+
description: RI exchange history list
839+
content:
840+
application/json:
841+
schema:
842+
type: object
843+
'401':
844+
$ref: '#/components/responses/Unauthorized'
845+
'403':
846+
$ref: '#/components/responses/Forbidden'
847+
848+
/api/ri-exchange/approve/{id}:
849+
parameters:
850+
- $ref: '#/components/parameters/ResourceID'
851+
post:
852+
operationId: approveRIExchange
853+
tags: [RIExchange]
854+
summary: Approve a pending RI exchange (token-based, no session required)
855+
security: []
856+
responses:
857+
'200':
858+
description: Exchange approved
859+
content:
860+
application/json:
861+
schema:
862+
$ref: '#/components/schemas/StatusResponse'
863+
'400':
864+
$ref: '#/components/responses/BadRequest'
865+
'404':
866+
$ref: '#/components/responses/NotFound'
867+
868+
/api/ri-exchange/reject/{id}:
869+
parameters:
870+
- $ref: '#/components/parameters/ResourceID'
871+
post:
872+
operationId: rejectRIExchange
873+
tags: [RIExchange]
874+
summary: Reject a pending RI exchange (token-based, no session required)
875+
security: []
876+
responses:
877+
'200':
878+
description: Exchange rejected
879+
content:
880+
application/json:
881+
schema:
882+
$ref: '#/components/schemas/StatusResponse'
883+
'400':
884+
$ref: '#/components/responses/BadRequest'
885+
'404':
886+
$ref: '#/components/responses/NotFound'
887+
619888
# ---- History ------------------------------------------------------------
620889
/api/history:
621890
get:

‎internal/api/router_660_permission_flips_test.go‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,13 +264,15 @@ func TestDeletePlannedPurchase_PermissionGate(t *testing.T) {
264264
// ---- RI Exchange ----------------------------------------------------------
265265

266266
// TestExecuteExchange_PermissionGate covers POST /api/ri-exchange/execute.
267-
// The handler requires execute:purchases.
267+
// The handler requires execute:ri-exchange (not execute:purchases) because
268+
// RI exchanges are financially irreversible; the two permissions are
269+
// intentionally disjoint so granting one does not implicitly grant the other.
268270
func TestExecuteExchange_PermissionGate(t *testing.T) {
269271
ctx := context.Background()
270272
const userID = "55555555-5555-5555-5555-555555555555"
271273

272-
t.Run("user without execute:purchases is rejected with 403", func(t *testing.T) {
273-
mockAuth := authForUserWith(ctx, t, userID, "execute", "purchases", false)
274+
t.Run("user without execute:ri-exchange is rejected with 403", func(t *testing.T) {
275+
mockAuth := authForUserWith(ctx, t, userID, "execute", "ri-exchange", false)
274276
h := &Handler{auth: mockAuth}
275277
body := `{"ri_ids":["ri-abc"],"targets":[{"offering_id":"of-1"}],"max_payment_due_usd":"1000"}`
276278
_, err := h.executeExchange(ctx, reqWithBearerAndBody("user-token", body))
@@ -281,14 +283,33 @@ func TestExecuteExchange_PermissionGate(t *testing.T) {
281283
// the real AWS exchange client. We verify the permission gate passes by
282284
// checking the error is NOT a 403 when the permission is granted; the
283285
// AWS SDK call will fail with a non-403 (connection refused / 500).
284-
t.Run("user with execute:purchases clears 403 gate", func(t *testing.T) {
285-
mockAuth := authForUserWith(ctx, t, userID, "execute", "purchases", true)
286+
t.Run("user with execute:ri-exchange clears 403 gate", func(t *testing.T) {
287+
mockAuth := authForUserWith(ctx, t, userID, "execute", "ri-exchange", true)
286288
h := &Handler{auth: mockAuth}
287289
body := `{"ri_ids":["ri-abc"],"targets":[{"offering_id":"of-1"}],"max_payment_due_usd":"1000"}`
288290
_, err := h.executeExchange(ctx, reqWithBearerAndBody("user-token", body))
289291
// The error will be from the AWS SDK (not a 403), proving the gate passed.
290292
assertNotForbidden(t, err)
291293
})
294+
295+
// Isolation test: execute:purchases does NOT grant access to RI exchange.
296+
// This is the key invariant of the permission split: the two resources are
297+
// disjoint; holding one does not imply the other.
298+
t.Run("user with execute:purchases cannot execute RI exchange (403)", func(t *testing.T) {
299+
// The mock grants execute:purchases (true) but the handler asks for
300+
// execute:ri-exchange. HasPermissionAPI will be called with the
301+
// ri-exchange resource and must return false.
302+
mockAuth := new(MockAuthService)
303+
mockAuth.On("ValidateSession", ctx, "user-token").Return(userSessionFixture(userID), nil)
304+
// Handler asks for "execute":"ri-exchange"; this user does NOT have it.
305+
mockAuth.On("HasPermissionAPI", ctx, userID, "execute", "ri-exchange").Return(false, nil)
306+
t.Cleanup(func() { mockAuth.AssertExpectations(t) })
307+
308+
h := &Handler{auth: mockAuth}
309+
body := `{"ri_ids":["ri-abc"],"targets":[{"offering_id":"of-1"}],"max_payment_due_usd":"1000"}`
310+
_, err := h.executeExchange(ctx, reqWithBearerAndBody("user-token", body))
311+
assert403(t, err)
312+
})
292313
}
293314

294315
// TestUpdateRIExchangeConfig_PermissionGate covers PUT /api/ri-exchange/config.

0 commit comments

Comments
 (0)