Repository navigation
fix(oauth): repair Google loopback callback flow #2796
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,9 @@ import { useEffect, useState } from "react"; | |
| */ | ||
| export default function CallbackPage() { | ||
| const [status, setStatus] = useState<"processing" | "success" | "done" | "manual">("processing"); | ||
| const [currentUrl, setCurrentUrl] = useState(""); | ||
| const [currentUrl] = useState(() => | ||
| typeof window === "undefined" ? "" : window.location.href | ||
| ); | ||
| const t = useTranslations("auth"); | ||
|
|
||
| useEffect(() => { | ||
|
|
@@ -35,9 +37,25 @@ export default function CallbackPage() { | |
| }; | ||
|
|
||
| let sent = false; | ||
| let openerSameOrigin = false; | ||
| const queueStatusUpdate = (nextStatus: "processing" | "success" | "done" | "manual") => { | ||
| queueMicrotask(() => setStatus(nextStatus)); | ||
| }; | ||
|
Comment on lines
+41
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
|
|
||
| if (window.opener) { | ||
| try { | ||
| openerSameOrigin = window.opener.location.origin === window.location.origin; | ||
| } catch { | ||
| openerSameOrigin = false; | ||
| } | ||
| } | ||
|
|
||
| // Method 1: postMessage to opener (popup mode). | ||
| // May be null when Google OAuth's COOP header severs the opener reference. | ||
| // For remote OmniRoute + local loopback callbacks, the callback page origin | ||
| // is http://127.0.0.1:<port> while the opener is the public OmniRoute origin. | ||
| // Use a wildcard fallback only for the opener that initiated this popup; the | ||
| // parent validates the OAuth state before accepting the callback. | ||
| if (window.opener) { | ||
| try { | ||
| // Target this origin specifically — popup mode is only used when isTrueLocalhost, | ||
|
|
@@ -50,6 +68,15 @@ export default function CallbackPage() { | |
| } catch (e) { | ||
| console.log("postMessage failed:", e); | ||
| } | ||
|
|
||
| if (!openerSameOrigin) { | ||
| try { | ||
| window.opener.postMessage({ type: "oauth_callback", data: callbackData }, "*"); | ||
| sent = true; | ||
| } catch (e) { | ||
| console.log("cross-origin postMessage failed:", e); | ||
| } | ||
| } | ||
|
Comment on lines
+72
to
+79
Comment on lines
+72
to
+79
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the wildcard RecommendationTo secure this cross-origin communication, you can encode the client's |
||
| } | ||
|
|
||
| // Method 2: BroadcastChannel — works across browsing context groups for same origin. | ||
|
|
@@ -75,23 +102,23 @@ export default function CallbackPage() { | |
| } | ||
|
|
||
| if (sent && (code || error)) { | ||
| if (window.opener) { | ||
| // eslint-disable-next-line react-hooks/set-state-in-effect -- initialization effect, window-only | ||
| setStatus("success"); | ||
| if (window.opener && openerSameOrigin) { | ||
| queueStatusUpdate("success"); | ||
| setTimeout(() => { | ||
| window.close(); | ||
| // If close is prevented (browser policy), fall through to manual close prompt. | ||
| setTimeout(() => setStatus("done"), 500); | ||
| }, 1500); | ||
| } else { | ||
| // Opened as new tab or opener severed by COOP — show close prompt. | ||
| setStatus("done"); | ||
| // Opened as a tab, opener severed by COOP, or remote dashboard using a | ||
| // loopback/tunnel callback. Keep the full URL visible as a manual fallback | ||
| // in case the opener cannot receive the cross-origin postMessage. | ||
| queueStatusUpdate("manual"); | ||
| } | ||
| } else { | ||
| // No code/error in URL or all send methods failed — show URL for manual copy. | ||
| // Batch the URL and status update so they render together (React 18 auto-batching). | ||
| setCurrentUrl(window.location.href); | ||
| setStatus("manual"); | ||
| queueStatusUpdate("manual"); | ||
| } | ||
|
Comment on lines
118
to
122
|
||
| }, []); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -381,9 +381,12 @@ export default function OAuthModal({ | |||||||||||||||||||||||||
| // - Codex/OpenAI: always port 1455 (registered in OAuth app) | ||||||||||||||||||||||||||
| // - Windsurf/Devin CLI (remote fallback): use localhost with OmniRoute port + /auth/callback | ||||||||||||||||||||||||||
| // (on true localhost the callback server handles it; this is only reached on remote) | ||||||||||||||||||||||||||
| // - Google OAuth providers (antigravity, gemini-cli): default to localhost so the | ||||||||||||||||||||||||||
| // bundled credentials keep working. The authorize route upgrades this to the public | ||||||||||||||||||||||||||
| // callback when custom Google credentials + NEXT_PUBLIC_BASE_URL are configured. | ||||||||||||||||||||||||||
| // - Google OAuth providers (antigravity, gemini-cli): default to loopback so the | ||||||||||||||||||||||||||
| // bundled native/desktop credentials keep working. Prefer 127.0.0.1 over | ||||||||||||||||||||||||||
| // localhost for the Google native-app handoff; Google documents that localhost | ||||||||||||||||||||||||||
| // can run into local firewall/name-resolution edge cases. The authorize route | ||||||||||||||||||||||||||
| // upgrades this to the public callback when custom Google web credentials plus | ||||||||||||||||||||||||||
| // NEXT_PUBLIC_BASE_URL or OMNIROUTE_PUBLIC_BASE_URL are configured. | ||||||||||||||||||||||||||
| // - Other providers on remote: use actual origin (supports PUBLIC_URL env var) | ||||||||||||||||||||||||||
| // - Localhost: use localhost:port | ||||||||||||||||||||||||||
| let redirectUri: string; | ||||||||||||||||||||||||||
|
|
@@ -395,10 +398,10 @@ export default function OAuthModal({ | |||||||||||||||||||||||||
| const port = window.location.port || "20128"; | ||||||||||||||||||||||||||
| redirectUri = `http://localhost:${port}/auth/callback`; | ||||||||||||||||||||||||||
| } else if (GOOGLE_OAUTH_PROVIDERS.has(provider)) { | ||||||||||||||||||||||||||
| // Google OAuth built-in credentials only accept localhost redirect URIs. | ||||||||||||||||||||||||||
| // Even in remote deployments we use localhost — user copies the callback URL manually. | ||||||||||||||||||||||||||
| // Google OAuth built-in credentials only accept loopback redirect URIs. | ||||||||||||||||||||||||||
| // Even in remote deployments we use loopback — user copies the callback URL manually. | ||||||||||||||||||||||||||
| const port = window.location.port || "20128"; | ||||||||||||||||||||||||||
| redirectUri = `http://localhost:${port}/callback`; | ||||||||||||||||||||||||||
| redirectUri = `http://127.0.0.1:${port}/callback`; | ||||||||||||||||||||||||||
| } else if (!isLocalhost) { | ||||||||||||||||||||||||||
| // Behind reverse proxy: use actual origin (e.g., https://omniroute.example.com/callback) | ||||||||||||||||||||||||||
| // Supports PUBLIC_URL env var override, or falls back to window.location.origin. | ||||||||||||||||||||||||||
|
|
@@ -497,6 +500,13 @@ export default function OAuthModal({ | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const { code, state, error: callbackError, errorDescription } = data; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (authData?.state && state && state !== authData.state) { | ||||||||||||||||||||||||||
| callbackProcessedRef.current = true; | ||||||||||||||||||||||||||
| setError("OAuth state mismatch. Restart the connection and try again."); | ||||||||||||||||||||||||||
| setStep("error"); | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+503
to
+508
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If To ensure robust CSRF protection, the state validation should fail if the expected state is present but the returned state is either missing or does not match.
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (callbackError) { | ||||||||||||||||||||||||||
| callbackProcessedRef.current = true; | ||||||||||||||||||||||||||
| setError(errorDescription || callbackError); | ||||||||||||||||||||||||||
|
|
@@ -515,12 +525,26 @@ export default function OAuthModal({ | |||||||||||||||||||||||||
| // Accept same-origin OR localhost with same port (remote access scenario: | ||||||||||||||||||||||||||
| // dashboard at 192.168.x:port, callback redirects to localhost:port) | ||||||||||||||||||||||||||
| const currentPort = window.location.port; | ||||||||||||||||||||||||||
| const isLocalhostSamePort = | ||||||||||||||||||||||||||
| event.origin.match(/^https?:\/\/(localhost|127\.0\.0\.1)(:\d+)?$/) && | ||||||||||||||||||||||||||
| new URL(event.origin).port === currentPort; | ||||||||||||||||||||||||||
| if (event.origin !== window.location.origin && !isLocalhostSamePort) return; | ||||||||||||||||||||||||||
| let isLoopbackOrigin = false; | ||||||||||||||||||||||||||
| let isLocalhostSamePort = false; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| const eventUrl = new URL(event.origin); | ||||||||||||||||||||||||||
| isLoopbackOrigin = /^(localhost|127\.0\.0\.1|\[::1\]|::1)$/i.test(eventUrl.hostname); | ||||||||||||||||||||||||||
| isLocalhostSamePort = isLoopbackOrigin && eventUrl.port === currentPort; | ||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||
| // Ignore malformed origins. | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const payload = event.data?.data; | ||||||||||||||||||||||||||
| const hasMatchingState = !!authData?.state && payload?.state === authData.state; | ||||||||||||||||||||||||||
| const isGoogleLoopbackRelay = | ||||||||||||||||||||||||||
| GOOGLE_OAUTH_PROVIDERS.has(provider) && isLoopbackOrigin && hasMatchingState; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (event.origin !== window.location.origin && !isLocalhostSamePort && !isGoogleLoopbackRelay) { | ||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (event.data?.type === "oauth_callback") { | ||||||||||||||||||||||||||
| handleCallback(event.data.data); | ||||||||||||||||||||||||||
| handleCallback(payload); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+538
to
548
Comment on lines
546
to
548
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the received message has Adding a check to ensure
Suggested change
|
||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| window.addEventListener("message", handleMessage); | ||||||||||||||||||||||||||
|
|
@@ -568,7 +592,7 @@ export default function OAuthModal({ | |||||||||||||||||||||||||
| window.removeEventListener("storage", handleStorage); | ||||||||||||||||||||||||||
| if (channel) channel.close(); | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| }, [authData, exchangeTokens]); | ||||||||||||||||||||||||||
| }, [authData, exchangeTokens, provider]); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Fix #344: Detect when OAuth popup is closed without completing authorization | ||||||||||||||||||||||||||
| // Some providers (like Qoder) redirect to their own chat UI instead of sending a callback, | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
currentUrlis never updated via a state setter and is only read whenstatus === "manual"(which occurs after mount on the client side), there is no need to manage it as React state. Declaring it as a simple constant is cleaner, more performant, and avoids unnecessary state overhead.