From 55181a7d7982ad5588c9de4bec40e8d3f91f439e Mon Sep 17 00:00:00 2001 From: kokhlo Date: Sun, 30 Aug 2026 19:45:08 +0300 Subject: [PATCH] fix(oauth): surface actionable guidance when sign-in windows lapse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The local expires_in countdown killed OAuth sessions with a bare "Session expired" before the backend poller's enriched message (Portal sign-in stalled in the opened tab, retry/API-key fallback) could reach the UI, and the desktop onboarding poller had no local expiry at all — a dead session polled forever. Both surfaces now lapse with guidance naming the common cause, prefer the backend error_message when it has one, and keep polling when the backend still reports pending (clock skew). --- apps/desktop/src/i18n/ar.ts | 2 + apps/desktop/src/i18n/en.ts | 2 + apps/desktop/src/i18n/ja.ts | 2 + apps/desktop/src/i18n/types.ts | 1 + apps/desktop/src/i18n/zh-hant.ts | 2 + apps/desktop/src/i18n/zh.ts | 2 + apps/desktop/src/store/onboarding.test.ts | 95 +++++++++ apps/desktop/src/store/onboarding.ts | 33 +++ web/src/components/OAuthLoginModal.test.tsx | 210 ++++++++++++++++++++ web/src/components/OAuthLoginModal.tsx | 52 ++++- web/src/i18n/en.ts | 2 + web/src/i18n/types.ts | 1 + 12 files changed, 394 insertions(+), 10 deletions(-) create mode 100644 web/src/components/OAuthLoginModal.test.tsx diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 81f221ede5..656414a5e2 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -2169,6 +2169,8 @@ export const ar = defineLocale({ connectedProvider: provider => `تم ربط ${provider}`, connectedPicking: provider => `تم ربط ${provider}. جار اختيار نموذج افتراضي...`, signInFailed: 'فشل تسجيل الدخول. حاول مرة أخرى.', + signInExpired: + 'انتهت مهلة انتظار التفويض. السبب الأكثر شيوعًا هو تعطّل صفحة تسجيل الدخول في تبويب المتصفح (مشكلة من جهة الخادم) — أكمل تسجيل الدخول هناك ثم أعد المحاولة. إذا استمر الفشل، استخدم مفتاح API أو واجهة سطر الأوامر بدلاً من ذلك.', pickDifferentProvider: 'اختر مزوداً آخر', signInWith: provider => `تسجيل الدخول عبر ${provider}`, openedBrowser: provider => `فتحنا ${provider} في المتصفح.`, diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index 8d30169004..a06c363c3e 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -2798,6 +2798,8 @@ export const en: Translations = { connectedProvider: provider => `${provider} connected`, connectedPicking: provider => `${provider} connected. Picking a default model...`, signInFailed: 'Sign-in failed. Try again.', + signInExpired: + 'Sign-in expired waiting for authorization. This usually means the sign-in page stalled in the opened tab (server-side issue) — finish signing in there, then try again. If it keeps failing, use an API key or the CLI fallback instead.', pickDifferentProvider: 'Pick a different provider', signInWith: provider => `Sign in with ${provider}`, openedBrowser: provider => `We opened ${provider} in your browser.`, diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index 7ba2ec5e9c..585f229a83 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -2442,6 +2442,8 @@ export const ja = defineLocale({ connectedProvider: provider => `${provider} が接続されました`, connectedPicking: provider => `${provider} が接続されました。デフォルトモデルを選択中...`, signInFailed: 'サインインに失敗しました。再試行してください。', + signInExpired: + '承認待ちでタイムアウトしました。多くの場合、開いたタブのサインインページが止まっている（サーバー側の問題）ためです。そのページでサインインを完了してから再試行してください。解決しない場合は API キーまたは CLI を利用してください。', pickDifferentProvider: '別のプロバイダーを選択', signInWith: provider => `${provider} でサインイン`, openedBrowser: provider => `${provider} をブラウザーで開きました。`, diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index c1cd7dd8d4..cbb990042f 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -2366,6 +2366,7 @@ export interface Translations { connectedProvider: (provider: string) => string connectedPicking: (provider: string) => string signInFailed: string + signInExpired: string pickDifferentProvider: string signInWith: (provider: string) => string openedBrowser: (provider: string) => string diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index 0dec821785..277ed010a0 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -2352,6 +2352,8 @@ export const zhHant = defineLocale({ connectedProvider: provider => `${provider} 已連線`, connectedPicking: provider => `${provider} 已連線。正在選擇預設模型...`, signInFailed: '登入失敗，請重試。', + signInExpired: + '等待授權逾時。通常是因為登入頁面在開啟的分頁中卡住（伺服器端問題）——請在該頁面完成登入後重試。若仍失敗，請改用 API 金鑰或 CLI 方式。', pickDifferentProvider: '選擇其他提供方', signInWith: provider => `使用 ${provider} 登入`, openedBrowser: provider => `已在瀏覽器中開啟 ${provider}。`, diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index bdc1f61297..9bd1fdaeb8 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -2963,6 +2963,8 @@ export const zh: Translations = { connectedProvider: provider => `${provider} 已连接`, connectedPicking: provider => `${provider} 已连接。正在选择默认模型...`, signInFailed: '登录失败，请重试。', + signInExpired: + '等待授权超时。通常是因为登录页面在打开的标签页中卡住（服务端问题）——请在该页面完成登录后重试。若仍失败，请改用 API 密钥或 CLI 方式。', pickDifferentProvider: '选择其他提供方', signInWith: provider => `使用 ${provider} 登录`, openedBrowser: provider => `已在浏览器中打开 ${provider}。`, diff --git a/apps/desktop/src/store/onboarding.test.ts b/apps/desktop/src/store/onboarding.test.ts index d5392634d5..d836020cb9 100644 --- a/apps/desktop/src/store/onboarding.test.ts +++ b/apps/desktop/src/store/onboarding.test.ts @@ -1,3 +1,4 @@ +import { act } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import * as notifications from '@/store/notifications' @@ -643,3 +644,97 @@ describe('saveOnboardingLocalEndpoint', () => { expect($desktopOnboarding.get().configured).not.toBe(true) }) }) + +describe('device-code poll expiry', () => { + beforeEach(() => { + window.localStorage.clear() + $desktopOnboarding.set(baseState()) + }) + + afterEach(() => { + window.localStorage.clear() + $desktopOnboarding.set(baseState()) + vi.restoreAllMocks() + vi.useRealTimers() + }) + + function deviceCodeProvider() { + // makeOAuthProvider builds a pkce provider; device-code flows need the + // device_code branch instead. + return { ...makeOAuthProvider('nous', 'Nous Portal'), flow: 'device_code' as const } + } + + function deviceStart(expiresIn: number) { + return { + expires_in: expiresIn, + flow: 'device_code', + poll_interval: 5, + session_id: 'device-sess-1', + user_code: 'ABCD-EFGH', + verification_url: 'https://portal.example/device' + } + } + + it('lapses to an error with actionable guidance when the window expires still pending', async () => { + vi.useFakeTimers() + installApiMock(async ({ path }: { path: string }) => { + if (path === '/api/providers/oauth/nous/start') { + return deviceStart(2) + } + + if (path === '/api/providers/oauth/nous/poll/device-sess-1') { + return { status: 'pending' } + } + + throw new Error(`unexpected api path: ${path}`) + }) + + const { startProviderOAuth } = await import('./onboarding') + await startProviderOAuth(deviceCodeProvider(), onboardingContext(emptyOpenRouterGateway())) + + expect($desktopOnboarding.get().flow.status).toBe('polling') + + // Let both the poll interval and the expiry window lapse. + await act(async () => { + vi.advanceTimersByTime(3000) + }) + + const flow = $desktopOnboarding.get().flow + expect(flow.status).toBe('error') + + if (flow.status === 'error') { + expect(flow.message).toContain('Sign-in expired waiting for authorization') + } + }) + + it('keeps polling while the window is open and clears the expiry on cancel', async () => { + vi.useFakeTimers() + installApiMock(async ({ path }: { path: string }) => { + if (path === '/api/providers/oauth/nous/start') { + return deviceStart(600) + } + + if (path === '/api/providers/oauth/nous/poll/device-sess-1') { + return { status: 'pending' } + } + + throw new Error(`unexpected api path: ${path}`) + }) + + const { startProviderOAuth, cancelOnboardingFlow } = await import('./onboarding') + await startProviderOAuth(deviceCodeProvider(), onboardingContext(emptyOpenRouterGateway())) + + await act(async () => { + vi.advanceTimersByTime(10_000) + }) + expect($desktopOnboarding.get().flow.status).toBe('polling') + + cancelOnboardingFlow() + // Far past the original window: the cancelled flow must not flip to an + // expiry error after the fact. + await act(async () => { + vi.advanceTimersByTime(700_000) + }) + expect($desktopOnboarding.get().flow.status).toBe('idle') + }) +}) diff --git a/apps/desktop/src/store/onboarding.ts b/apps/desktop/src/store/onboarding.ts index 975e3fc72c..9108041503 100644 --- a/apps/desktop/src/store/onboarding.ts +++ b/apps/desktop/src/store/onboarding.ts @@ -11,6 +11,7 @@ import { submitOAuthCode, validateProviderCredential } from '@/hermes' +import { translateNow } from '@/i18n' import { isProviderSetupErrorMessage } from '@/lib/provider-setup-errors' import { evaluateRuntimeReadiness, type RuntimeReadinessResult } from '@/lib/runtime-readiness' import { setMainModelAssignment } from '@/store/cron-model-impact' @@ -175,6 +176,30 @@ function clearPoll() { window.clearInterval(pollTimer) pollTimer = null } + + clearPollExpiry() +} + +let pollExpiryTimer: number | null = null + +function clearPollExpiry() { + if (pollExpiryTimer !== null) { + window.clearTimeout(pollExpiryTimer) + pollExpiryTimer = null + } +} + +/** Lapse a device-code session locally when its window expires, instead of + * polling a dead session forever. Uses the flow's own `expires_in`; the + * backend poller may still flip the session to error first, and its message + * (surfaced by `pollSession`) is preferred whenever it arrives in time. */ +function schedulePollExpiry(start: DeviceStart, onExpire: () => void) { + clearPollExpiry() + const ttlMs = Math.max(1, Number(start.expires_in) || 0) * 1000 + pollExpiryTimer = window.setTimeout(() => { + pollExpiryTimer = null + onExpire() + }, ttlMs) } async function checkRuntime(ctx: OnboardingContext, requestedProvider?: string): Promise { @@ -630,6 +655,14 @@ export async function startProviderOAuth(provider: OAuthProvider, ctx: Onboardin } setFlow({ status: 'polling', provider, start, copied: false }) + schedulePollExpiry(start, () => + setFlow({ + status: 'error', + provider, + start, + message: translateNow('onboarding.signInExpired') + }) + ) pollTimer = window.setInterval(() => void pollSession(provider, start, ctx), POLL_MS) } catch (error) { setFlow({ status: 'error', provider, message: `Could not start sign-in: ${errMessage(error)}` }) diff --git a/web/src/components/OAuthLoginModal.test.tsx b/web/src/components/OAuthLoginModal.test.tsx new file mode 100644 index 0000000000..63453c3a7b --- /dev/null +++ b/web/src/components/OAuthLoginModal.test.tsx @@ -0,0 +1,210 @@ +// @vitest-environment jsdom +// OAuthLoginModal expiry behaviour: when the sign-in window lapses locally, +// the modal must surface the backend poller's actionable message when it has +// one, keep polling when the backend still considers the session pending +// (clock skew), and fall back to guidance naming the common cause instead of +// the old bare "Session expired". + +import { act, type ReactNode } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { I18nProvider } from "@/i18n"; + +const apiMocks = vi.hoisted(() => ({ + cancelOAuthSession: vi.fn(async () => ({ ok: true })), + pollOAuthSession: vi.fn(), + startOAuthLogin: vi.fn(), +})); + +vi.mock("@/lib/api", () => ({ + api: { + cancelOAuthSession: apiMocks.cancelOAuthSession, + pollOAuthSession: apiMocks.pollOAuthSession, + startOAuthLogin: apiMocks.startOAuthLogin, + }, +})); + +vi.mock("@/lib/clipboard", () => ({ + copyTextToClipboard: vi.fn(async () => true), +})); + +import { OAuthLoginModal } from "./OAuthLoginModal"; + +let container: HTMLDivElement; +let root: Root; + +const provider = { + cli_command: "hermes login nous", + disconnectable: true, + docs_url: "https://example.com/nous", + flow: "device_code" as const, + id: "nous", + name: "Nous Portal", + status: { logged_in: false }, +}; + +function deviceStart(expiresIn: number) { + return { + expires_in: expiresIn, + flow: "device_code", + poll_interval: 5, + session_id: "sess-1", + user_code: "ABCD-EFGH", + verification_url: "https://portal.example/device", + }; +} + +async function render(ui: ReactNode) { + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + await act(async () => root.render({ui})); +} + +/** Fast-forward the countdown (1s ticks) until it lapses. */ +async function exhaustCountdown(seconds: number) { + for (let i = 0; i <= seconds; i++) { + await act(async () => { + vi.advanceTimersByTime(1000); + }); + } + await act(async () => {}); +} + +beforeEach(() => { + (globalThis as Record).IS_REACT_ACT_ENVIRONMENT = true; + vi.useFakeTimers(); + apiMocks.startOAuthLogin.mockReset(); + apiMocks.pollOAuthSession.mockReset(); + apiMocks.cancelOAuthSession.mockReset().mockResolvedValue({ ok: true }); + // The steady state during the countdown: still pending. + apiMocks.pollOAuthSession.mockResolvedValue({ + session_id: "sess-1", + status: "pending", + }); +}); + +afterEach(() => { + vi.runOnlyPendingTimers(); + vi.useRealTimers(); + root?.unmount(); + container?.remove(); +}); + +describe("OAuthLoginModal local expiry", () => { + it("surfaces the backend error_message when the session lapsed", async () => { + apiMocks.startOAuthLogin.mockResolvedValue(deviceStart(3)); + await render( + {}} + onSuccess={() => {}} + onError={() => {}} + />, + ); + + // The final poll flips to the backend's enriched message. + apiMocks.pollOAuthSession.mockResolvedValue({ + session_id: "sess-1", + status: "error", + error_message: + "Timed out waiting for device authorization. Portal sign-in is required before the device code can be approved.", + }); + await exhaustCountdown(5); + await act(async () => { + vi.advanceTimersByTime(2000); + }); + await act(async () => {}); + + expect(container.textContent).toContain( + "Portal sign-in is required", + ); + }); + + it("keeps polling when the backend still reports pending (clock skew)", async () => { + apiMocks.startOAuthLogin.mockResolvedValue(deviceStart(3)); + await render( + {}} + onSuccess={() => {}} + onError={() => {}} + />, + ); + + await exhaustCountdown(5); + await act(async () => {}); + + // No error box: the modal is still in the polling phase. + expect(container.textContent).toContain("ABCD-EFGH"); + expect(apiMocks.pollOAuthSession.mock.calls.length).toBeGreaterThan(0); + }); + + it("falls back to guidance naming the stalled-tab cause (pkce flow, no poll)", async () => { + apiMocks.startOAuthLogin.mockResolvedValue({ + auth_url: "https://portal.example/auth", + expires_in: 3, + flow: "pkce", + session_id: "sess-1", + }); + await render( + {}} + onSuccess={() => {}} + onError={() => {}} + />, + ); + + await exhaustCountdown(6); + await act(async () => {}); + + expect(container.textContent).toContain("stalled in the opened tab"); + // PKCE has no poll endpoint call at expiry. + expect(apiMocks.pollOAuthSession).not.toHaveBeenCalled(); + }); + + it("a Retry after expiry starts fresh instead of insta-lapsing", async () => { + apiMocks.startOAuthLogin.mockResolvedValueOnce(deviceStart(2)); + await render( + {}} + onSuccess={() => {}} + onError={() => {}} + />, + ); + + apiMocks.pollOAuthSession.mockResolvedValue({ + session_id: "sess-1", + status: "error", + error_message: "expired_token: code expired", + }); + await exhaustCountdown(4); + await act(async () => { + vi.advanceTimersByTime(2000); + }); + await act(async () => {}); + expect(container.textContent).toContain("expired_token"); + + // Retry: new start response, fresh countdown. + apiMocks.startOAuthLogin.mockResolvedValueOnce(deviceStart(60)); + apiMocks.pollOAuthSession.mockResolvedValue({ + session_id: "sess-1", + status: "pending", + }); + const retry = [...container.querySelectorAll("button")].find((b) => + b.textContent?.includes("Retry"), + ); + expect(retry).toBeTruthy(); + await act(async () => retry!.click()); + await act(async () => {}); + + // Well within the new window: still showing the code, no error. + await act(async () => { + vi.advanceTimersByTime(5000); + }); + expect(container.textContent).toContain("ABCD-EFGH"); + }); +}); diff --git a/web/src/components/OAuthLoginModal.tsx b/web/src/components/OAuthLoginModal.tsx index dd815c2ee0..c20990c14f 100644 --- a/web/src/components/OAuthLoginModal.tsx +++ b/web/src/components/OAuthLoginModal.tsx @@ -69,23 +69,54 @@ export function OAuthLoginModal({ provider, onClose, onSuccess }: Props) { // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - // Tick the countdown + // When the sign-in window lapses locally, the backend poller usually has + // the real story (e.g. "Portal sign-in is required before the device code + // can be approved") — but its next poll tick is up to 2s away. Rather than + // preempt it with a bare "expired", ask the poll endpoint once, then fall + // back to guidance that names the common cause (sign-in stalled in the + // opened tab) instead of a dead-end. + const handleLocalExpiry = async () => { + if (!isMounted.current) return; + let backendMessage: string | null = null; + if (start && start.flow === "device_code") { + try { + const resp = await api.pollOAuthSession(provider.id, start.session_id); + if (resp.error_message) backendMessage = resp.error_message; + else if (resp.status === "pending") { + // Still pending server-side: the local countdown fired early + // (clock skew or a stalled tab). Keep the session alive for the + // poller instead of killing it with a wrong "expired". + if (isMounted.current) setPhase("polling"); + return; + } + } catch { + // Poll endpoint unreachable — fall through to generic guidance. + } + } + if (!isMounted.current) return; + setPhase("error"); + setErrorMsg(backendMessage || t.oauth.sessionExpiredNoError); + }; + + // Tick the countdown down to zero — never further. What happens AT zero + // is owned by the lapse effect below, so the updater stays pure. useEffect(() => { if (secondsLeft === null) return; + if (secondsLeft <= 0) return; if (phase === "approved" || phase === "error") return; const tick = window.setInterval(() => { if (!isMounted.current) return; - setSecondsLeft((s) => { - if (s !== null && s <= 1) { - setPhase("error"); - setErrorMsg(t.oauth.sessionExpired); - return 0; - } - return s !== null && s > 0 ? s - 1 : 0; - }); + setSecondsLeft((s) => (s !== null && s > 0 ? s - 1 : 0)); }, 1000); return () => window.clearInterval(tick); - }, [secondsLeft, phase, t]); + }, [secondsLeft, phase]); + + useEffect(() => { + if (secondsLeft !== 0) return; + if (phase === "approved" || phase === "error") return; + void handleLocalExpiry(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [secondsLeft, phase]); // Device-code: poll backend every 2s useEffect(() => { @@ -350,6 +381,7 @@ export function OAuthLoginModal({ provider, onClose, onSuccess }: Props) { setErrorMsg(null); setStart(null); setPkceCode(""); + setSecondsLeft(null); setPhase("starting"); api .startOAuthLogin(provider.id) diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 603e2638a8..657c003eb6 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -534,6 +534,8 @@ export const en: Translations = { copyCliCommand: "Copy CLI command (for external / fallback)", connect: "Connect", sessionExpires: "Session expires in {time}", + sessionExpiredNoError: + "Sign-in expired without reaching the provider. This usually means the sign-in page stalled in the opened tab (server-side issue) — finish signing in there, then click Retry. If it keeps failing, use an API key or the CLI fallback instead.", initiatingLogin: "Initiating login flow…", exchangingCode: "Exchanging code for tokens…", connectedClosing: "Connected! Closing…", diff --git a/web/src/i18n/types.ts b/web/src/i18n/types.ts index 0fee160c5b..9e7080ec55 100644 --- a/web/src/i18n/types.ts +++ b/web/src/i18n/types.ts @@ -551,6 +551,7 @@ export interface Translations { copyCliCommand: string; connect: string; sessionExpires: string; + sessionExpiredNoError: string; initiatingLogin: string; exchangingCode: string; connectedClosing: string;