From e4ef2fa3d6e198100dea3c94d0f469024b213590 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 27 Sep 2026 04:56:48 +0200 Subject: [PATCH] fix: keep an abandoned pairing re-auth from breaking the next sign-in The Link Mobile App step-up marked the tab before asking the server to start the provider round trip. When that request failed, or the user backed out at the provider, the mark stayed, and the next sign-in's callback in that tab went to the pairing route and failed. The mark is now set only once the round trip starts, holds that round trip's state, and the callback drops one that does not match. --- app/(main)/[locale]/auth/callback/page.tsx | 6 +++- .../__tests__/link-device-section.test.tsx | 20 +++++++++++-- .../settings/account-security-settings.tsx | 30 +++++++++++-------- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/app/(main)/[locale]/auth/callback/page.tsx b/app/(main)/[locale]/auth/callback/page.tsx index fafb122bc..380ac321d 100644 --- a/app/(main)/[locale]/auth/callback/page.tsx +++ b/app/(main)/[locale]/auth/callback/page.tsx @@ -44,9 +44,13 @@ function OAuthCallbackInner() { // via prompt=login. Don't create a login session — just confirm the fresh // auth (sets the short-lived pairing proof cookie) and bounce back to the // Security settings, where the QR generation auto-resumes. + // The flag holds the state the step-up started with; one left behind by an + // abandoned round trip is dropped here instead of capturing this login. let pairReauthResume = false; try { - pairReauthResume = sessionStorage.getItem("pair_reauth_resume") === "1"; + const resumeState = sessionStorage.getItem("pair_reauth_resume"); + pairReauthResume = !!state && resumeState === state; + if (resumeState !== null && !pairReauthResume) sessionStorage.removeItem("pair_reauth_resume"); } catch { /* sessionStorage unavailable */ } if (pairReauthResume && state) { try { sessionStorage.removeItem("pair_reauth_resume"); } catch { /* ignore */ } diff --git a/components/settings/__tests__/link-device-section.test.tsx b/components/settings/__tests__/link-device-section.test.tsx index 019b9df38..459757d24 100644 --- a/components/settings/__tests__/link-device-section.test.tsx +++ b/components/settings/__tests__/link-device-section.test.tsx @@ -55,6 +55,7 @@ const ERR = (status: number, error: string): Reply => ({ status, body: { error } let createReplies: Reply[] = []; let pairStatus = 'pending'; +let ssoStartStatus = 200; function reply({ status, body }: Reply) { return { ok: status >= 200 && status < 300, status, json: async () => body }; @@ -121,6 +122,7 @@ beforeEach(() => { vi.clearAllMocks(); createReplies = []; pairStatus = 'pending'; + ssoStartStatus = 200; mocks.lite.value = false; mocks.oauthEnabled.value = false; mocks.apiFetch.mockImplementation(async (url: string) => { @@ -129,7 +131,7 @@ beforeEach(() => { } if (url.startsWith('/api/auth/pair/status')) return reply({ status: 200, body: { status: pairStatus } }); // A fragment-only URL keeps jsdom from attempting a real navigation. - if (url === '/api/auth/sso/start') return reply({ status: 200, body: { authorize_url: '#idp' } }); + if (url === '/api/auth/sso/start') return reply({ status: ssoStartStatus, body: { authorize_url: '#idp', state: 'sso-state-1' } }); throw new Error(`unexpected fetch ${url}`); }); sessionStorage.clear(); @@ -152,7 +154,19 @@ describe('LinkDeviceSection step-up', () => { fireEvent.click(await screen.findByRole('button', { name: 'link_device.use_sso' })); await waitFor(() => expect(ssoBodies()).toHaveLength(1)); expect(ssoBodies()[0]).toMatchObject({ purpose: 'reauth', slot: 2 }); - expect(sessionStorage.getItem('pair_reauth_resume')).toBe('1'); + expect(sessionStorage.getItem('pair_reauth_resume')).toBe('sso-state-1'); + }); + + it('leaves no re-auth flag behind when the IdP round trip cannot start', async () => { + mocks.oauthEnabled.value = true; + ssoStartStatus = 500; + createReplies = [ERR(401, 'reauth_required')]; + render(); + clickGenerate(); + + fireEvent.click(await screen.findByRole('button', { name: 'link_device.use_sso' })); + expect(await screen.findByRole('alert')).toHaveTextContent('link_device.error'); + expect(sessionStorage.getItem('pair_reauth_resume')).toBeNull(); }); it('shows no single sign-on option without it', async () => { @@ -224,7 +238,7 @@ describe('LinkDeviceSection step-up', () => { locale: 'en', redirect_uri: `${origin()}/en/auth/callback`, }); - expect(sessionStorage.getItem('pair_reauth_resume')).toBe('1'); + expect(sessionStorage.getItem('pair_reauth_resume')).toBe('sso-state-1'); expect(screen.queryByLabelText('password.current')).toBeNull(); }); diff --git a/components/settings/account-security-settings.tsx b/components/settings/account-security-settings.tsx index 78f1b0f42..eea8db5fb 100644 --- a/components/settings/account-security-settings.tsx +++ b/components/settings/account-security-settings.tsx @@ -1041,26 +1041,30 @@ export function LinkDeviceSection() { // callback page sets the pairing re-auth proof and bounces back here, where // the resume effect below calls create() again. const startReauth = useCallback(async () => { - try { - sessionStorage.setItem('pair_reauth_resume', '1'); - } catch { /* sessionStorage unavailable */ } const prefix = getPathPrefix(locale); const redirectUri = `${window.location.origin}${prefix}/${locale}/auth/callback`; // The slot lets the server re-authenticate against this account's own // server entry (its IdP), read from the slot's server cookie. const slot = useAccountStore.getState().getActiveAccount()?.cookieSlot ?? 0; - const res = await apiFetch('/api/auth/sso/start', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - credentials: 'include', - body: JSON.stringify({ redirect_uri: redirectUri, locale, purpose: 'reauth', slot }), - }); - if (!res.ok) { + try { + const res = await apiFetch('/api/auth/sso/start', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + credentials: 'include', + body: JSON.stringify({ redirect_uri: redirectUri, locale, purpose: 'reauth', slot }), + }); + if (!res.ok) throw new Error(`sso/start answered ${res.status}`); + const { authorize_url, state } = await res.json(); + if (typeof authorize_url !== 'string' || typeof state !== 'string') throw new Error('sso/start answered no URL'); + // The callback takes the re-auth route only for this state: a flag left + // behind by an abandoned round trip must not capture a later login. + try { + sessionStorage.setItem('pair_reauth_resume', state); + } catch { /* sessionStorage unavailable */ } + window.location.href = authorize_url; + } catch { if (aliveRef.current) setMessage({ text: t('link_device.error'), tone: 'error' }); - return; } - const { authorize_url } = await res.json(); - window.location.href = authorize_url; }, [locale, t]); const create = useCallback(async ({ password: pw, totp: code, fromResume = false }: PairCreateOptions = {}) => {