🛡️ fix: Add Origin Binding to Admin OAuth Exchange Codes#12469
Conversation
Bind admin OAuth exchange codes to the admin panel's origin at generation time and validate the origin on redemption. This prevents an intercepted code (via referrer leakage, logs, or network capture) from being redeemed by a different origin within the 30-second TTL. - Store the admin panel origin alongside the exchange code in cache - Extract the request origin (from Origin/Referer headers) on the exchange endpoint and pass it for validation - Reject code redemption when the request origin does not match the stored origin (code is still consumed to prevent replay) - Backward compatible: codes without a stored origin are accepted
There was a problem hiding this comment.
Pull request overview
This PR strengthens the admin OAuth exchange-code flow by binding exchange codes to the admin panel’s origin and rejecting redemptions from mismatched origins, reducing the risk of leaked codes being redeemed by unintended clients.
Changes:
- Store an optional
originwith admin exchange codes at generation time. - Resolve
Origin/Refereron/api/admin/oauth/exchangeand pass it into the exchange validator. - Add Jest unit tests for origin match/mismatch, backward compatibility, and one-time-use behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/api/src/auth/exchange.ts | Adds optional origin storage and origin-mismatch rejection during code redemption. |
| packages/api/src/auth/exchange.spec.ts | Introduces unit tests for origin binding and one-time-use semantics. |
| api/server/routes/admin/auth.js | Resolves request origin from headers and supplies it to the exchange function. |
| api/server/controllers/auth/oauth.js | Stores the admin panel origin when generating exchange codes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 866cf11224
ℹ️ 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".
Add a PKCE-like code_challenge/code_verifier flow to the admin OAuth exchange so that intercepting the exchange code alone is insufficient to redeem it. The admin panel generates a code_verifier (stored in its HttpOnly session cookie) and sends sha256(verifier) as code_challenge through the OAuth initiation URL. LibreChat stores the challenge keyed by OAuth state and attaches it to the exchange code. On redemption, the admin panel sends the verifier and LibreChat verifies the hash match. - Add verifyCodeChallenge() helper using SHA-256 - Store code_challenge in ADMIN_OAUTH_EXCHANGE cache (pkce: prefix, 5min TTL) - Capture OAuth state in callback middleware before passport processes it - Accept code_verifier in exchange endpoint body - Backward compatible: no challenge stored → PKCE check skipped
Addressing Review CommentsCodex P1: "Bind exchange codes to an unforgeable client signal"Addressed in commit 98cd54f. The exchange flow now uses a PKCE-like
The Copilot: Normalize Origin header in
|
- Await cache.set for PKCE challenge storage with error handling - Use crypto.timingSafeEqual for PKCE hash comparison - Drop case-insensitive flag from hex validation regexes - Add code_verifier length validation (max 512 chars) - Normalize Origin header via URL parsing in resolveRequestOrigin - Add test for undefined requestOrigin rejection - Clarify JSDoc: hex-encoded SHA-256, not RFC 7636 S256
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a67dcc944
ℹ️ 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".
…/buffer handling - Callback middleware now redirects to error URL on cache.get failure instead of silently continuing without PKCE challenge - resolveRequestOrigin returns undefined (not raw header) on parse failure - Remove dead try/catch around Buffer.from which never throws for string input
|
@codex review |
…validate verifier length, deduplicate URL parse - Log warning when OAuth state is present but no PKCE challenge found - Add minimum length check (>= 1) on code_verifier input validation - Update POST /oauth/exchange JSDoc to document code_verifier param - Deduplicate new URL(redirectUri) parse in createOAuthHandler - Restore intent comment on pre-delete pattern in exchangeAdminCode
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2bed758c5
ℹ️ 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".
| const challenge = await cache.get(`pkce:${req.oauthState}`); | ||
| if (challenge) { | ||
| req.pkceChallenge = challenge; |
There was a problem hiding this comment.
If a login starts with code_challenge but the pkce:${state} entry is missing at callback time (e.g., user takes longer than the 5-minute TTL, cache eviction, or transient cache inconsistency), this middleware silently continues without req.pkceChallenge. The downstream code then generates an exchange code with no stored challenge, and /oauth/exchange accepts it without code_verifier, causing a silent downgrade from PKCE-protected flow back to bearer-code behavior.
Useful? React with 👍 / 👎.
- Use real Keyv in-memory store instead of hand-rolled Map mock - Replace jest.fn mocks with jest.spyOn on real Keyv instance - Remove redundant store.has() assertion, use cache.get() instead - Eliminate all eslint-disable and as-any suppressions - User fixture no longer needs any cast (Keyv accepts plain objects)
…#12469) * fix(auth): add origin binding to admin OAuth exchange codes Bind admin OAuth exchange codes to the admin panel's origin at generation time and validate the origin on redemption. This prevents an intercepted code (via referrer leakage, logs, or network capture) from being redeemed by a different origin within the 30-second TTL. - Store the admin panel origin alongside the exchange code in cache - Extract the request origin (from Origin/Referer headers) on the exchange endpoint and pass it for validation - Reject code redemption when the request origin does not match the stored origin (code is still consumed to prevent replay) - Backward compatible: codes without a stored origin are accepted * fix(auth): add PKCE proof-of-possession to admin OAuth exchange codes Add a PKCE-like code_challenge/code_verifier flow to the admin OAuth exchange so that intercepting the exchange code alone is insufficient to redeem it. The admin panel generates a code_verifier (stored in its HttpOnly session cookie) and sends sha256(verifier) as code_challenge through the OAuth initiation URL. LibreChat stores the challenge keyed by OAuth state and attaches it to the exchange code. On redemption, the admin panel sends the verifier and LibreChat verifies the hash match. - Add verifyCodeChallenge() helper using SHA-256 - Store code_challenge in ADMIN_OAUTH_EXCHANGE cache (pkce: prefix, 5min TTL) - Capture OAuth state in callback middleware before passport processes it - Accept code_verifier in exchange endpoint body - Backward compatible: no challenge stored → PKCE check skipped * fix(auth): harden PKCE and origin binding in admin OAuth exchange - Await cache.set for PKCE challenge storage with error handling - Use crypto.timingSafeEqual for PKCE hash comparison - Drop case-insensitive flag from hex validation regexes - Add code_verifier length validation (max 512 chars) - Normalize Origin header via URL parsing in resolveRequestOrigin - Add test for undefined requestOrigin rejection - Clarify JSDoc: hex-encoded SHA-256, not RFC 7636 S256 * fix(auth): fail closed on PKCE callback cache errors, clean up origin/buffer handling - Callback middleware now redirects to error URL on cache.get failure instead of silently continuing without PKCE challenge - resolveRequestOrigin returns undefined (not raw header) on parse failure - Remove dead try/catch around Buffer.from which never throws for string input * chore(auth): remove narration comments, scope eslint-disable to lines * chore(auth): narrow query.state to string, remove narration comments in exchange.ts * fix(auth): address review findings — warn on missing PKCE challenge, validate verifier length, deduplicate URL parse - Log warning when OAuth state is present but no PKCE challenge found - Add minimum length check (>= 1) on code_verifier input validation - Update POST /oauth/exchange JSDoc to document code_verifier param - Deduplicate new URL(redirectUri) parse in createOAuthHandler - Restore intent comment on pre-delete pattern in exchangeAdminCode * test(auth): replace mock cache with real Keyv, remove all as-any casts - Use real Keyv in-memory store instead of hand-rolled Map mock - Replace jest.fn mocks with jest.spyOn on real Keyv instance - Remove redundant store.has() assertion, use cache.get() instead - Eliminate all eslint-disable and as-any suppressions - User fixture no longer needs any cast (Keyv accepts plain objects) * fix(auth): add IUser type cast for test fixture to satisfy tsc
Summary
Adds two layered security controls to the admin OAuth exchange code flow to mitigate a security finding where exchange codes acted as bearer secrets. If an exchange code leaked (via referrer headers, logs, or network interception) within its 30-second TTL, any party could redeem it for admin tokens since the
/api/admin/oauth/exchangeendpoint had no client binding.Layer 1: Origin Binding
Origin/Refererheaders) against the stored origin at redemptionLayer 2: PKCE Proof-of-Possession
code_challenge(hex-encoded SHA-256) at OAuth initiation, stored in cache keyed by OAuth statesha256(code_verifier) === stored_challengeusingcrypto.timingSafeEqualcode_verifieronly exists in the admin panel's encrypted HttpOnly session cookie — it never appears in URLs, logs, or headersImportant: Origin binding alone provides weak protection. Full mitigation requires the companion admin panel PR which implements the client-side PKCE flow. Both layers are backward-compatible: codes generated without an origin or challenge are accepted, allowing independent deployment.
Change Type
Testing
packages/api/src/auth/exchange.spec.tsusing real Keyv in-memory store withjest.spyOnChecklist