fix(api): return session authz denials when no email token is present

[cristim]

What

fallsThroughToToken (approve, cancel, email revoke) and the sessionHasApproveRight pre-check in approveRIExchange now hand a session 403 to the email-token branch only when a token is present. Without a token the denial is returned.

Why

Issue #173: with token == "" a session denial fell through to the session path again (approvePurchaseViaSession(...) / cancelPurchaseViaSession(...) / approveRIExchangeViaSession(..., nil)). That path re-resolves the caller through requireSession, which reads the context principal first. So the outer denial was thrown away, and safety depended on both lookups resolving the same principal. The email revoke path returned a 401 "sign in" to a signed-in user instead of the 403.

Unchanged: with a token, a logged-in user without approve-/cancel- still reaches the contact_email token flow (owner's design, PR #101). The account-scope 404 with a token is unchanged too, and so are CSRF and constraint denials, which were already terminal.

Tests

New internal/api/session_denial_final_test.go. The bearer session is denied, CSRF is valid, and the request context carries a different, authorized principal. This is the divergence the issue describes, so the test checks "outer denies, so the mutation never runs" directly.

• approve, cancel, email revoke, RI exchange approve, each without a token: expects a 403 and no ApproveAndExecute / CancelExecutionAtomic / TransitionRIExchangeStatus call.
• RI exchange approve with a valid token and a denied session: still reaches the token branch.

On origin/main code (fix stashed), all four no-token tests fail:

approve: expected: 403 actual: 409 ... could not be approved: mutation reached; Expected "ApproveAndExecute" to not have been called
cancel:  expected a ClientError, got: cancel execution ...: mutation reached
revoke:  expected: 403 actual: 401 ... sign in or use the revocation link from the notification email
RI:      expected a ClientError, got: failed to transition exchange status: mutation reached

Mutation check: reverting only the RI line (case isPermissionDenied(err):) fails TestApproveRIExchange_SessionDenialWithoutTokenIsFinal.

Four existing tests passed only because of the fall-through. Each is updated:

• TestHandler_cancelPurchase_Session_RejectsTerminalStatus and ..._RejectsEachNonCancelableStatus used a session with no cancel permission (their own comment says "admin session"). They reached the status guard only through the fall-through. They now grant cancel-any.
• TestApproveViaSession_RequiresCSRF used a principal that RBAC denied (the issue points to this one). It now uses grantAdminPurchaser plus a non-zero cost, so it still reaches the CSRF guard.
• TestHandler_revokePurchase_SessionNoPermissionNoToken now expects 403 instead of 401.

Commands run, all with GOTOOLCHAIN=go1.26.6 GOWORK=off GOFLAGS='-p=2 -count=1':

• go build ./...
• go vet and go vet -tags integration on internal/api, internal/purchase and internal/server
• go test on internal/api, internal/purchase and internal/server: pass
• golangci-lint run --new-from-rev=origin/main ./internal/api/...: no issues

Frontend: the dashboard approve/cancel calls already got a 403 on denial. The SPA calls /purchases/{id}/revoke, not the email revoke endpoint, so nothing in the frontend relied on the old 401.

Deliberately left

• The account-scope gap in authorizeSessionApproveRIExchange, sec(money): currency is dropped at every layer boundary, so USD-denominated caps are compared against non-USD amounts [platform part] #361 and the pre-auth status 409 ordering (cli#1749) are separate issues.

Closes #173

Summary by CodeRabbit

• Bug Fixes
  • Session permission denials for purchase approval, cancellation, revocation, and exchange approval now return a 403 when no token is provided, rather than proceeding to token-based authorization.
  • Token-based approval remains available when a valid token is provided.
• Tests
  • Added coverage for authorization outcomes and verified that denied requests do not perform changes.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 8c992571-32e5-4a22-ba5c-5fad16a80f6a

📥 Commits

Reviewing files that changed from the base of the PR and between 87b8ce8 and 9a6038c.

📒 Files selected for processing (5)

• internal/api/handler_purchases.go
• internal/api/handler_purchases_test.go
• internal/api/handler_ri_exchange.go
• internal/api/middleware_test.go
• internal/api/session_denial_final_test.go

Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

---

📝 Walkthrough

Walkthrough

Purchase and RI-exchange handlers now return session permission denials when no token is present. When a token is present, eligible denials can proceed to token handling. Tests cover these paths and related cancellation-status checks.

Changes

Authorization dispatch

Layer / File(s)	Summary
Gate token fallback on token presence internal/api/handler_purchases.go, internal/api/handler_ri_exchange.go	Purchase and RI-exchange dispatch paths only fall through to token handling for eligible session denials when a token is present.
Verify denial and token paths internal/api/session_denial_final_test.go, internal/api/handler_purchases_test.go, internal/api/middleware_test.go	Tests assert 403 responses for session denials without a token and verify that mutation calls are not reached. RI-exchange tests also verify the valid-token path. Cancellation-status tests grant permission so they exercise status checks.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant approvePurchase
  participant authorizeSessionApprove
  participant approveViaToken
  Client->>approvePurchase: Send request with session and optional token
  approvePurchase->>authorizeSessionApprove: Check session permissions
  authorizeSessionApprove-->>approvePurchase: Return permission denial
  alt Token absent
    approvePurchase-->>Client: Return session denial
  else Token present
    approvePurchase->>approveViaToken: Check eligible token fallback
    approveViaToken-->>Client: Return token-path result
  end

Loading

Merge Risk: ⚪ Minimal · up to 9a603

The supplied evidence identifies no outstanding issue that would prevent merging after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: return session authorization denials when no email token is present.
Linked Issues check	✅ Passed	[ #173 ] The approval and cancellation dispatches return session denials when no token is present. They retain the token branch when a token is present. The new tests use valid CSRF tokens and a diffe…
Out of Scope Changes check	✅ Passed	No unrelated changes are evident. The email-revoke change applies the same session-denial versus token-fallback rule. The test-fixture updates and added tests support the denial behavior and preserve …

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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