fix(api): jwtProcedure accepts x-api-key sessions, not just Bearer#3895
Conversation
Better-auth's apiKey plugin (`enableSessionForAPIKeys: true`) populates ctx.session for x-api-key requests, but jwtProcedure was throwing on the very first line if the request didn't carry an Authorization Bearer header — so any procedure marked jwtProcedure rejected api-key auth before reaching the session fallback. The CLI's `superset hosts`, `workspaces.list`, etc. all 401'd with `--api-key sk_live_…`. Accept any of: a verifiable Bearer JWT, a successful Bearer JWT fallback, or an x-api-key-derived session. Throw only if none of those produce identity. The TRPCError re-throw on explicit JWT rejection is preserved. Trailing error message updated to reflect the broader contract.
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Greptile SummaryThis PR fixes Confidence Score: 5/5Safe to merge — the fix is minimal, targeted, and the security invariants (TRPCError re-throw for revoked/forged tokens) are preserved. Only one file changed with a simple structural refactor. The JWT verification path and its error-handling guard are unchanged; the only behavioral delta is removing the upfront throw that blocked the session branch for non-Bearer callers. No new attack surface is introduced. No files require special attention.
|
| Filename | Overview |
|---|---|
| packages/trpc/src/trpc.ts | Refactors jwtProcedure to extract the Bearer token conditionally rather than gating the entire middleware on its presence, enabling x-api-key session fallback; logic is correct and the TRPCError re-throw guard is preserved. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Request] --> B{Authorization: Bearer header?}
B -- Yes --> C[Extract bearer token]
B -- No --> F
C --> D{verifyJWT succeeds\nwith payload.sub?}
D -- Yes --> E[✅ Authenticated via JWT\nReturn next with JWT ctx]
D -- No: TRPCError --> G[❌ Re-throw TRPCError\nrevoked / forged token]
D -- No: other error or\nno payload.sub --> F{ctx.session present?\ncookie OR x-api-key}
F -- Yes --> H[✅ Authenticated via Session\nLookup org memberships from DB]
F -- No --> I[❌ UNAUTHORIZED\nNot authenticated]
Reviews (1): Last reviewed commit: "fix(api): jwtProcedure accepts x-api-key..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/trpc/src/trpc.ts`:
- Line 75: The current extraction of the token into the variable bearer uses a
case-sensitive startsWith check on authHeader and thus rejects valid schemes
like "bearer ". Update the logic in the area that sets bearer (the authHeader
handling) to perform a case-insensitive check for the "bearer " scheme—e.g.,
compare authHeader.toLowerCase().startsWith("bearer ") or use a case-insensitive
regex—and then slice the original authHeader after the scheme length to extract
the token so casing is preserved in the token value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| message: "Bearer token required", | ||
| }); | ||
| } | ||
| const bearer = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : null; |
There was a problem hiding this comment.
Parse bearer scheme case-insensitively to avoid rejecting valid headers.
Authorization auth-scheme tokens are case-insensitive. The current check only accepts exact "Bearer " casing, so valid variants (for example bearer <token>) won’t be parsed.
Suggested patch
- const bearer = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : null;
+ const bearerMatch = authHeader?.match(/^Bearer\s+(.+)$/i);
+ const bearer = bearerMatch?.[1]?.trim() ?? null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const bearer = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : null; | |
| const bearerMatch = authHeader?.match(/^Bearer\s+(.+)$/i); | |
| const bearer = bearerMatch?.[1]?.trim() ?? null; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/trpc/src/trpc.ts` at line 75, The current extraction of the token
into the variable bearer uses a case-sensitive startsWith check on authHeader
and thus rejects valid schemes like "bearer ". Update the logic in the area that
sets bearer (the authHeader handling) to perform a case-insensitive check for
the "bearer " scheme—e.g., compare authHeader.toLowerCase().startsWith("bearer
") or use a case-insensitive regex—and then slice the original authHeader after
the scheme length to extract the token so casing is preserved in the token
value.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
2 participants
Summary
jwtProcedurewas rejecting any request without anAuthorization: Bearer …header before reaching its session fallback. Better-auth's apiKey plugin (enableSessionForAPIKeys: true) populatesctx.sessionforx-api-keyrequests, so they're session-equivalent — but they couldn't reach that branch because the upfront throw fired first.Concretely, the CLI's `--api-key sk_live_…` flag round-tripped through better-auth (returns 200 against `protectedProcedure` like `user.me`), but every `jwtProcedure`-gated procedure (`host.list`, `v2-workspace.list`, etc.) returned 401. `superset hosts list` and `superset workspaces list` were unusable with API keys.
This PR rewrites the procedure to accept any of:
Bot-fix from #3889 preserved: TRPCError instances from `verifyJWT` (revoked/forged tokens) still re-throw rather than silently falling through.
Test plan
Summary by cubic
Allow
jwtProcedureto authenticate with a Bearer JWT or any live session (cookie orx-api-key), fixing CLI API key flows that were returning 401. CLI commands likesuperset hosts listandsuperset workspaces listnow work with--api-keyorSUPERSET_API_KEY.ctx.session(includesx-api-keysessions frombetter-auth).TRPCErrorfromverifyJWTfor explicit JWT rejection.Written for commit e19be15. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Release Notes