feat(api/auth): finer-grained per-role write permissions across plans/purchases/RI-exchange #660

Activity

1. cristim

   added

   triagedItem has been triagedItem has been triaged

   priority/p2Backlog-worthyBacklog-worthy

   severity/mediumModerate harmModerate harm

   urgency/this-quarterWithin the quarterWithin the quarter

   impact/manyAffects most usersAffects most users

   effort/lWeeksWeeks

   type/featNew capabilityNew capability

   on May 22, 2026
2. cristim commented on May 25, 2026

   

   cristim

   on May 25, 2026

   MemberAuthor

   More actions

   Design for issue #660 — per-role write permissions

   Read-only investigation done against feat/multicloud-web-frontend (HEAD 5abf40bef). The good news: the permission model already exists in code and most handler-level verb checks are already in place. The work is mostly router gate flips plus a few targeted decisions and frontend visibility wiring, not a schema rebuild.

   1. Current model

   Schema (internal/auth/types.go):

   • User.Role is one of RoleAdmin, RoleUser, RoleReadOnly.
   • Group carries Permissions []Permission (JSON column on auth_groups in Postgres). Users get a union of their group permissions plus their role's defaults.
   • Permission{Action, Resource, Constraints} already supports per-account/provider/service/region scoping and MaxPurchaseAmount.
   • AuthContext.HasPermission(action, resource) (types.go:111) admin-shortcuts then walks the merged permission slice; ActionAdmin+ResourceAll is the wildcard.
   • Defaults:
     • DefaultAdminPermissions -> {admin, *}.
     • DefaultUserPermissions (types.go:389) already includes view recs/plans/purchases/history, create/update plans, plus cancel-own/retry-own/approve-own purchases.
     • DefaultReadOnlyPermissions -> view-only on recs, plans, history.

   Enforcement (two layers):

   • internal/api/router.go — Route.Auth levels (AuthAdmin / AuthUser / AuthPublic) checked in Router.Route (router.go:321). After PR fix(api): route Auth field becomes mandatory + readonly/user roles get access to read endpoints (P0) #364 this field is mandatory.
   • internal/api/handler.go:181 — requirePermission(ctx, req, action, resource) does the fine-grained verb check inside each handler, with admin shortcuts for both the admin API key and admin role.

   Coverage of requirePermission today (grepped, file:line):

   • Plans CRUD: handler_plans.go:20,66,95,123,174,195,364 (view/create/update/delete) — fully covered.
   • Plan-accounts join: handler_accounts.go:1216,1255 (update/view plans) — covered.
   • Purchases planned ops: handler_purchases.go:88,186,207,228,254,1031,1075 (view/update/execute/delete purchases) — covered.
   • RI-exchange: handler_ri_exchange.go:115,198,239,260,368,473,534,683,704,738 — covered (view purchases for reads, execute purchases for execute, view/update config for the config endpoint).
   • Accounts CUD: handler_accounts.go:91,172,248,469,488,533,606,697,956,985,1117 — covered (view/create/update/delete accounts).
   • Users CUD: handler_users.go:17,31,100,119,143 — covered.
   • Groups CUD: handler_groups.go:16,30,65,84,108 — covered.
   • Config writes: handler_config.go:80,203 — covered.
   • Recommendations refresh: handler_recommendations_refresh.go:57.
   • Federation IaC: handler_federation.go:128.
   • Inventory: handler_inventory.go:29.
   • Analytics reads: handler_analytics.go:34,87; analytics POST collect uses requireAdmin (handler_analytics.go:151).
   • Discover-org: handler_accounts.go:1303 uses requireAdmin.
   • Registrations approve/reject: handler_registrations.go:204 uses requireAdmin via setReviewMetadata.

   The chokepoint. PR #364 left every mutating route at Auth: AuthAdmin (router.go:117,119,145,147,149,151,152,153,156,177,178,179,180,192,220-224,230,231,233-235,237-238,245,247,248,265,266,268,277-282,233). Router.Route calls requireAdmin first, so even when the handler's requirePermission would let a non-admin user with update:plans through, the router rejects with admin required before the handler runs. The follow-up work is mostly flipping these route guards from AuthAdmin to AuthUser (so the handler's verb check becomes authoritative).

   2. Proposed schema changes

   None. The Permission model, the Group.Permissions JSON column, and the requirePermission plumbing all exist. No DB migration needed.

   Two small additions to default sets in types.go:

   • RoleUser already gets create/update plans. Add delete plans if we want the user role to delete plans (PR fix(api): route Auth field becomes mandatory + readonly/user roles get access to read endpoints (P0) #364's "user = read + write plans" wording reads as full plan CRUD). Or keep delete admin-only by omitting it from DefaultUserPermissions; non-admin users would then see plan-write but not plan-delete after the router flip.
   • For account writes, update/delete accounts are not in any default role today. The default policy stays admin-only; custom groups can grant {update, accounts} to delegated operators without changing roles.

   Open verb-design questions (call out for product):

   • Plan delete for user role: yes or admin-only? (My read: yes — symmetric with create/update.)
   • Planned-purchase ops (pause, resume, run, delete): the handlers already gate on update/execute/delete purchases. DefaultUserPermissions does NOT include these. Either (a) add them to user defaults so the "user" role can manage its own planned purchases, or (b) keep them admin-only and require explicit group grant. Recommend (a) for update purchases (pause/resume) and (b) for execute/delete purchases (those are higher-blast-radius).
   • RI-exchange quote/execute/config writes: keep admin-only by default; expose via group grants only. Same argument as for execute purchases.
   • Recommendations refresh: already AuthUser+handler-gated. No change.

   3. Decisions on admin-management endpoints (scope item 2 in issue body)

   • /api/users writes — stay AuthAdmin. User management is org-admin territory; no roadmap need for non-admin user CRUD.
   • /api/groups writes — stay AuthAdmin. Editing groups means editing the permission system itself.
   • /api/registrations writes — stay AuthAdmin. Approving a federation registration grants access to new accounts.
   • /api/accounts create/credentials/test — stay AuthAdmin by default; allow custom groups to grant create/update accounts to delegated operators (no router flip; the handler's verb gate already supports this once the route is AuthUser). If we don't want non-admin operators ever editing accounts, keep AuthAdmin on the route — the handler verb check then never fires for non-admins, which is fine.
   • /api/config PUT — stay AuthAdmin for global config. The RI-exchange config PUT (which already uses update:config) can flip to AuthUser since the handler enforces.
   • /api/analytics/collect — admin-only; no change. Bulk collection is operational.

   4. Audit task (scope item 3)

   AuthUser routes that read account-scoped data already call requirePermission and then resolve getAllowedAccounts + scope the query — confirmed in handler_plans.go, handler_purchases.go, handler_analytics.go, handler_federation.go. The flips proposed below only enable mutations on rows the user is already authorized to see, so the scoping invariant is preserved. The audit is a focused per-handler check (mainly: setPlanAccounts, pausePlannedPurchase, resumePlannedPurchase, deletePlannedPurchase, executePurchase, updateRIExchangeConfig) confirming each calls getAllowedAccounts(session) and rejects out-of-scope IDs before writing.

   5. Effort + recommended PR breakdown

   Estimate: medium-to-large. Three PRs. Not a single small PR.

   PR-A — router flips + verb defaults (medium). No migration, no frontend change.

   • internal/auth/types.go: extend DefaultUserPermissions with delete:plans, update:purchases (planned-purchase pause/resume), and (if we choose to) leave execute:purchases/delete:purchases out so non-admin users can't fire purchases or wipe planned ones without an explicit group grant.
   • internal/api/router.go: flip the following from AuthAdmin to AuthUser (handler-level requirePermission then enforces):
     • Plans: POST/PUT/PATCH/DELETE /api/plans and /api/plans/{id}, plus POST /api/plans/{id}/purchases and PUT /api/plans/{id}/accounts.
     • Planned purchases: POST pause/resume/run, DELETE /api/purchases/planned/{id}.
     • Purchases: POST /api/purchases/execute.
     • RI-exchange: POST quote/execute, PUT /api/ri-exchange/config.
   • Keep AuthAdmin on: /api/config PUT, all /api/users, /api/groups, /api/registrations, /api/accounts writes, /api/accounts/discover-org, /api/analytics/collect.
   • Tests: extend internal/api/router_authuser_test.go + router_auth_test.go with role-permutation cases (user vs readonly vs admin -> 200/403 on each flipped route). Audit-scoping unit tests already exist per handler.

   PR-B — frontend visibility (medium). Once non-admin users can hit write endpoints, the UI needs to surface the right buttons.

   • Today the FE often hides write controls based on user.role === 'admin' (see issue Surface inline Cancel button to non-admin operator roles with cancel-any (frontend permission visibility) #158 for the exact pattern — frontend/src/history.ts:303 and similar). After PR-A, those hides are wrong.
   • Either ship /api/auth/me/permissions (small endpoint returning the merged permission list — handler reuse trivial) and gate buttons on hasPermission(action, resource), or expand UserInfo in LoginResponse//api/auth/me to include permissions: Permission[]. The latter avoids an extra round trip; payload is small.
   • Sweep frontend/src/: plans page (create/edit/delete buttons), planned-purchases (pause/resume/run/delete), history (already partially scoped via cancel-own/retry-own/approve-own — issue Surface inline Cancel button to non-admin operator roles with cancel-any (frontend permission visibility) #158 is the open thread here), RI-exchange (quote/execute/config).
   • This is the bulk of the user-visible change. Should be its own PR for review focus.

   PR-C — admin UI for granular grants (large, optional). Add a "Permissions" tab to the existing group editor so admins can compose {action, resource, constraints} rows without hand-editing JSON. Skip if the only consumers are admins willing to use the API directly; revisit when a real operator persona shows up. This is the only piece that needs significant new UI.

   6. Related work to fold in

   • feat(api,recs): permission-gated direct purchase execute (execute-{any,own}) bypassing approval email, with cost warning + confirm gate #289 (execute-{any,own} verbs) overlaps with the execute:purchases decision in PR-A — keep them coordinated; the granular execute-own verb is the right long-term shape for delegated buyers and should land as part of PR-A or right after.
   • Surface inline Cancel button to non-admin operator roles with cancel-any (frontend permission visibility) #158 (cancel-any button surfacing) is a strict subset of PR-B's frontend visibility sweep — close-or-merge depending on which lands first.

   Recommendation

   Land PR-A and PR-B in that order; defer PR-C until there's a concrete operator persona. PR-A alone (router flips + default-permission tweaks) is what unblocks the "user role can actually manage plans" goal. PR-B turns it into a clean UX.

   Stopping here per the issue's "design comment first" instruction; no code changes pushed.
3. cristim

   added 2 commits that reference this issue on May 25, 2026

   test(frontend/perms): update permission test fixtures for new user de…

   ...

   a47247e

   feat(api/auth): flip mutating-route gate to handler-level requirePerm…

   ...

   4c62718
4. cristim commented on May 28, 2026

   

   cristim

   on May 28, 2026

   MemberAuthor

   More actions

   Verification: cannot-verify-autopilot - Partially shipped. PR #726 (merged 2026-05-27) is PR-A: handler-level requirePermission gates wired into mutating routes. PR body explicitly defers PR-B (frontend visibility of role-gated actions) and PR-C (optional admin grant UI). The issue's full acceptance criteria (finer-grained per-role write permissions across plans/purchases/RI-exchange visible and configurable) is not complete.
5. cristim

   added 4 commits that reference this issue on Jun 1, 2026

   feat(api/auth): split execute:ri-exchange from execute:purchases (closes

   ...

   This issue will close once commit 608087a is merged into the 'main' branch.

   608087a

   feat(api/auth): split execute:ri-exchange from execute:purchases (closes

   ...

   This issue will close once commit 65caed4 is merged into the 'main' branch.

   65caed4

   feat(api/auth): split execute:ri-exchange from execute:purchases (closes

   ...

   This issue will close once commit eb85a50 is merged into the 'main' branch.

   eb85a50

   fix(purchases): gate scheduled-purchase management on creator-scope o…

   ...

   This issue will close once commit cb1abb7 is merged into the 'main' branch.

   cb1abb7
6. cristim

   closed this as completedin eb85a50on Jun 9, 2026