Skip to content

fix(api): authorize API-key clients on delete/cancel purchase permission check - #1491

Merged
cristim merged 1 commit into
mainfrom
fix/1421-apikey-delete-cancel-auth
Jul 22, 2026
Merged

cristim merged 1 commit into
mainfrom
fix/1421-apikey-delete-cancel-auth

Conversation

@cristim

@cristim cristim commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

requireDeleteOrCancelPurchasePermission (gating DELETE /api/purchases/planned/{id}) hand-rolled its own auth dispatch instead of using the shared requirePermission path, and in doing so dropped the user-API-key branch entirely. A scoped API-key automation client presenting only an x-api-key header (no bearer token) got a 401 no authorization token provided on delete/cancel, even when the key held delete:purchases.

This is a follow-up to #1421, which introduced the hand-rolled gate while adding cancel-own support.

Fix

  • Added requireAnyPermission / authorizeAPIKeyAny as multi-verb counterparts to the existing requirePermission / authorizeAPIKey dispatch chain (principal-in-context -> admin API key -> user API key -> bearer-token session), so a multi-verb gate can't drift from the single-verb dispatch order every other endpoint relies on.
  • requireDeleteOrCancelPurchasePermission now delegates to requireAnyPermission instead of hand-rolling dispatch.
  • Widened the verb set to include update-any so a custom group holding only that permission clears this entry gate before the ownership check runs (documented in the code comment; doesn't widen the effective grant since update-any already passes the later ownership-scoped cancel check).
  • Both authorizeAPIKey and authorizeAPIKeyAny fail closed: an incomplete identity after a granted check is a hard error, and a denied-but-valid key returns 403 rather than silently falling through to bearer-token auth.

Test plan

  • go build ./... — clean
  • go vet ./... — clean
  • go test ./... (full repo) — all packages pass, 0 failures
  • Regression test TestHandler_deletePlannedPurchase_UserAPIKey_Allowed verified to fail pre-fix (401 no authorization token provided, reproducing the reported bug) and pass post-fix
  • Updated pre-existing sibling test TestDeletePlannedPurchase_PermissionGate whose mock set predated the update-any verb addition
  • golangci-lint (CI-pinned v2.10.1) on internal/api/...: 2 pre-existing findings, both outside this diff's changed lines (unrelated debt at handler_purchases.go:1832 and :2545)
  • gocyclo -over 10 on changed files: clean

…ion check

requireDeleteOrCancelPurchasePermission hand-rolled its own auth
dispatch (admin API key -> session) and dropped the user-API-key
branch that every other permission gate relies on via
requirePermission/authorizeAPIKey. Scoped API-key automation clients
presenting only an x-api-key header got a 401 "no authorization token
provided" instead of being authorized against their key's granted
verbs, even when the key held delete:purchases.

Add requireAnyPermission/authorizeAPIKeyAny as the multi-verb
counterparts to the existing requirePermission/authorizeAPIKey
dispatch chain, and delegate requireDeleteOrCancelPurchasePermission to
it so the delete/cancel gate cannot drift from the single-verb
dispatch order used everywhere else. Also widen the verb set to
include update-any so a custom group holding only that permission
clears this entry gate before ownership is checked.

Follow-up to #1421, which introduced the hand-rolled gate.

Also updates a pre-existing sibling test
(TestDeletePlannedPurchase_PermissionGate) whose mock set predates the
update-any verb addition.
@cristim cristim added triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm urgency/this-sprint Within the current sprint impact/many Affects most users type/bug Defect labels Jul 22, 2026
@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 73136e61-84d8-4bf7-851e-69d6443cc662

📥 Commits

Reviewing files that changed from the base of the PR and between f8cdea6 and 3766119.

⛔ Files ignored due to path filters (1)
  • go.work.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • internal/api/handler.go
  • internal/api/handler_purchases.go
  • internal/api/handler_purchases_test.go
  • internal/api/router_660_permission_flips_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1421-apikey-delete-cancel-auth

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

@cristim

cristim commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

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 0ae6c6a into main Jul 22, 2026
19 checks passed
@cristim
cristim deleted the fix/1421-apikey-delete-cancel-auth branch July 22, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact/many Affects most users priority/p1 Next up; this sprint severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant