From f08f71313f5ef0e7517a237f34f97cf726a008e7 Mon Sep 17 00:00:00 2001 From: Aidan Date: Sun, 14 Jun 2026 07:55:24 -0700 Subject: [PATCH] [aidan] ui/ux: auth flow fixes (#84) * aidan] bug: fixed 2 auth claude login issue * [aidan] bug: claude login w email+pw * [aidan] ui: edited message for cases where anthropic redirects to enter email code even after login' * [aidan] ui/ux: fix connecting status accuracy --- backend/apps/agents/9router_gpt5_patch.js | 27 +++++++ backend/apps/agents/agents.py | 16 ++++- backend/apps/nine_router/oauth.py | 20 ++++-- backend/main.py | 2 +- .../subscription/SubscriptionCards.tsx | 7 +- .../subscription/subscriptionConnect.ts | 71 +++++++++++++++++-- .../src/shared/state/subscriptionsSlice.ts | 24 ++++++- 7 files changed, 151 insertions(+), 16 deletions(-) diff --git a/backend/apps/agents/9router_gpt5_patch.js b/backend/apps/agents/9router_gpt5_patch.js index 75b035f4..885efebe 100644 --- a/backend/apps/agents/9router_gpt5_patch.js +++ b/backend/apps/agents/9router_gpt5_patch.js @@ -39,6 +39,33 @@ const _http = require('http'); } catch (_) {} })(); +// 9Router's /callback page is a client-side relay (postMessage/BroadcastChannel/ +// localStorage) that fails when the OAuth flow runs in the user's system browser: +// no opener, different cookie jar. 302 to the backend so the exchange happens +// server-side. Idempotent via _completed_oauth (backend/apps/oauth_state.py) so +// a racing renderer-driven exchange in popup mode dedups. +(function patchOauthCallbackRedirect() { + try { + const http = require('http'); + const origEmit = http.Server.prototype.emit; + http.Server.prototype.emit = function patchedEmit(event, req, res) { + if (event === 'request' && req && res) { + try { + const url = req.url || ''; + if (url.startsWith('/callback?')) { + const backendPort = process.env.OPENSWARM_PORT || '8324'; + const target = 'http://localhost:' + backendPort + '/api/subscriptions/callback' + url.slice('/callback'.length); + res.writeHead(302, { Location: target }); + res.end(); + return true; + } + } catch (_) {} + } + return origEmit.apply(this, arguments); + }; + } catch (_) {} +})(); + const TARGET_HOSTS = new Set(['api.openai.com']); const DEBUG = process.env.OPENSWARM_DEBUG_GPT5_PATCH === '1'; diff --git a/backend/apps/agents/agents.py b/backend/apps/agents/agents.py index cec4a8c6..96746f02 100644 --- a/backend/apps/agents/agents.py +++ b/backend/apps/agents/agents.py @@ -432,6 +432,11 @@ async def subscriptions_poll(body: dict): async def subscriptions_exchange(body: dict): """Exchange OAuth code for tokens via 9Router.""" from backend.apps.nine_router import exchange_oauth + from backend.apps.oauth_state import ( + _pending_oauth as pending_oauth, + _completed_oauth as completed_oauth, + _mark_oauth_completed as mark_completed, + ) provider = body.get("provider", "") code = body.get("code", "") redirect_uri = body.get("redirect_uri", "") @@ -444,11 +449,18 @@ async def subscriptions_exchange(body: dict): try: result = await exchange_oauth(provider, code, redirect_uri, code_verifier, state) if result.get("success"): - from backend.apps.service.client import sync as _sync + # Claude races this path against /api/subscriptions/callback (popup + 9router patch + # 302 to backend); dedup so the loser sees the success page, not "Session expired". + if state: + pending_oauth.pop(state, None) + mark_completed(state) + from backend.apps.service.client import sync as do_sync from backend.apps.settings.settings import load_settings - _sync(load_settings().model_dump()) + do_sync(load_settings().model_dump()) return result except Exception as e: + if state and state in completed_oauth: + return {"success": True, "deduped": True} raise HTTPException(status_code=500, detail=str(e)) diff --git a/backend/apps/nine_router/oauth.py b/backend/apps/nine_router/oauth.py index 55b7ba85..efcc5e75 100644 --- a/backend/apps/nine_router/oauth.py +++ b/backend/apps/nine_router/oauth.py @@ -205,9 +205,14 @@ async def _start_codex_callback_listener(timeout: float = 300.0) -> asyncio.base # own Desktop-app OAuth guidance both prescribe the system browser. # - codex: auth.openai.com renders blank in our popup on some machines (newer # embed detection + regional checks); system browser surfaces the real error. +# - claude: email magic-link opens in the user's default browser, which is a +# different cookie jar from the embedded popup, so the popup can never receive +# the auth. Forcing the OAuth flow into the system browser keeps everything +# in one cookie jar. # The callback for gemini-cli/antigravity lands on /api/subscriptions/callback -# and runs the exchange server-side; codex uses its fixed 1455 listener. -_EXTERNAL_BROWSER_PROVIDERS: set[str] = {"gemini-cli", "antigravity", "codex"} +# and runs the exchange server-side; codex uses its fixed 1455 listener; claude +# is special-cased in _callback_uri_for_provider below. +_EXTERNAL_BROWSER_PROVIDERS: set[str] = {"gemini-cli", "antigravity", "codex", "claude"} def _should_use_external_browser(provider: str) -> bool: @@ -232,18 +237,21 @@ def _callback_uri_for_provider(provider: str) -> str: """Return the redirect URI to pass to 9Router's authorize endpoint. Most providers accept 9Router's built-in callback page at port 20128. - Two special cases: + Special cases: - Codex/OpenAI's OAuth client is bound to a fixed http://localhost:1455/auth/callback URI; handled by _start_codex_callback_listener above. - Gemini/Google's OAuth consent page rejects embedded browsers, so we route the callback through OpenSwarm's backend endpoint at - /api/subscriptions/callback (backend/main.py:138) which runs the - exchange itself. This is the only provider where the callback lands - on OpenSwarm's port rather than 9Router's. + /api/subscriptions/callback (backend/main.py) which runs the + exchange itself. """ if provider == "codex": return f"http://localhost:{_CODEX_CALLBACK_PORT}{_CODEX_CALLBACK_PATH}" + # Anthropic's OAuth client only whitelists localhost:20128/callback; + # 9router_gpt5_patch.js 302-rewrites the hit to the backend handler. + if provider == "claude": + return f"http://localhost:{NINE_ROUTER_PORT}/callback" if provider in _EXTERNAL_BROWSER_PROVIDERS: return f"http://localhost:{_backend_port()}/api/subscriptions/callback" return f"http://localhost:{NINE_ROUTER_PORT}/callback" diff --git a/backend/main.py b/backend/main.py index 42daf1d3..9aa7d69f 100644 --- a/backend/main.py +++ b/backend/main.py @@ -453,7 +453,7 @@ _SUCCESS_HTML = ( '
' '
✓
' '

Connected!

' - '

You can close this window

' + '

You can close this tab, and any other Claude login tab still open.

' '
' '' '' diff --git a/frontend/src/app/pages/Settings/sections/subscription/SubscriptionCards.tsx b/frontend/src/app/pages/Settings/sections/subscription/SubscriptionCards.tsx index 70d82e75..d3dba0d5 100644 --- a/frontend/src/app/pages/Settings/sections/subscription/SubscriptionCards.tsx +++ b/frontend/src/app/pages/Settings/sections/subscription/SubscriptionCards.tsx @@ -8,6 +8,7 @@ import { fetchModels } from '@/shared/state/modelsSlice'; import { fetchSubscriptionStatus, setSubscriptionStatus, + markSubscriptionConnected, selectSubscriptionConnections, } from '@/shared/state/subscriptionsSlice'; import { API_BASE } from '@/shared/config'; @@ -37,6 +38,10 @@ const SubscriptionCards: React.FC = () => { // Refetch model picker after sub changes so newly-connected providers surface in the dropdown immediately. const refreshPickerModels = () => { dispatch(fetchModels()); }; + const markConnected = useCallback((provider: string) => { + dispatch(markSubscriptionConnected({ provider })); + }, [dispatch]); + useEffect(() => { let cancelled = false; (async () => { @@ -73,7 +78,7 @@ const SubscriptionCards: React.FC = () => { }); if (!r.ok) { setConnecting(null); return; } const data = await r.json(); - runConnectFlow({ providerId, data, setConnecting, setUserCode, setPollTimer, fetchStatus, refreshPickerModels }); + runConnectFlow({ providerId, data, setConnecting, setUserCode, setPollTimer, fetchStatus, refreshPickerModels, markConnected }); } catch { setConnecting(null); } }; diff --git a/frontend/src/app/pages/Settings/sections/subscription/subscriptionConnect.ts b/frontend/src/app/pages/Settings/sections/subscription/subscriptionConnect.ts index cbd657e4..c63447c8 100644 --- a/frontend/src/app/pages/Settings/sections/subscription/subscriptionConnect.ts +++ b/frontend/src/app/pages/Settings/sections/subscription/subscriptionConnect.ts @@ -8,11 +8,12 @@ interface ConnectCtx { setPollTimer: (v: any) => void; fetchStatus: (opts?: { preserveTransient?: boolean }) => Promise; refreshPickerModels: () => void; + markConnected: (provider: string) => void; } // Device-code OAuth flow: popup + dual poller (device-code + status) + focus-listener safety net + 5min hard timeout. function runDeviceCodeFlow(ctx: ConnectCtx) { - const { providerId, data, setConnecting, setUserCode, setPollTimer, fetchStatus, refreshPickerModels } = ctx; + const { providerId, data, setConnecting, setUserCode, setPollTimer, fetchStatus, refreshPickerModels, markConnected } = ctx; const code = data.user_code || ''; setUserCode(code); // Named window + features so Electron's setWindowOpenHandler spawns a BrowserWindow popup, not a webview tab. @@ -31,6 +32,7 @@ function runDeviceCodeFlow(ctx: ConnectCtx) { setPollTimer(null); setConnecting(null); setUserCode(''); + markConnected(providerId); fetchStatus(); refreshPickerModels(); // Auto-close popup 2s after success so the "Congratulations" page is briefly visible then closes. @@ -131,7 +133,7 @@ function runDeviceCodeFlow(ctx: ConnectCtx) { // Authorization-code flow: external-browser or popup + status poller + postMessage/IPC relay + bounded timeout. function runAuthCodeFlow(ctx: ConnectCtx) { - const { providerId, data, setConnecting, setPollTimer, fetchStatus, refreshPickerModels } = ctx; + const { providerId, data, setConnecting, setPollTimer, fetchStatus, refreshPickerModels, markConnected } = ctx; // Gemini/Google block embedded browsers; backend sets use_external_browser and exchange happens server-side via /api/subscriptions/callback. Detect via status poller (no postMessage possible). const useExternal = !!data.use_external_browser; let popup: Window | null = null; @@ -141,16 +143,24 @@ function runAuthCodeFlow(ctx: ConnectCtx) { popup = window.open(data.auth_url, 'oauth_connect', 'width=600,height=700'); } + let stopped = false; + let resetTimer: ReturnType | null = null; + // Status polling: primary for external-browser flow, secondary for popup flow (postMessage is faster). const statusPoller = setInterval(async () => { + if (stopped) return; try { const sr = await fetch(`${API_BASE}/agents/subscriptions/status`); const sd = await sr.json(); const connections = sd.providers?.connections || []; if (connections.some((p: any) => p.provider === providerId && (p.isActive || p.testStatus === 'active'))) { + stopped = true; + if (resetTimer) clearTimeout(resetTimer); clearInterval(statusPoller); setPollTimer(null); if (!useExternal) window.removeEventListener('message', msgHandler); + window.removeEventListener('blur', onBlur); + window.removeEventListener('focus', onFocus); setConnecting(null); fetchStatus(); refreshPickerModels(); @@ -162,15 +172,20 @@ function runAuthCodeFlow(ctx: ConnectCtx) { // Shared exchange helper invoked by whichever relay path delivers the code first. let exchanged = false; const runExchange = async (code: string, state?: string) => { - if (exchanged) return; + if (exchanged || stopped) return; exchanged = true; + stopped = true; + if (resetTimer) clearTimeout(resetTimer); window.removeEventListener('message', msgHandler); if (ipcUnsub) ipcUnsub(); + window.removeEventListener('blur', onBlur); + window.removeEventListener('focus', onFocus); clearInterval(statusPoller); setPollTimer(null); if (popup && !popup.closed) popup.close(); + let succeeded = false; try { - await fetch(`${API_BASE}/agents/subscriptions/exchange`, { + const r = await fetch(`${API_BASE}/agents/subscriptions/exchange`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ provider: providerId, code, @@ -178,9 +193,13 @@ function runAuthCodeFlow(ctx: ConnectCtx) { state: state || data.state, }), }); + let body: any = null; + try { body = await r.json(); } catch {} + succeeded = r.ok && !!body?.success; } catch {} + // 9Router /providers lags /exchange; an immediate fetchStatus would clobber the UI. + if (succeeded) markConnected(providerId); setConnecting(null); - fetchStatus(); refreshPickerModels(); }; @@ -201,13 +220,55 @@ function runAuthCodeFlow(ctx: ConnectCtx) { }); } + // If the user comes back to openswarm without finishing OAuth (closed the browser, cancelled), + // 3s of sustained focus + no active connection means abandoned; clear Connecting so they can retry. + // A blur during the wait cancels, so brief tab-backs to check progress don't false-positive. + const onBlur = () => { + if (resetTimer) { clearTimeout(resetTimer); resetTimer = null; } + }; + const onFocus = () => { + if (stopped) return; + if (resetTimer) clearTimeout(resetTimer); + resetTimer = setTimeout(async () => { + resetTimer = null; + if (stopped) return; + try { + const sr = await fetch(`${API_BASE}/agents/subscriptions/status`); + const sd = await sr.json(); + const conns = sd.providers?.connections || []; + if (conns.some((p: any) => p.provider === providerId && (p.isActive || p.testStatus === 'active'))) return; + } catch {} + if (stopped) return; + stopped = true; + clearInterval(statusPoller); + setPollTimer(null); + if (!useExternal) window.removeEventListener('message', msgHandler); + if (ipcUnsub) ipcUnsub(); + window.removeEventListener('blur', onBlur); + window.removeEventListener('focus', onFocus); + setConnecting(null); + }, 3000); + }; + // Delay attach; popup mode's window.open blurs/refocuses the parent and would false-trigger. + setTimeout(() => { + if (!stopped) { + window.addEventListener('blur', onBlur); + window.addEventListener('focus', onFocus); + } + }, 2000); + // 3min popup / 5min external-browser; bounds the Connecting indicator, safety-net poller is the real exit. const timeoutMs = useExternal ? 300_000 : 180_000; setTimeout(() => { + if (stopped) return; + stopped = true; + if (resetTimer) clearTimeout(resetTimer); clearInterval(statusPoller); setPollTimer(null); if (!useExternal) window.removeEventListener('message', msgHandler); if (ipcUnsub) ipcUnsub(); + window.removeEventListener('blur', onBlur); + window.removeEventListener('focus', onFocus); setConnecting(null); }, timeoutMs); } diff --git a/frontend/src/shared/state/subscriptionsSlice.ts b/frontend/src/shared/state/subscriptionsSlice.ts index ca5c5054..4fe46b6d 100644 --- a/frontend/src/shared/state/subscriptionsSlice.ts +++ b/frontend/src/shared/state/subscriptionsSlice.ts @@ -51,6 +51,28 @@ const subscriptionsSlice = createSlice({ setSubscriptionStatus(state, action: PayloadAction) { state.status = action.payload; }, + // Optimistic: 9Router /providers lags /exchange, so refetching right after would + // clobber the just-connected state with stale data. The 30s poller reconciles. + markSubscriptionConnected(state, action: PayloadAction<{ provider: string }>) { + if (!state.status) return; + const { provider } = action.payload; + const isArr = Array.isArray(state.status.providers); + const conns: SubscriptionConnection[] = isArr + ? (state.status.providers as SubscriptionConnection[]) + : ((state.status.providers as { connections?: SubscriptionConnection[] } | undefined)?.connections ?? []); + const existing = conns.find((c) => c.provider === provider); + if (existing) { + existing.isActive = true; + existing.testStatus = 'active'; + } else { + conns.push({ provider, isActive: true, testStatus: 'active' }); + } + if (isArr) { + state.status.providers = conns; + } else { + state.status.providers = { connections: conns }; + } + }, }, extraReducers: (builder) => { builder.addCase(fetchSubscriptionStatus.fulfilled, (state, action) => { @@ -59,7 +81,7 @@ const subscriptionsSlice = createSlice({ }, }); -export const { setSubscriptionStatus } = subscriptionsSlice.actions; +export const { setSubscriptionStatus, markSubscriptionConnected } = subscriptionsSlice.actions; // Stable empty ref so the selector doesn't hand back a fresh [] each call (forces needless rerenders). const EMPTY_CONNECTIONS: SubscriptionConnection[] = [];