fix(auth): derive OAuth callback URL from request origin to prevent session loss#6912
fix(auth): derive OAuth callback URL from request origin to prevent session loss#6912devin-ai-integration[bot] wants to merge 1 commit into
Conversation
…ession loss The OAuth callback URL was statically set to SITE_URL (app.infisical.com), but users initiating login from us.infisical.com had their session cookie scoped to that domain. When the OAuth provider redirected back to app.infisical.com, the session cookie was not sent, causing state/PKCE verification to fail. Now the callback URL is derived from the request's protocol and hostname so that the OAuth provider redirects back to the same domain the user started from, keeping the session cookie in scope. Co-Authored-By: jake <jake@infisical.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 825e25521b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const getOAuthCallbackUrl = (req: { protocol: string; hostname: string }, path: string) => | ||
| `${req.protocol}://${req.hostname}${path}`; |
There was a problem hiding this comment.
Preserve ports when deriving OAuth callback URLs
In this Fastify v5 app, req.hostname is the host without any :port, so deployments reached at a non-default public origin such as http://localhost:8080 or https://example.com:8443 now send OAuth providers and post-login redirects to http(s)://host/api/v1/sso/... instead. Those callbacks land on the wrong port and miss the callback route/session cookie, whereas the previous SITE_URL value could include the port. Use the request host/origin value that preserves the port when building these URLs.
Useful? React with 👍 / 👎.
|
| Filename | Overview |
|---|---|
| backend/src/server/routes/v1/sso-router.ts | Derives OAuth callbackURL and post-auth redirect base from req.hostname/req.protocol (attacker-controllable headers) without allowlist validation, creating an open redirect and a broken fallback to SITE_URL. |
Reviews (1): Last reviewed commit: "fix(auth): derive OAuth callback URL fro..." | Re-trigger Greptile
| const getOAuthCallbackUrl = (req: { protocol: string; hostname: string }, path: string) => | ||
| `${req.protocol}://${req.hostname}${path}`; |
There was a problem hiding this comment.
Host-header injection enables open redirect post-authentication
req.hostname is derived from the HTTP Host header (and req.protocol from X-Forwarded-Proto when trustProxy: true, which is the default in app.ts). Both values are attacker-controllable without MitM. On the callback endpoints (/google, /github, /gitlab), reqOrigin is built from these headers and used as the base URL for post-auth redirects (/login/select-organization, /signup/sso). An attacker who sends the OAuth callback to the backend directly (e.g., bypassing the reverse proxy) with Host: evil.com will get the authenticated browser redirected to evil.com/login/select-organization, which is a post-login open redirect. The fix should validate req.hostname against a set of trusted origins (e.g. parsed from appCfg.SITE_URL plus any additional known hostnames) before using it in URLs, and fall back to appCfg.SITE_URL on mismatch.
| @@ -506,7 +514,7 @@ export const registerSsoRouter = async (server: FastifyZodProvider) => { | |||
|
|
|||
| if (passportResult.result === ProviderAuthResult.SIGNUP_REQUIRED) { | |||
| const serverCfg = await getServerCfg(); | |||
| const signupUrl = new URL("/signup/sso", appCfg.SITE_URL); | |||
| const signupUrl = new URL("/signup/sso", reqOrigin || appCfg.SITE_URL); | |||
There was a problem hiding this comment.
Fallback to
appCfg.SITE_URL is unreachable dead code
getOAuthCallbackUrl(req, "") always returns a non-empty string — at minimum "https://" if req.hostname is empty. Because the result is always truthy, reqOrigin || appCfg.SITE_URL always resolves to reqOrigin and never reaches appCfg.SITE_URL. When req.hostname is absent or blank, new URL("/login/select-organization", "https://") throws a TypeError: Invalid URL (same applies to the /signup/sso redirect), causing an unhandled 500 rather than a graceful fallback. The intended guard should check that req.hostname is non-empty before building reqOrigin, or the fallback condition should compare the built URL against the empty-hostname case.
Context
OAuth login (Google/GitHub/GitLab) fails when initiated from
us.infisical.combecause:us.infisical.com${SITE_URL}/api/v1/sso/<provider>(resolving toapp.infisical.com)app.infisical.com, the session cookie is not sent (different domain)/login/provider/errorThis affects the VS Code extension (which opens
us.infisical.com/login?callback_port=PORT) and any user onus.infisical.comusing OAuth.Fix: Derive the callback URL from the request's
protocolandhostnameinstead of the staticSITE_URL. This ensures the OAuth provider redirects back to the same domain the user started from, keeping the session cookie in scope.The dynamic
callbackURLis passed topassport.authenticate()in both the redirect and callback phases for all three providers (Google, GitHub, GitLab). Post-login redirects also use the request origin.Note: The OAuth apps in Google/GitHub/GitLab developer consoles must have both
us.infisical.comandapp.infisical.comcallback URLs registered for this to work.Steps to verify the change
us.infisical.com/login(or use the VS Code extension with US Region)us.infisical.com/login/select-organization(notapp.infisical.com)/login/provider/errorredirect)Type
Checklist
type(scope): short description(scope is optional, e.g.,fix: prevent crash on syncorfix(api): handle null response).Link to Devin session: https://app.devin.ai/sessions/919c400b80094b71b80c37b22bcaf540