🔐 fix: Stop Deriving the Exchange Origin from the Referer on SSO Callbacks - #114
🔐 fix: Stop Deriving the Exchange Origin from the Referer on SSO Callbacks#114dustinhealy wants to merge 5 commits into
Conversation
…backs When the OAuth exchange request reaches the panel without an Origin header (proxies or clients that drop it), the referer identifies the initiating page or IdP rather than the panel, and multi-hop proxy chains produce comma-joined x-forwarded-proto values that parse into an invalid origin. Either way LibreChat's exchange-code origin binding rejects the code with INVALID_OR_EXPIRED_CODE, shown as "Authorization code has expired. Please try again." despite successful IdP authentication. Resolve the panel's own origin from the Origin header when present, otherwise from host plus the first x-forwarded-proto value, and never from the referer. Verified end to end against a stub implementing LibreChat's exchange contract: with an IdP referer and no Origin, main forwards the IdP origin and fails; this branch forwards the panel origin and completes login, including the full browser SSO flow.
|
@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: b155debc3b
ℹ️ 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".
When a reverse proxy rewrites Host to the internal upstream authority (for example admin-panel:3000) while preserving the browser-visible authority in X-Forwarded-Host, the host fallback constructed an internal origin that LibreChat's exchange-code origin binding rejects. getRequestOrigin now takes the first x-forwarded-host value over host, mirroring the existing x-forwarded-proto handling.
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
…roto is absent Defaulting the scheme to http when both the Origin header and x-forwarded-proto are missing broke HTTPS deployments whose proxy strips Origin without forwarding proto metadata. getRequestOrigin now uses the referer's origin in that case, but only when the referer host matches the serving host, so a foreign referer such as an IdP origin can never be forwarded as the panel's own origin.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca39b0073a
ℹ️ 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".
…in override Deployments behind proxies that strip the Origin header and forward no x-forwarded-proto or x-forwarded-host metadata had no deterministic way to satisfy LibreChat's exchange-code origin binding. When ADMIN_PANEL_PUBLIC_URL is set, getRequestOrigin now sends its origin directly, before any header derivation, and operators can mirror LibreChat's ADMIN_PANEL_URL value. The header chain (origin, forwarded host and proto, same-host referer) is unchanged as the fallback, and a malformed value is warned about and ignored.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5078f2cedb
ℹ️ 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".
A syntactically valid non-web ADMIN_PANEL_PUBLIC_URL such as file: or mailto: parses to origin null or an unusable scheme and would override valid request headers, so the override now requires an http or https protocol and a non-null origin before it wins. A serving authority carrying an explicit default port like example.com:443 failed the same-host referer comparison because URL strips default ports, so the comparison now strips the referer scheme's default port from the serving host first.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Fixes the Azure EntraID admin SSO failure where login succeeds at the IdP but the panel shows "Authentication failed / Authorization code has expired. Please try again." (401 from
/api/admin/oauth/exchange).When the OAuth exchange request reaches the panel without an
Originheader (proxies or clients that drop it),getRequestOriginfell back to therefererheader, which identifies whatever page or IdP initiated the request rather than the panel itself; on an IdP-initiated callback that ishttps://login.microsoftonline.com. Separately, multi-hop proxy chains (the report runsTRUST_PROXY=3) produce comma-joinedx-forwarded-protovalues that parsed into an invalid origin. Either way the panel forwarded a wrongOriginto LibreChat, whose exchange-code origin binding rejects it withINVALID_OR_EXPIRED_CODEeven though authentication succeeded.The panel's own origin now resolves in a deterministic order: the new optional
ADMIN_PANEL_PUBLIC_URLenv var when set (an authoritative override for deployments whose proxies strip Origin and forward no metadata; mirror LibreChat'sADMIN_PANEL_URL), then theOriginheader, then forwarding metadata: the firstx-forwarded-hostvalue wins overhost(Host-rewriting proxies replacehostwith the internal upstream authority, e.g.admin-panel:3000, which would equally fail the origin binding), the firstx-forwarded-protovalue supplies the scheme, and when proto metadata is absent the scheme is recovered from the referer only when the referer host matches the serving host, so a foreign IdP referer can never be forwarded as the panel origin.The "State present but no PKCE challenge found" warning in the reporter's LibreChat logs is a separate LibreChat-side defect: openid-client v6 overwrites the admin OAuth
statewith a random one when the IdP metadata does not advertisecode_challenge_methods_supported(Azure omits it), orphaning the cached PKCE challenge. That degrades PKCE to unenforced but does not cause the 401; it needs an upstream fix inopenidStrategy.js(pin a nonce when the IdP does not advertise PKCE so the state survives). SettingOPENID_GENERATE_NONCE=trueon LibreChat is a working mitigation today.Fixes #79
Change Type
Testing
Regression tests drive the real
oauthExchangeFnpath: an exchange carrying an IdP referer and noOriginmust forward the panel's serving origin (fails before the fix withhttps://login.microsoftonline.com), multi-valuex-forwarded-protomust resolve to the first value,x-forwarded-hostmust win over a proxy-rewritten upstreamhost, multi-valuex-forwarded-hostmust use the first value, a same-host https referer must recover the scheme when proto metadata is absent while a foreign referer must not,ADMIN_PANEL_PUBLIC_URLmust win over all headers when set, and a malformed value must fall back to header derivation with a warning. Also verified end to end at runtime against a stub implementing LibreChat's exact exchange contract (one-time codes, origin binding, PKCE verification): with an IdP referer and noOrigin, main forwardsOrigin: https://login.microsoftonline.comand fails with the exact user-visible expired-code message, while this branch forwards the panel's own origin and completes login, including the full browser SSO flow on both dev and production builds. The commax-forwarded-protocase was likewise verified at runtime (main produced an unparseable origin and failed; this branch succeeds). Local gates: fullvitestsuite passes (803 tests),eslintclean,tsc --noEmitclean.Checklist