perf: skip the proxy session-refresh getUser for API routes (round 2, phase 3 — auth)#481
Conversation
Round-2 phase 3, auth part (isolated for security review). Every authenticated request previously paid two serial auth-server round trips: src/proxy.ts ran supabase.auth.getUser() for every matched path including /api/*, and the route handler then independently validated the caller via getOptionalAuthenticatedUser. Nothing consumed the proxy's result on API routes — the call existed only to keep session cookies fresh. The proxy now skips only the session-refresh getUser for /api/* paths: - API handlers keep validating the caller themselves (bearer token and/or @supabase/ssr cookie) — no authorization decision moves or weakens, and the handlers remain fail-closed exactly as before. - The CSP nonce header still applies to every matched response including /api/* (matcher unchanged). - Session cookies still refresh on every page navigation, and the browser Supabase client refreshes its own token (autoRefreshToken) for bearer-based API calls, so long-lived sessions behave as before. Deliberately NOT done: the trusted-internal-header design (proxy forwards a validated user id for handlers to trust) was rejected — handlers validating tokens independently is the stronger defense-in-depth posture. The legacy cookie-token extraction in supabase/auth.ts was left untouched: tests define legacy sb-access-token / plain-JSON cookie support as intended behavior. New tests pin the contract: getUser is skipped for /api/* (CSP still stamped), runs on page navigations with an sb- cookie, and never runs without one. Existing CSP nonce tests unchanged and green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82376e73f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a21a2a9690
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Security-relevant change — please review with the tenancy defense-in-depth lens (
docs/tenancy-defense-in-depth-review.md). Isolated in its own PR for that reason (stacked on #480).Every authenticated request paid two serial auth-server (GoTrue) round trips:
src/proxy.tsransupabase.auth.getUser()for every matched path including/api/*, then the route handler independently validated the caller viagetOptionalAuthenticatedUser. On API routes nothing consumed the proxy's result — it existed only to refresh session cookies.The proxy now skips only the session-refresh
getUserfor/api/*paths.Why this does not weaken auth
@supabase/ssrcookie), fail-closed, exactly as before. The proxy'sgetUserresult was never consulted for access on API routes./api/*; every response still carries the per-request nonce CSP.setAllis a no-op — so API-only traffic never persisted refreshed cookies even before this change.)Rejected alternative
A trusted-internal-header design (proxy validates once, forwards the user id, handlers trust it) was considered and rejected: independent handler validation is the stronger defense-in-depth posture, and header-trust introduces spoofing surface.
Explicitly untouched
The legacy cookie-token extraction in
supabase/auth.ts(sb-access-token, plain-JSON auth cookies) was flagged as possibly dead during exploration, butprivate-access-routestests define it as supported behavior (including stale-bearer → valid-cookie recovery), so it stays.Impact
Removes one serial GoTrue network round trip from every authenticated API request — answers, typeahead, documents, registry. This is the highest-frequency single win in the round; it also reduces pressure on the documented auth connection-cap failure mode (
docs/capacity-review.md).Verification
tests/proxy-session-refresh.test.tspins the contract:getUserskipped for/api/*(CSP still stamped), called once on page navigations with ansb-cookie, never called without onepublic-access-deep+private-access-routessuites green (106 tests)tsc,eslint,prettierclean🤖 Generated with Claude Code