Skip to content

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

Merged
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
fix/660-wave22
Jun 3, 2026
Merged

feat(api/auth): split write into per-resource permissions (closes #660)#891
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
fix/660-wave22

Conversation

@cristim
Copy link
Copy Markdown
Member

@cristim cristim commented May 30, 2026

Summary

  • Adds ResourceRIExchange = "ri-exchange" constant to separate RI exchange permissions from purchase permissions
  • Updates executeExchange handler to require execute:ri-exchange instead of execute:purchases — RI exchanges are financially irreversible once submitted to AWS; disjoint permissions prevent accidental elevation
  • Adds /api/ri-exchange/* routes to openapi.yaml with 403 responses and explicit permission documentation on the execute endpoint
  • Extends frontend Resource closed-union type with 'ri-exchange' and tests that execute:ri-exchange is admin-only by default

Backward compatibility

No breaking change. The PR-A commits (already on this branch) flipped mutating-route router gates from AuthAdmin to AuthUser + handler-level requirePermission. Existing admin users retain full access via the admin:* wildcard. Non-admin users who previously couldn't reach these routes now hit the handler-level gate — no access is loosened.

execute:ri-exchange has no default user-role grant — same as before (RI exchange execute was admin-only before this PR). Non-admin roles must be explicitly granted execute:ri-exchange via a custom group.

No DB migration needed

Role defaults are computed at runtime from Go constants (DefaultAdminPermissions / DefaultUserPermissions / DefaultReadOnlyPermissions). No DB table stores the permission set; no migration is needed.

Coordination with PR #884

PR #884 (fix/476) adds 403 responses to plans/purchases routes in openapi.yaml. This PR adds a new # ---- RI Exchange ---- section that #884 does not touch — no conflict.

Test plan

  • go test ./internal/auth/... — 497 passed
  • go test ./internal/api/... — 1302 passed, including:
    • TestExecuteExchange_PermissionGate — asserts execute:ri-exchange required, not execute:purchases
    • Isolation sub-test: user with execute:purchases is rejected (403) for RI exchange
    • TestRouter_MutatingRoutes_RequireAuth — unauthenticated callers get 401 for /api/ri-exchange/execute
  • npm test (frontend) — 1989 passed, including:
    • Admin sees execute:ri-exchange (admin:* short-circuit)
    • User role is denied execute:ri-exchange (no default grant)

@cristim cristim added triaged Item has been triaged priority/p2 Backlog-worthy urgency/this-quarter Within the quarter impact/many Affects most users effort/l Weeks type/feat New capability labels May 30, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

Warning

Rate limit exceeded

@cristim has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 53 minutes and 8 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 956adbb1-d91e-4f6c-818c-e2eea9619345

📥 Commits

Reviewing files that changed from the base of the PR and between 6943207 and 65caed4.

📒 Files selected for processing (6)
  • frontend/src/__tests__/permissions.test.ts
  • frontend/src/permissions.ts
  • internal/api/handler_ri_exchange.go
  • internal/api/openapi.yaml
  • internal/api/router_660_permission_flips_test.go
  • internal/auth/types.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/660-wave22

Comment @coderabbitai help to get the list of available commands and usage tips.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented May 30, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 1, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 1, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 1, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

 #660)

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.
@cristim
Copy link
Copy Markdown
Member Author

cristim commented Jun 3, 2026

Rebased on feat/multicloud-web-frontend (commit 65caed41d). Conflict in frontend/src/__tests__/permissions.test.ts was resolved by re-expressing the user-default-deny invariant in the new effectivePermissions model introduced by #922 and #912.

Permission-bypass review (per the issue brief):

  • The handler flip is scoped to a single route: internal/api/handler_ri_exchange.go::executeExchange now requires execute:ri-exchange. All other execute:purchases checks in handler_purchases.go (lines 241, 1139) correctly stay on the regular-purchases permission; no coarse-check path was left behind for the RI-exchange flow.
  • Backend tests in internal/api/router_660_permission_flips_test.go cover:
    • deny by default: user without execute:ri-exchange gets 403,
    • allow when granted: user with execute:ri-exchange clears the 403 gate,
    • isolation: a user holding execute:purchases but NOT execute:ri-exchange still gets 403 on the exchange handler (the key disjointness invariant).
  • Frontend tests in frontend/src/__tests__/permissions.test.ts mirror the same shape against canAccess() in the new effective-permissions model:
    • admin via group fallback: canAccess('execute', 'ri-exchange') is true,
    • explicit grant of execute:purchases does NOT imply execute:ri-exchange (new test),
    • explicit grant of execute:ri-exchange does NOT imply execute:purchases (new test, inverse).

Em-dashes scrubbed from the six PR-introduced occurrences across the touched files (style-only fix that landed alongside the conflict resolution).

Tests passing locally:

  • go test ./internal/api/... -count=1: 1395/1395 pass.
  • go test ./internal/auth/... -count=1: 508/508 pass.
  • go vet ./... && go build ./...: clean.
  • Frontend permissions Jest suite: 75/75 pass.
  • npx tsc --noEmit: clean.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

@cristim: Acknowledged — the rebase onto feat/multicloud-web-frontend, the conflict resolution in permissions.test.ts against the new effectivePermissions model, and the disjointness invariant tests (both backend and frontend) all look well-documented. Triggering an incremental review now against the updated branch.


🧠 Learnings used
Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim cristim merged commit eb85a50 into feat/multicloud-web-frontend Jun 3, 2026
5 checks passed
@cristim cristim deleted the fix/660-wave22 branch June 3, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/l Weeks impact/many Affects most users priority/p2 Backlog-worthy triaged Item has been triaged type/feat New capability urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant