From b046055ff171559153cdf8819c615b3885088dbd Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 20:48:27 +0000 Subject: [PATCH 01/11] Fix sign-in links, the OAuth callback and per-account state (T2 auth batch) Google and the emailed sign-in link now return to /auth/callback, which holds a loading screen until the code exchange settles and then opens the deep link or /login. PKCE flow ids are appended to redirects and passed to the exchange, so a failed resend or another tab's sign-in no longer deletes the verifier a pending link needs. Auth errors in the callback URL and failed exchanges show a localized message on /login instead of a console warning. Links already in inboxes (/?code=, /app?code=) are moved onto the callback route before the first render. Sign-out uses local scope. After a sign-out, /login returns only the same account to the page it left. The onboarding tour is tracked per account, and on phones it no longer blocks the header menu. The local confirmation email template is code-only with the code out of the subject. Fixes AUTH-01, AUTH-18, AUTH-17, AUTH-02, AUTH-19, AUTH-06, AUTH-07, AUTH-13. Co-Authored-By: Claude Opus 5.5 --- .../verify-debatelab/features/README.md | 2 +- .../features/auth-demo-login.md | 8 +- .../verify-debatelab/recipes/auth.spec.ts | 161 ++++++++++++++++-- docs/architecture.md | 2 + e2e/login.spec.ts | 11 +- public/theme-init.js | 2 +- src/App.tsx | 25 ++- src/components/auth/RequireAuth.tsx | 13 +- .../features/onboarding/OnboardingTour.tsx | 34 +++- src/hooks/use-onboarding.ts | 40 ++++- src/lib/auth-callback.ts | 79 +++++++++ src/lib/native.ts | 4 - src/lib/route-modules.ts | 1 + src/lib/seo/use-page-head.ts | 2 +- src/lib/supabase/AuthProvider.tsx | 91 +++++----- src/lib/supabase/auth-context.ts | 9 +- src/lib/supabase/client.ts | 4 + src/locales/en.json | 6 +- src/locales/es.json | 6 +- src/locales/hu.json | 6 +- src/main.tsx | 7 + src/pages/AuthCallbackPage.tsx | 29 ++++ src/pages/LoginPage.tsx | 38 ++++- src/prerender/entry.tsx | 2 + src/test/auth/AuthProvider.behavior.test.tsx | 130 ++++++++++++-- src/test/auth/account-switch-route.test.tsx | 64 +++++++ src/test/auth/auth-callback.test.ts | 76 +++++++++ src/test/auth/onboarding-per-account.test.ts | 45 +++++ src/test/vercel-deployment-config.test.ts | 4 +- supabase/config.toml | 7 + supabase/templates/confirmation.html | 6 + vercel.json | 13 ++ 32 files changed, 818 insertions(+), 109 deletions(-) create mode 100644 src/lib/auth-callback.ts create mode 100644 src/pages/AuthCallbackPage.tsx create mode 100644 src/test/auth/account-switch-route.test.tsx create mode 100644 src/test/auth/auth-callback.test.ts create mode 100644 src/test/auth/onboarding-per-account.test.ts create mode 100644 supabase/templates/confirmation.html diff --git a/.claude/skills/verify-debatelab/features/README.md b/.claude/skills/verify-debatelab/features/README.md index 012a5abbd..340ad2948 100644 --- a/.claude/skills/verify-debatelab/features/README.md +++ b/.claude/skills/verify-debatelab/features/README.md @@ -39,7 +39,7 @@ Each feature file starts with an H1 title and one paragraph describing the user- ## Features -- [Demo-code sign-in](./auth-demo-login.md): sign-in, reload, rejected code, sign-out (`e2e/full-stack/auth.spec.ts`), and email one-time-code sign-in through Mailpit (`recipes/auth.spec.ts`). +- [Demo-code sign-in](./auth-demo-login.md): sign-in, reload, rejected code, sign-out (`e2e/full-stack/auth.spec.ts`); email one-time-code sign-in through Mailpit, the emailed sign-in link through `/auth/callback` (including after a failed resend, and a used link), and a second account on the same browser (`recipes/auth.spec.ts`). - [Case generation](./case-generation.md): generate, web research, save and reopen from History on a fresh device, PDF and DOCX export, single-section regeneration, provider failure and retry, a reply cut off at its length cap, quota block (`recipes/case-generation.spec.ts`). - [Library sources](./library-sources.md): add, pause, delete, grounding, and URL import (`recipes/library.spec.ts`). - [AI tools](./ai-tools.md): Attack, Brainstorm, Factoids, and Ask streamed, cut off at its length cap, cited, and with web search (`recipes/ai-tools.spec.ts`). diff --git a/.claude/skills/verify-debatelab/features/auth-demo-login.md b/.claude/skills/verify-debatelab/features/auth-demo-login.md index e5ce02605..c6c94c13d 100644 --- a/.claude/skills/verify-debatelab/features/auth-demo-login.md +++ b/.claude/skills/verify-debatelab/features/auth-demo-login.md @@ -10,6 +10,8 @@ A visitor signs in with a demo access code on the Login page. The `demo-code-log - `auth-signout` signs out from Settings, after which `/app/generate` redirects to `/login`. - `auth-refresh` refreshes an expired access token before an AI request. - `auth-session-expired` signs out a session the server revoked: the stored tokens are refused, so the app goes to `/login` and shows `Your session expired. Sign in again.` once (`recipes/auth.spec.ts`). +- `auth-email-link` opens the sign-in link from the email after a resend failed with 429. It lands on `/app` through `/auth/callback` without showing `/login` on the way, and the same link opened again shows `This sign-in link has expired or was already used. Send a new code.` on `/login` (`recipes/auth.spec.ts`). +- `auth-switch-account` signs out from Settings (the logout request has `scope=local`) and signs a second account in on the same `/login` page. It lands on `/app`, not on Settings, and gets its own onboarding tour. On phones the menu still opens and navigates while the tour shows (`recipes/auth.spec.ts`). ## How to get to it (user POV) @@ -27,11 +29,13 @@ Preconditions: - **Persist.** Reload the page. The URL stays `/app`. On `/app/settings`, the page shows `user.email`. - **Reject.** On a signed-out page, submit `DEMO-ZZZZ-ZZZZ`. The element with role `alert` has the text `Demo code not found.`, and the URL still matches `/login`. - **Sign out.** On `/app/settings`, click the button named `Sign out`, then go to `/app/generate`. The URL matches `/login`. +- **Email link.** Local email templates are code-only, so the recipe builds the link prod's email carries: `${LOCAL_SUPABASE_URL}/auth/v1/verify?token=pkce_&type=magiclink&redirect_to=`. The `redirect_to` is `http://127.0.0.1:18081/auth/callback?sb_flow_id=<32 hex>`. Route `**/auth/v1/otp**` to a 429 before clicking `Resend code` to reproduce the failed resend. +- **Switch account.** After `Sign out`, sign the second account in on the same `/login` page. `page.goto("/login")` drops the history state that the bug depended on. Create the second user with `createDemoUser` and delete it in `finally`. - **Revoked session.** Signed in on `/app`, replace `access_token` in the `debate-auth-local` localStorage entry with a JWT the server refuses, `refresh_token` with `x`, and `expires_at` with now + 3000 s, then reload. Within a few seconds the URL matches `/login` and a `status` element says `Your session expired. Sign in again.`. A second reload no longer shows it. ## Gotchas -- A first sign-in on a new account starts the onboarding tour on `/app`. Its first step (`Welcome to DebateLab!`, `Start your first debate here.`) offers `Start debate`. The fixtures pre-seed `debate-trainer-onboarding` to skip it. Interactive runs click `Skip Tour` (`[data-test-id="button-skip"]`). The tour itself is covered in [current-debate](./current-debate.md). +- A first sign-in on a new account starts the onboarding tour on `/app`. Its first step (`Welcome to DebateLab!`, `Start your first debate here.`) offers `Start debate`. The fixtures pre-seed `debate-trainer-onboarding` to skip it. The tour is tracked per account (store version 1, `completedUserIds`). A version 0 seed with `hasCompletedOnboarding: true` covers only the next account that signs in, so a second account in the same browser sees the tour. Interactive runs click `Skip Tour` (`[data-test-id="button-skip"]`). The tour itself is covered in [current-debate](./current-debate.md). - Sessions are stored under the `debate-auth-local` localStorage key in local-preview mode. Don't reuse stub-session helpers (`e2e/helpers/auth.ts`). They fake auth and prove rendering only. -- Google sign-in is not wired to the local stack. Only demo codes work here. +- Google sign-in is not wired to the local stack. Demo codes, email codes and the email link work here. - The revoked-session check forces one token refresh before it signs out. An Auth outage (network error, 5xx, 429) keeps the session, and that path is covered by `src/test/auth/session-expiry.test.ts`, not by a recipe. diff --git a/.claude/skills/verify-debatelab/recipes/auth.spec.ts b/.claude/skills/verify-debatelab/recipes/auth.spec.ts index 8a2b551ad..e13607fb0 100644 --- a/.claude/skills/verify-debatelab/recipes/auth.spec.ts +++ b/.claude/skills/verify-debatelab/recipes/auth.spec.ts @@ -1,26 +1,20 @@ -import { expect, test } from "./fixtures"; +import { createHash } from "node:crypto"; +import type { APIRequestContext, Page } from "@playwright/test"; +import { createDemoUser, debateControl, deleteDemoUser, expect, test } from "./fixtures"; -// Feature map: features/auth.md. Demo-code sign-in, reload, rejection, and +// Feature map: features/auth-demo-login.md. Demo-code sign-in, reload, rejection, and // sign-out are covered by e2e/full-stack/auth.spec.ts, which drive also runs. const mailer = process.env.WORKSPACE_TEST_MAILER_URL; +const supabaseUrl = process.env.LOCAL_SUPABASE_URL; -test("auth-email-otp: an emailed one-time code signs an existing account in", async ({ page, user, request, evidence }) => { - test.skip(!mailer, "WORKSPACE_TEST_MAILER_URL is missing; run through verify.sh drive"); - const sentAfter = Date.now(); - await page.goto("/login"); - await page.getByRole("tab", { name: "Email" }).click(); - await page.getByLabel("Email address").fill(user.email); - await page.getByRole("button", { name: "Send code" }).click(); - await expect(page.getByText("Code sent").first()).toBeVisible(); - await evidence.shot("01-code-sent"); - - // Read the real message from the local Mailpit inbox. +/** Reads the six-digit code sent to `email` after `sentAfter` from the local Mailpit inbox. */ +async function readOtp(request: APIRequestContext, email: string, sentAfter: number) { let otp = ""; await expect.poll(async () => { const payload = await (await request.get(`${mailer}/api/v1/messages`)).json() as { messages?: Array> }; for (const message of payload.messages ?? []) { - if (!JSON.stringify(message).includes(user.email)) continue; + if (!JSON.stringify(message).includes(email)) continue; if (Date.parse(String(message.Created ?? 0)) < sentAfter - 5_000) continue; const detail = await (await request.get(`${mailer}/api/v1/message/${message.ID}`)).text(); otp = detail.match(/\b(\d{6})\b/)?.[1] ?? ""; @@ -28,6 +22,42 @@ test("auth-email-otp: an emailed one-time code signs an existing account in", as } return otp; }, { timeout: 30_000 }).toMatch(/^\d{6}$/); + return otp; +} + +/** Every path this tab shows, kept across reloads, so a flash of /login is caught. */ +async function recordPaths(page: Page) { + await page.addInitScript(() => { + const record = () => { + const seen = JSON.parse(sessionStorage.getItem("verify-paths") ?? "[]") as string[]; + sessionStorage.setItem("verify-paths", JSON.stringify([...seen, location.pathname])); + }; + for (const method of ["pushState", "replaceState"] as const) { + const original = history[method].bind(history); + history[method] = (...args: Parameters) => { + original(...args); + record(); + }; + } + record(); + }); + return { + clear: () => page.evaluate(() => sessionStorage.removeItem("verify-paths")), + seen: () => page.evaluate(() => JSON.parse(sessionStorage.getItem("verify-paths") ?? "[]") as string[]), + }; +} + +test("auth-email-otp: an emailed one-time code signs an existing account in", async ({ page, user, request, evidence }) => { + test.skip(!mailer, "WORKSPACE_TEST_MAILER_URL is missing; run through verify.sh drive"); + const sentAfter = Date.now(); + await page.goto("/login"); + await page.getByRole("tab", { name: "Email" }).click(); + await page.getByLabel("Email address").fill(user.email); + await page.getByRole("button", { name: "Send code" }).click(); + await expect(page.getByText("Code sent").first()).toBeVisible(); + await evidence.shot("01-code-sent"); + + const otp = await readOtp(request, user.email, sentAfter); await evidence.step("Read the one-time code from Mailpit"); await page.getByRole("textbox", { name: "Verification code" }).fill(otp); @@ -66,3 +96,106 @@ test("auth-session-expired: a session the server revoked signs out to the login await page.reload(); await expect(page.getByText("Your session expired. Sign in again.")).toHaveCount(0); }); + +test("auth-email-link: the emailed sign-in link signs in after a failed resend, and a used link says so", async ({ page, user, request, evidence }) => { + test.skip(!mailer || !supabaseUrl, "Mailpit or LOCAL_SUPABASE_URL is missing; run through verify.sh drive"); + const paths = await recordPaths(page); + const sentAfter = Date.now(); + await page.goto("/login"); + await page.getByRole("tab", { name: "Email" }).click(); + await page.getByLabel("Email address").fill(user.email); + const otpRequest = page.waitForRequest((request) => request.url().includes("/auth/v1/otp")); + await page.getByRole("button", { name: "Send code" }).click(); + // The link returns to the callback route with this sign-in's own flow id. + const redirectTo = new URL((await otpRequest).url()).searchParams.get("redirect_to") ?? ""; + expect(redirectTo).toMatch(/^http:\/\/127\.0\.0\.1:18081\/auth\/callback\?sb_flow_id=[0-9a-f]{32}$/); + await expect(page.getByText("Code sent").first()).toBeVisible(); + const otp = await readOtp(request, user.email, sentAfter); + await evidence.step(`Sent a code; the link returns to ${redirectTo.replace(/sb_flow_id=.*/, "sb_flow_id=")}`); + + // AUTH-01: the resend hits Auth's rate limit, and auth-js drops that resend's verifier. + await page.route("**/auth/v1/otp**", (route) => route.fulfill({ + status: 429, + contentType: "application/json", + body: JSON.stringify({ code: 429, error_code: "over_email_send_rate_limit", msg: "email rate limit exceeded" }), + })); + await page.getByRole("button", { name: "Resend code" }).click(); + await expect(page.getByText("Too many attempts. Try again later.")).toBeVisible(); + await page.unroute("**/auth/v1/otp**"); + await evidence.shot("01-resend-rate-limited"); + + // The link in prod's email: Auth's /verify with the PKCE token hash of this code. + const link = `${supabaseUrl}/auth/v1/verify?${new URLSearchParams({ + token: `pkce_${createHash("sha224").update(user.email + otp).digest("hex")}`, + type: "magiclink", + redirect_to: redirectTo, + })}`; + await paths.clear(); + await page.goto(link); + await expect(page).toHaveURL(/\/app$/, { timeout: 20_000 }); + await expect(debateControl(page)).toBeVisible(); + // AUTH-17: the callback waits for the exchange; it never shows /login on the way. + const seen = await paths.seen(); + await evidence.json("02-paths-after-link", seen); + expect(seen).toContain("/auth/callback"); + expect(seen).not.toContain("/login"); + await page.reload(); + await expect(page).toHaveURL(/\/app$/); + await evidence.shot("02-signed-in-by-link"); + await evidence.step("Opened the emailed link after the failed resend; signed in on /app without passing /login"); + + // AUTH-02: the same link again, in a browser that is no longer signed in. + await page.evaluate(() => { + for (const key of Object.keys(localStorage)) if (/^debate-auth-[a-z]+$/.test(key)) localStorage.removeItem(key); + }); + await page.goto(link); + await expect(page).toHaveURL(/\/login$/, { timeout: 20_000 }); + await expect(page.getByRole("alert")).toHaveText("This sign-in link has expired or was already used. Send a new code."); + await evidence.shot("03-used-link-message"); + await evidence.aria("03-used-link-message"); + await evidence.step("Opened the used link again; /login says it expired or was used"); +}); + +test("auth-switch-account: the next account on this browser starts on /app, gets its own tour, and sign-out stays local", async ({ signedIn: page, evidence }, testInfo) => { + const second = await createDemoUser(`${testInfo.project.name}-second`); + try { + // The first account (tour already seen) leaves off on Settings and signs out. + await page.goto("/app/settings"); + const logout = page.waitForRequest((request) => request.url().includes("/auth/v1/logout")); + await page.getByRole("button", { name: "Sign out" }).click(); + // AUTH-19: this device only. + expect(new URL((await logout).url()).searchParams.get("scope")).toBe("local"); + await expect(page).toHaveURL(/\/login$/); + await evidence.step("Signed out from /app/settings with scope=local"); + + // AUTH-06: a different account signs in on the same /login page. + await page.getByRole("tab", { name: "Demo code" }).click(); + await page.getByRole("textbox", { name: "Demo code" }).fill(second.code); + await page.getByRole("button", { name: "Sign in with demo code" }).click(); + await expect(page).toHaveURL(/\/app$/, { timeout: 20_000 }); + await evidence.step("Signed in as a second account; landed on /app, not /app/settings"); + + // AUTH-07: the second account gets its own tour. + const tour = page.getByText("Welcome to DebateLab!"); + await expect(tour).toBeVisible({ timeout: 10_000 }); + await evidence.shot("01-second-account-tour"); + if (await page.getByRole("button", { name: "Open menu" }).isVisible()) { + // On phones the tour must not block the header's menu: open it and use it. + await page.getByRole("button", { name: "Open menu" }).click(); + const menu = page.getByRole("navigation", { name: "Mobile navigation" }); + await expect(menu).toBeVisible(); + await evidence.shot("02-menu-open-during-tour"); + await menu.getByRole("link", { name: /History/ }).click(); + await expect(page).toHaveURL(/\/app\/history$/); + await evidence.step("Opened the menu and went to History while the tour showed"); + // The tour waits on /app and picks up again there. + await page.goBack(); + await expect(tour).toBeVisible({ timeout: 10_000 }); + } + await page.locator('[data-test-id="button-skip"]').click(); + await expect(tour).toBeHidden(); + await evidence.step("Skipped the tour"); + } finally { + await deleteDemoUser(second); + } +}); diff --git a/docs/architecture.md b/docs/architecture.md index b4d6b739e..7d48b9db8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -130,6 +130,8 @@ Admin-authored rows such as promo codes, curated sources, announcements, and gra Supabase Auth owns user sessions. Protected routes provide a client navigation boundary, but server authorization is decisive. - Google OAuth, six-digit email OTP, and demo-code token-hash handoffs all finish as ordinary Supabase sessions and use the same store hydration path. +- Google and any emailed sign-in link return to `/auth/callback` (with `?next=/app/...` for a deep link; only `/app` paths are accepted). [`AuthProvider`](../src/lib/supabase/AuthProvider.tsx) exchanges the code there and keeps `loading` true until the exchange settles. The client sets `experimental.appendPkceFlowIdToRedirects`, so each sign-in keeps its own PKCE verifier and its redirect carries `sb_flow_id`; another tab's sign-in or a failed resend no longer breaks a pending link. An `error`/`error_code` from Auth, or a failed exchange, opens `/login` with a localized message. Links that still point at `/?code=` or `/app?code=` are moved onto the callback route before the first render ([`src/lib/auth-callback.ts`](../src/lib/auth-callback.ts)). Each project's Auth redirect allowlist needs `/auth/callback**`; a redirect to the Site URL's own host passes without it. +- Sign-out ends only this device's session (`scope: 'local'`). After a sign-out, `/login` returns only the same account to the page it left; another account starts on `/app`. The onboarding tour is tracked per account on each browser. - RLS owns row visibility and mutation rights. - Shared backend helpers validate bearer tokens and admin roles. - Admin endpoints verify server-side roles. diff --git a/e2e/login.spec.ts b/e2e/login.spec.ts index 659677ba1..7af8d8f1a 100644 --- a/e2e/login.spec.ts +++ b/e2e/login.spec.ts @@ -11,7 +11,8 @@ test.describe('login passwordless flows', { tag: '@smoke' }, () => { await page.goto('/login', { waitUntil: 'domcontentloaded' }); const googleButton = page.getByRole('button', { name: 'Continue with Google' }); - const expectedRedirect = `${new URL(page.url()).origin}/app`; + // Google returns to the callback route with this sign-in's own PKCE flow id. + const expectedRedirect = new RegExp(`^${new URL(page.url()).origin.replace(/[.]/g, '\\.')}/auth/callback\\?sb_flow_id=[0-9a-f]{32}$`); await expect(googleButton).toBeVisible(); await expect(page.getByRole('tab', { name: 'Email' })).toBeVisible(); await expect(page.getByRole('tab', { name: 'Demo code' })).toBeVisible(); @@ -20,7 +21,7 @@ test.describe('login passwordless flows', { tag: '@smoke' }, () => { await googleButton.click(); const requestUrl = new URL((await requestPromise).url()); expect(requestUrl.searchParams.get('provider')).toBe('google'); - expect(requestUrl.searchParams.get('redirect_to')).toBe(expectedRedirect); + expect(requestUrl.searchParams.get('redirect_to')).toMatch(expectedRedirect); }); test('exchanges a demo-code magic-link hash for a Supabase session', async ({ page }) => { @@ -55,9 +56,12 @@ test.describe('login passwordless flows', { tag: '@smoke' }, () => { test('sends an email code and focuses the verification field', async ({ page }) => { let requestEmail: string | undefined; + let redirectTo: string | null = null; let releaseOtpResponse: (() => void) | undefined; const otpResponseReleased = new Promise((resolve) => { releaseOtpResponse = resolve; }); - await page.route('**/auth/v1/otp', async (route) => { + // The request carries `?redirect_to=`, the callback the emailed link opens. + await page.route('**/auth/v1/otp**', async (route) => { + redirectTo = new URL(route.request().url()).searchParams.get('redirect_to'); const body = route.request().postDataJSON() as { email?: unknown }; requestEmail = typeof body.email === 'string' ? body.email : undefined; await otpResponseReleased; @@ -78,6 +82,7 @@ test.describe('login passwordless flows', { tag: '@smoke' }, () => { await expect(page.getByText('Code sent', { exact: true })).toBeVisible(); await expect(page.getByRole('textbox', { name: 'Verification code' })).toBeFocused(); expect(requestEmail).toBe('student@example.com'); + expect(redirectTo).toMatch(/\/auth\/callback\?sb_flow_id=[0-9a-f]{32}$/); }); test('shows a demo-code backend error while keeping the form editable', async ({ page }) => { diff --git a/public/theme-init.js b/public/theme-init.js index e56ec9bb0..b90a8ba25 100644 --- a/public/theme-init.js +++ b/public/theme-init.js @@ -20,7 +20,7 @@ try { var pathname = window.location.pathname; - if (pathname === '/app' || pathname.indexOf('/app/') === 0 || pathname === '/login') { + if (pathname === '/app' || pathname.indexOf('/app/') === 0 || pathname === '/login' || pathname === '/auth/callback') { document.documentElement.classList.add('app-shell'); } } catch { diff --git a/src/App.tsx b/src/App.tsx index 432da506b..ee8980297 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -11,7 +11,7 @@ import { RequireAuth } from "@/components/auth/RequireAuth"; import { AccountQueryCache } from "@/components/auth/AccountQueryCache"; import { ThemeProvider } from "@/components/ThemeProvider"; import { useSessionStorageSync } from "@/hooks/use-session-storage-sync"; -import { useOnboardingStore } from "@/hooks/use-onboarding"; +import { hasCompletedOnboarding, useOnboardingStore } from "@/hooks/use-onboarding"; import { OfflineIndicator } from "@/components/OfflineIndicator"; import { StagingBanner } from "@/components/StagingBanner"; import { captureException, captureProductEvent, toTelemetrySafeError } from "@/lib/observability"; @@ -21,6 +21,7 @@ import { usePrivateRouteHead } from "@/lib/seo/use-page-head"; import { RouteSkeleton, type RouteSkeletonVariant } from "@/components/loading/RouteSkeleton"; import { useTranslation } from "react-i18next"; import { COLLABORATION_URL } from "@/lib/workspace/collaboration-config"; +import { AUTH_CALLBACK_PATH } from "@/lib/auth-callback"; /** * A page the build prerenders. mountApp loads its module before the first @@ -60,6 +61,7 @@ const AdminPage = lazy(routeLoaders["/app/admin"]); const SharedPage = lazy(routeLoaders["/share/:code"]); const FlowchartPage = lazy(routeLoaders["/app/flowchart"]); const LoginPage = lazy(routeLoaders["/login"]); +const AuthCallbackPage = lazy(routeLoaders["/auth/callback"]); const SourcesPage = lazy(routeLoaders["/expocit/sources"]); const NotFound = prerenderedPage("*"); const ToolsHubPage = prerenderedPage("/tools"); @@ -119,6 +121,7 @@ const ROUTE_FALLBACKS: Record = { "/privacy": { variant: "public", label: "Privacy", labelKey: "footer.privacy" }, "/expocit/sources": { variant: "public", label: "Sources", labelKey: "crossfire.sources" }, "/login": { variant: "public", label: "Sign in", labelKey: "login.title" }, + "/auth/callback": { variant: "public", label: "Sign in", labelKey: "login.title" }, "/app": { variant: "home-grid", label: "Tools", labelKey: "nav.tools" }, "/app/debate": { variant: "settings/list", label: "Current debate", labelKey: "nav.currentDebate" }, "/app/workspaces": { variant: "settings/list", label: "Workspaces", labelKey: "nav.workspaces" }, @@ -231,21 +234,28 @@ function RouteContent({ children }: { children: ReactNode }) { function OptionalOnboardingTour() { const location = useLocation(); - const hasCompletedOnboarding = useOnboardingStore((state) => state.hasCompletedOnboarding); + const userId = useAuth().user?.id; + const completed = useOnboardingStore((state) => hasCompletedOnboarding(state, userId)); const isTourRunning = useOnboardingStore((state) => state.isTourRunning); - const shouldLoadTour = isTourRunning || (location.pathname === "/app" && !hasCompletedOnboarding); + const claimLegacyCompletion = useOnboardingStore((state) => state.claimLegacyCompletion); + const shouldLoadTour = isTourRunning || (location.pathname === "/app" && !completed); - if (!shouldLoadTour) return null; + useEffect(() => { + if (userId) claimLegacyCompletion(userId); + }, [claimLegacyCompletion, userId]); + + if (!userId || !shouldLoadTour) return null; - return ; + // Keyed by account, so one account's half-finished tour never carries into the next. + return ; } -function TimedOnboardingTour() { +function TimedOnboardingTour({ userId }: { userId: string }) { const [eligibleAt] = useState(() => typeof performance === "undefined" ? 0 : performance.now()); return ( - + ); } @@ -280,6 +290,7 @@ function AppRoutes() { } /> } /> } /> + } /> } /> } /> } /> diff --git a/src/components/auth/RequireAuth.tsx b/src/components/auth/RequireAuth.tsx index 22e876317..a82ed1c3e 100644 --- a/src/components/auth/RequireAuth.tsx +++ b/src/components/auth/RequireAuth.tsx @@ -1,3 +1,4 @@ +import { useRef } from 'react'; import { Navigate, useLocation } from 'react-router-dom'; import { useAuth } from '@/lib/supabase/AuthProvider'; import { Loader2 } from 'lucide-react'; @@ -6,6 +7,10 @@ import { consumeWorkspaceInvite, stashWorkspaceInvite } from '@/lib/workspace/in export function RequireAuth({ children }: { children: React.ReactNode }) { const { user, loading } = useAuth(); const location = useLocation(); + // The account this layout last showed. A sign-out here stamps the redirect + // with it, so LoginPage returns only that account to the page it left. + const lastUserId = useRef(undefined); + if (user) lastUserId.current = user.id; if (loading) { return ( @@ -17,7 +22,13 @@ export function RequireAuth({ children }: { children: React.ReactNode }) { if (!user) { const invite = stashWorkspaceInvite(location); - return ; + return ( + + ); } const invite = location.pathname === "/app/workspaces/invite" diff --git a/src/components/features/onboarding/OnboardingTour.tsx b/src/components/features/onboarding/OnboardingTour.tsx index b3b09f6bb..5e235f0ef 100644 --- a/src/components/features/onboarding/OnboardingTour.tsx +++ b/src/components/features/onboarding/OnboardingTour.tsx @@ -4,12 +4,33 @@ import { useTranslation } from 'react-i18next'; import Joyride, { ACTIONS, CallBackProps, EVENTS, STATUS, Step } from 'react-joyride'; import { useLocation } from 'react-router-dom'; import { Button } from '@/components/ui/button'; -import { useOnboardingStore } from '@/hooks/use-onboarding'; +import { hasCompletedOnboarding as completedFor, useOnboardingStore } from '@/hooks/use-onboarding'; import { useDebateStore } from '@/lib/stores/debate-store'; -export function OnboardingTour({ eligibleAt }: { eligibleAt?: number } = {}) { +// Below `lg` the header's menu button is the navigation. There the tour draws no +// overlay, so a tap on the menu (or anything else) still works while it shows, +// and its tooltip sits under the header (z-50) and the open menu and its scrim +// (z-40). Joyride draws the tooltip at zIndex + 100. +const COMPACT_LAYOUT_QUERY = '(max-width: 1023px)'; +const COMPACT_TOOLTIP_Z_INDEX = 35; + +function useCompactLayout() { + const [compact, setCompact] = useState(() => typeof window !== 'undefined' && window.matchMedia?.(COMPACT_LAYOUT_QUERY).matches === true); + useEffect(() => { + const query = window.matchMedia?.(COMPACT_LAYOUT_QUERY); + if (!query) return; + const update = () => setCompact(query.matches); + query.addEventListener('change', update); + return () => query.removeEventListener('change', update); + }, []); + return compact; +} + +export function OnboardingTour({ eligibleAt, userId }: { eligibleAt?: number; userId: string }) { const { t } = useTranslation(); - const { isTourRunning, hasCompletedOnboarding, setOnboardingComplete, startTour, stopTour } = useOnboardingStore(); + const hasCompletedOnboarding = useOnboardingStore((state) => completedFor(state, userId)); + const { isTourRunning, setOnboardingComplete, startTour, stopTour } = useOnboardingStore(); + const compactLayout = useCompactLayout(); const location = useLocation(); const currentDebateId = useDebateStore((state) => state.currentDebateId); const debateDialog = useDebateStore((state) => state.dialog); @@ -151,12 +172,12 @@ export function OnboardingTour({ eligibleAt }: { eligibleAt?: number } = {}) { const finishedStatuses: string[] = [STATUS.FINISHED, STATUS.SKIPPED]; if (finishedStatuses.includes(status)) { - setOnboardingComplete(); + setOnboardingComplete(userId); setStepIndex(0); } else if (type === EVENTS.STEP_AFTER || type === EVENTS.TARGET_NOT_FOUND) { setStepIndex(index + (action === ACTIONS.PREV ? -1 : 1)); } - }, [setOnboardingComplete]); + }, [setOnboardingComplete, userId]); return ( void; + claimLegacyCompletion: (userId: string) => void; + setOnboardingComplete: (userId: string) => void; startTour: () => void; stopTour: () => void; } +type PersistedOnboarding = Pick; + +const withUser = (userIds: string[], userId: string) => (userIds.includes(userId) ? userIds : [...userIds, userId]); + +export function hasCompletedOnboarding(state: PersistedOnboarding, userId: string | undefined): boolean { + return !userId || state.legacyCompleted || state.completedUserIds.includes(userId); +} + export const useOnboardingStore = create()( persist( (set) => ({ - hasCompletedOnboarding: false, + completedUserIds: [] as string[], + legacyCompleted: false, isTourRunning: false, - setOnboardingComplete: () => set({ hasCompletedOnboarding: true, isTourRunning: false }), + claimLegacyCompletion: (userId) => set((state) => (state.legacyCompleted + ? { legacyCompleted: false, completedUserIds: withUser(state.completedUserIds, userId) } + : state)), + setOnboardingComplete: (userId) => set((state) => ({ + completedUserIds: withUser(state.completedUserIds, userId), + isTourRunning: false, + })), startTour: () => set({ isTourRunning: true }), stopTour: () => set({ isTourRunning: false }), }), { name: 'debate-trainer-onboarding', + version: 1, + // A running tour belongs to this page load, not to the next account. + partialize: ({ completedUserIds, legacyCompleted }): PersistedOnboarding => ({ completedUserIds, legacyCompleted }), + migrate: (persisted, version): PersistedOnboarding => { + if (version === 0) { + const legacy = persisted as { hasCompletedOnboarding?: unknown } | null; + return { completedUserIds: [], legacyCompleted: legacy?.hasCompletedOnboarding === true }; + } + return persisted as PersistedOnboarding; + }, } ) ); diff --git a/src/lib/auth-callback.ts b/src/lib/auth-callback.ts new file mode 100644 index 000000000..8c607dbfb --- /dev/null +++ b/src/lib/auth-callback.ts @@ -0,0 +1,79 @@ +// Google and the emailed sign-in link both return here. AuthProvider exchanges +// the code; AuthCallbackPage waits for it and then opens `next` or /login. +export const AUTH_CALLBACK_PATH = '/auth/callback'; + +/** Why a callback did not sign anyone in. LoginPage shows a message for each. */ +export type AuthCallbackError = 'link-expired' | 'other-browser' | 'failed'; + +export type AuthCallbackParams = { + code: string | null; + flowId: string | null; + error: AuthCallbackError | null; +}; + +// Only in-app pages, as plain path segments: no other origin, scheme, `..` or query. +const NEXT_PATH = /^\/app(?:\/[A-Za-z0-9_-]+)*$/; +// auth-js's own shape for the `sb_flow_id` it appends to redirect URLs. +const FLOW_ID = /^[a-zA-Z0-9_-]{8,64}$/; +const CALLBACK_PARAMS = ['code', 'state', 'error', 'error_code', 'error_description', 'sb_flow_id']; +// Before /auth/callback existed, links returned to / (email) or /app (Google). +const LEGACY_CALLBACK_PATHS = new Set(['/', '/app', '/app/']); + +export function safeNextPath(value: string | null | undefined): string { + return value && NEXT_PATH.test(value) ? value : '/app'; +} + +export function authCallbackUrl(origin: string, next?: string): string { + const target = safeNextPath(next); + const query = target === '/app' ? '' : `?${new URLSearchParams({ next: target })}`; + return `${origin}${AUTH_CALLBACK_PATH}${query}`; +} + +function hashParams(hash: string): URLSearchParams { + return new URLSearchParams(hash.replace(/^#/, '')); +} + +/** + * The callback route for a link that still points at / or /app (one already in + * an inbox, or a Google sign-in started before a deploy). Null for any other URL. + */ +export function legacyCallbackPath(location: { pathname: string; search: string; hash: string }): string | null { + if (!LEGACY_CALLBACK_PATHS.has(location.pathname)) return null; + const query = new URLSearchParams(location.search); + const fragment = hashParams(location.hash); + const isCallback = ['code', 'error', 'error_code'].some((name) => query.has(name) || fragment.has(name)); + return isCallback ? `${AUTH_CALLBACK_PATH}${location.search}${location.hash}` : null; +} + +function errorFromCode(code: string | null | undefined): AuthCallbackError { + if (code === 'pkce_code_verifier_not_found') return 'other-browser'; + if (code === 'otp_expired' || code === 'flow_state_expired' || code === 'flow_state_not_found') return 'link-expired'; + return 'failed'; +} + +export function readAuthCallbackParams(url: URL): AuthCallbackParams { + // PKCE errors come back in the query; implicit-flow errors in the fragment. + const fragment = hashParams(url.hash); + const read = (name: string) => url.searchParams.get(name) ?? fragment.get(name); + const flowId = url.searchParams.get('sb_flow_id'); + const hasError = Boolean(read('error') || read('error_code')); + return { + code: url.searchParams.get('code'), + flowId: flowId && FLOW_ID.test(flowId) ? flowId : null, + error: hasError ? errorFromCode(read('error_code')) : null, + }; +} + +export function authCallbackErrorFor(error: unknown): AuthCallbackError { + const code = typeof error === 'object' && error !== null && 'code' in error ? (error as { code?: unknown }).code : undefined; + return errorFromCode(typeof code === 'string' ? code : undefined); +} + +/** The callback URL without the auth server's parameters. `next` stays. */ +export function withoutCallbackParams(url: URL): string { + const next = new URL(url.href); + for (const name of CALLBACK_PARAMS) next.searchParams.delete(name); + const fragment = hashParams(next.hash); + const hash = fragment.has('error') || fragment.has('error_code') ? '' : next.hash; + return `${next.pathname}${next.search}${hash}`; +} diff --git a/src/lib/native.ts b/src/lib/native.ts index bd69af02a..44caa0fce 100644 --- a/src/lib/native.ts +++ b/src/lib/native.ts @@ -2,10 +2,6 @@ export function getWebAppRedirectUrl(path = '/app'): string { return `${window.location.origin}${path}`; } -export function getOAuthRedirectUrl(): string { - return `${window.location.origin}/app`; -} - const ALLOWED_EXTERNAL_PROTOCOLS = new Set(['http:', 'https:']); function getSafeExternalUrl(url: string): string { diff --git a/src/lib/route-modules.ts b/src/lib/route-modules.ts index 6f285e2a4..79350538a 100644 --- a/src/lib/route-modules.ts +++ b/src/lib/route-modules.ts @@ -28,6 +28,7 @@ export const routeLoaders = { "/privacy": cacheLoader(() => import("@/pages/PrivacyPage")), "/expocit/sources": cacheLoader(() => import("@/pages/SourcesPage")), "/login": cacheLoader(() => import("@/pages/LoginPage")), + "/auth/callback": cacheLoader(() => import("@/pages/AuthCallbackPage")), "/share/:code": cacheLoader(() => import("@/pages/SharedPage")), "/app": cacheLoader(() => import("@/pages/HomePage")), "/app/debate": cacheLoader(() => import("@/pages/CurrentDebatePage")), diff --git a/src/lib/seo/use-page-head.ts b/src/lib/seo/use-page-head.ts index 7e6b15740..4517be1ca 100644 --- a/src/lib/seo/use-page-head.ts +++ b/src/lib/seo/use-page-head.ts @@ -46,7 +46,7 @@ export function usePageHead(route: PrerenderedRoute) { } /** Sign-in, the signed-in app and share links: never indexed (vercel.json sends the same X-Robots-Tag). */ -const PRIVATE_ROUTE = /^\/(login|app|share)(\/|$)/; +const PRIVATE_ROUTE = /^\/(login|auth|app|share)(\/|$)/; /** * Titles each private page " · DebateLab" (the shell's title) and marks it noindex while it diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index 67e55dedd..abdf76d7d 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -9,7 +9,15 @@ import { useSettingsStore } from '@/lib/stores/settings-store'; import { useSparringStore } from '@/lib/stores/sparring-store'; import { useDebateStore } from '@/lib/stores/debate-store'; import { useDebateContextStore } from '@/lib/stores/debate-context-store'; -import { getOAuthRedirectUrl, openExternalUrl } from '@/lib/native'; +import { openExternalUrl } from '@/lib/native'; +import { + AUTH_CALLBACK_PATH, + authCallbackErrorFor, + authCallbackUrl, + readAuthCallbackParams, + withoutCallbackParams, + type AuthCallbackError, +} from '@/lib/auth-callback'; import { setJamUserId } from '@/lib/jam-metadata'; import { clearCrossfireDraftOnAuthChange } from '@/lib/crossfire/storage'; import { abortToolRuns } from '@/components/features/tools/tool-run'; @@ -104,6 +112,7 @@ function clearGoogleTokens() { export function AuthProvider({ children }: { children: React.ReactNode }) { const [session, setSession] = useState(null); const [loading, setLoading] = useState(true); + const [callbackError, setCallbackError] = useState(null); const hydrateSessionStore = useSessionStore((state) => state.hydrateFromServer); const prepareSessionStore = useSessionStore((state) => state.prepareForUser); const hydrateSettingsStore = useSettingsStore((state) => state.hydrateFromServer); @@ -173,39 +182,39 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { localStorage.removeItem('google_provider_token'); localStorage.removeItem('google_provider_refresh_token'); - const hydrateOAuthCallbackSession = async () => { - const code = new URLSearchParams(window.location.search).get('code'); - if (code) { - const cleanupOAuthParams = () => { - const nextUrl = new URL(window.location.href); - nextUrl.searchParams.delete('code'); - nextUrl.searchParams.delete('state'); - nextUrl.searchParams.delete('error'); - nextUrl.searchParams.delete('error_description'); - window.history.replaceState(window.history.state, '', `${nextUrl.pathname}${nextUrl.search}`); - }; - - let result: Awaited>; - let preserveOAuthParams = false; - try { - result = await withAuthTimeout( - supabase.auth.exchangeCodeForSession(code), - 'OAuth code exchange timeout', - OAUTH_CODE_EXCHANGE_TIMEOUT_MS, - ); - } catch (error) { - preserveOAuthParams = error instanceof AuthTimeoutError; - throw error; - } finally { - if (!preserveOAuthParams) cleanupOAuthParams(); - } + // Set before the first await: auth-js announces the empty stored session + // while the code is still being exchanged, and that must not end loading. + const callbackUrl = new URL(window.location.href); + let callbackPending = callbackUrl.pathname === AUTH_CALLBACK_PATH; + + const hydrateOAuthCallbackSession = async (): Promise => { + if (!callbackPending) return null; + const { code, flowId, error } = readAuthCallbackParams(callbackUrl); + window.history.replaceState(window.history.state, '', withoutCallbackParams(callbackUrl)); + if (error) { + setCallbackError(error); + return null; + } + if (!code) return null; - const { data, error } = result; - if (error) throw error; + // Finishing a sign-in is as explicit as pressing a sign-in button. + allowSessionsAfterSignIn(); + try { + // The flow id picks this sign-in's own verifier, so another tab's + // sign-in or a failed resend can't break the link. + const { data, error: exchangeError } = await withAuthTimeout( + supabase.auth.exchangeCodeForSession(code, flowId ? { flowId } : undefined), + 'OAuth code exchange timeout', + OAUTH_CODE_EXCHANGE_TIMEOUT_MS, + ); + if (exchangeError) throw exchangeError; return data.session; + } catch (err) { + // A session this browser already had still stands; getSession restores it below. + console.warn('[Auth] sign-in callback failed:', err); + if (mounted) setCallbackError(authCallbackErrorFor(err)); + return null; } - - return null; }; const init = async () => { @@ -255,6 +264,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { currentUserIdRef.current = undefined; setJamUserId(undefined); } finally { + callbackPending = false; if (mounted) setLoading(false); } }; @@ -277,6 +287,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { setJamUserId(nextUserId); setActiveAccessToken(newSession?.access_token ?? null, newSession?.user.id ?? null); setSession(newSession); + if (event === 'SIGNED_IN') setCallbackError(null); // Provider access tokens are short-lived. Keep them only for this browser // session; a later export must reauthorize if no fresh token is available. @@ -293,7 +304,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { resetDebateStore(); useDebateContextStore.getState().resetLocalData(); clearGoogleTokens(); - setLoading(false); + if (!callbackPending) setLoading(false); return; } @@ -350,7 +361,9 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { session, user: session?.user ?? null, loading, - signInWithGoogle: async ({ driveAccess = false } = {}) => { + callbackError, + clearCallbackError: () => setCallbackError(null), + signInWithGoogle: async ({ driveAccess = false, next } = {}) => { allowSessionsAfterSignIn(); // Always use the SDK-generated URL so PKCE's code_verifier is written // to storage before redirect. The previous Promise.race against a 1200ms @@ -360,7 +373,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { const { data, error } = await supabase.auth.signInWithOAuth({ provider: 'google', options: { - redirectTo: getOAuthRedirectUrl(), + redirectTo: authCallbackUrl(window.location.origin, next), skipBrowserRedirect: true, ...(driveAccess ? { scopes: 'https://www.googleapis.com/auth/drive.file' } : {}), queryParams: undefined, @@ -372,14 +385,15 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { await openExternalUrl(data.url); }, - signInWithEmailOtp: async (email) => { + signInWithEmailOtp: async (email, { next } = {}) => { allowSessionsAfterSignIn(); const normalizedEmail = email.trim(); if (!normalizedEmail) throw new Error('Email is required.'); const { error } = await supabase.auth.signInWithOtp({ email: normalizedEmail, - options: { shouldCreateUser: true }, + // Prod's email also carries a link. It must open the callback, not the landing page. + options: { shouldCreateUser: true, emailRedirectTo: authCallbackUrl(window.location.origin, next) }, }); if (error) throw error; }, @@ -418,7 +432,8 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { if (verificationError) throw verificationError; }, signOut: async () => { - await supabase.auth.signOut(); + // Local scope: signing out here must not sign the account out on other devices. + await supabase.auth.signOut({ scope: 'local' }); clearCrossfireDraftOnAuthChange(session?.user?.id, undefined); resetSessionStore(); resetSettingsStore(); @@ -435,7 +450,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { discardLocalState(); }, discardTabState, - }), [session, loading, discardLocalState, discardTabState, resetSessionStore, resetSettingsStore, resetSparringStore]); + }), [session, loading, callbackError, discardLocalState, discardTabState, resetSessionStore, resetSettingsStore, resetSparringStore]); return {children}; } diff --git a/src/lib/supabase/auth-context.ts b/src/lib/supabase/auth-context.ts index 4bcc8ced2..263026f50 100644 --- a/src/lib/supabase/auth-context.ts +++ b/src/lib/supabase/auth-context.ts @@ -1,5 +1,6 @@ import { createContext, useContext } from 'react'; import type { Session, User } from '@supabase/supabase-js'; +import type { AuthCallbackError } from '@/lib/auth-callback'; // Kept apart from AuthProvider so public pages can read the signed-in user // without importing the Supabase client, which needs a browser. The build @@ -9,8 +10,12 @@ export interface AuthContextValue { session: Session | null; user: User | null; loading: boolean; - signInWithGoogle: (options?: { driveAccess?: boolean }) => Promise; - signInWithEmailOtp: (email: string) => Promise; + /** Why the last sign-in callback (Google or an email link) did not sign anyone in. */ + callbackError: AuthCallbackError | null; + clearCallbackError: () => void; + /** `next` is the in-app page to open after the callback. */ + signInWithGoogle: (options?: { driveAccess?: boolean; next?: string }) => Promise; + signInWithEmailOtp: (email: string, options?: { next?: string }) => Promise; verifyEmailOtp: (email: string, token: string) => Promise; signInWithDemoCode: (code: string) => Promise; signOut: () => Promise; diff --git a/src/lib/supabase/client.ts b/src/lib/supabase/client.ts index 193866f05..9a9e8ab7c 100644 --- a/src/lib/supabase/client.ts +++ b/src/lib/supabase/client.ts @@ -65,6 +65,10 @@ export const supabase = createClient(SUPABASE_URL, SUPABASE_ANON_KEY, { detectSessionInUrl: false, flowType: 'pkce', storageKey: AUTH_STORAGE_KEY, + // Each sign-in keeps its own PKCE verifier and its redirect carries + // `sb_flow_id`, so a second sign-in (another tab, a resend) can't + // overwrite or delete the verifier an emailed link needs. + experimental: { appendPkceFlowIdToRedirects: true }, }, global: { fetch: watchedFetch }, }); diff --git a/src/locales/en.json b/src/locales/en.json index b4f86a9f3..586fcb9e9 100644 --- a/src/locales/en.json +++ b/src/locales/en.json @@ -539,6 +539,7 @@ "demoCodePlaceholder": "DEMO-AB12-CD34", "signInWithDemoCode": "Sign in with demo code", "signingIn": "Signing in...", + "finishingSignIn": "Signing you in…", "invalidEmail": "Enter a valid email address.", "invalidCode": "Enter a valid six-digit code.", "invalidDemoCode": "Enter a valid demo code.", @@ -559,7 +560,10 @@ "demoUsedUp": "This demo code has no uses left.", "rateLimited": "Too many attempts. Try again later.", "codeExpired": "That code is wrong or has expired. Check it or send a new one.", - "network": "Can't reach DebateLab. Check your connection and try again." + "network": "Can't reach DebateLab. Check your connection and try again.", + "linkExpired": "This sign-in link has expired or was already used. Send a new code.", + "linkOtherBrowser": "This sign-in link only works in the browser where you asked for it. Send a new code here.", + "callbackFailed": "Sign-in didn't finish. Try again." } }, "admin": { diff --git a/src/locales/es.json b/src/locales/es.json index a0280a9bd..6ee227626 100644 --- a/src/locales/es.json +++ b/src/locales/es.json @@ -539,6 +539,7 @@ "demoCodePlaceholder": "DEMO-AB12-CD34", "signInWithDemoCode": "Iniciar sesión con código demo", "signingIn": "Iniciando sesión...", + "finishingSignIn": "Iniciando sesión…", "invalidEmail": "Introduce una dirección de email válida.", "invalidCode": "Introduce un código válido de seis dígitos.", "invalidDemoCode": "Introduce un código demo válido.", @@ -559,7 +560,10 @@ "demoUsedUp": "Este código de demostración ya no tiene usos.", "rateLimited": "Demasiados intentos. Inténtalo más tarde.", "codeExpired": "Ese código es incorrecto o ha caducado. Revísalo o solicita uno nuevo.", - "network": "No se puede conectar con DebateLab. Revisa tu conexión e inténtalo de nuevo." + "network": "No se puede conectar con DebateLab. Revisa tu conexión e inténtalo de nuevo.", + "linkExpired": "Este enlace de inicio de sesión ha caducado o ya se usó. Solicita un código nuevo.", + "linkOtherBrowser": "Este enlace de inicio de sesión solo funciona en el navegador donde lo pediste. Solicita aquí un código nuevo.", + "callbackFailed": "No se completó el inicio de sesión. Inténtalo de nuevo." } }, "admin": { diff --git a/src/locales/hu.json b/src/locales/hu.json index 69405e4dd..dfbc021c3 100644 --- a/src/locales/hu.json +++ b/src/locales/hu.json @@ -539,6 +539,7 @@ "demoCodePlaceholder": "DEMO-AB12-CD34", "signInWithDemoCode": "Bejelentkezés demókóddal", "signingIn": "Bejelentkezés...", + "finishingSignIn": "Bejelentkezés…", "invalidEmail": "Adj meg érvényes e-mail-címet.", "invalidCode": "Adj meg érvényes, hatjegyű kódot.", "invalidDemoCode": "Adj meg érvényes demókódot.", @@ -559,7 +560,10 @@ "demoUsedUp": "Ezzel a demókóddal már nem lehet többször belépni.", "rateLimited": "Túl sok próbálkozás. Próbáld újra később.", "codeExpired": "Ez a kód hibás vagy lejárt. Ellenőrizd, vagy kérj újat.", - "network": "A DebateLab nem érhető el. Ellenőrizd a kapcsolatot, és próbáld újra." + "network": "A DebateLab nem érhető el. Ellenőrizd a kapcsolatot, és próbáld újra.", + "linkExpired": "Ez a bejelentkezési link lejárt, vagy már felhasználták. Kérj új kódot.", + "linkOtherBrowser": "Ez a bejelentkezési link csak abban a böngészőben működik, ahol kérted. Kérj itt új kódot.", + "callbackFailed": "A bejelentkezés nem fejeződött be. Próbáld újra." } }, "admin": { diff --git a/src/main.tsx b/src/main.tsx index 400423e09..30f7afe46 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -9,6 +9,7 @@ import { registerServiceWorker } from "./register-service-worker"; import { installStaleChunkReload } from "./lib/reload-stale-chunk"; import { initializeObservability } from "./lib/observability"; import { preloadRoute } from "./lib/route-modules"; +import { legacyCallbackPath } from "./lib/auth-callback"; const initializeTelemetry = (): void => { const run = (): void => { void initializeObservability(); }; @@ -28,6 +29,12 @@ function prerenderedRoute(container: HTMLElement): string | undefined { installStaleChunkReload(); export function mountApp(): void { + // Sign-in links already in inboxes return to / or /app. Move them onto the + // callback route before the first render, so the prerendered landing page + // is replaced and AuthProvider exchanges the code there. + const callbackPath = legacyCallbackPath(window.location); + if (callbackPath) window.history.replaceState(window.history.state, "", callbackPath); + const container = document.getElementById("root")!; const root = createRoot(container); const app = ( diff --git a/src/pages/AuthCallbackPage.tsx b/src/pages/AuthCallbackPage.tsx new file mode 100644 index 000000000..63d1d3a5a --- /dev/null +++ b/src/pages/AuthCallbackPage.tsx @@ -0,0 +1,29 @@ +import { Navigate, useSearchParams } from 'react-router-dom'; +import { useTranslation } from 'react-i18next'; +import { Loader2 } from 'lucide-react'; +import { useAuth } from '@/lib/supabase/AuthProvider'; +import { safeNextPath } from '@/lib/auth-callback'; + +/** + * Where Google and the emailed sign-in link return. AuthProvider exchanges the + * code and holds `loading` until it settles; this page then opens `next`, or + * /login, which says why the sign-in failed. + */ +export default function AuthCallbackPage() { + const { user, loading } = useAuth(); + const [params] = useSearchParams(); + const { t } = useTranslation(); + const next = safeNextPath(params.get('next')); + + if (loading) { + return ( +
+
+ ); + } + + if (user) return ; + return ; +} diff --git a/src/pages/LoginPage.tsx b/src/pages/LoginPage.tsx index 6d0aebd3c..666027501 100644 --- a/src/pages/LoginPage.tsx +++ b/src/pages/LoginPage.tsx @@ -13,6 +13,7 @@ import { useExternalRedirectPending } from '@/hooks/use-external-redirect-pendin import { consumeWorkspaceInvite } from '@/lib/workspace/invite-handoff'; import { localizeAuthError } from '@/lib/auth-errors'; import { clearSessionExpiredNotice, sessionExpiredNotice } from '@/lib/supabase/session-expiry'; +import type { AuthCallbackError } from '@/lib/auth-callback'; type AuthMode = 'email' | 'demo'; type AuthStatus = 'idle' | 'submitting' | 'code-sent' | 'verifying' | 'success' | 'error'; @@ -30,8 +31,17 @@ function isAuthMode(value: string): value is AuthMode { return value === 'email' || value === 'demo'; } +const CALLBACK_ERROR_KEYS: Record = { + 'link-expired': 'login.errors.linkExpired', + 'other-browser': 'login.errors.linkOtherBrowser', + failed: 'login.errors.callbackFailed', +}; + +/** RequireAuth's redirect: the page that needed sign-in, and whose session just ended there. */ +type LoginRouteState = { from?: { pathname?: string }; fromUserId?: string } | null; + export default function LoginPage() { - const { user, signInWithGoogle, signInWithEmailOtp, verifyEmailOtp, signInWithDemoCode } = useAuth(); + const { user, callbackError, clearCallbackError, signInWithGoogle, signInWithEmailOtp, verifyEmailOtp, signInWithDemoCode } = useAuth(); const { pending: isRedirecting, run: runRedirect } = useExternalRedirectPending(); const [authMode, setAuthMode] = useState('email'); const [authStatus, setAuthStatus] = useState('idle'); @@ -42,6 +52,9 @@ export default function LoginPage() { const [errorMessage, setErrorMessage] = useState(''); // Set when the server refused this tab's session and it was signed out. const [sessionExpired] = useState(sessionExpiredNotice); + // Why the sign-in link or Google sign-in that opened this page failed. Kept + // for this visit; the provider's copy is cleared so it doesn't come back. + const [linkError] = useState(callbackError); const otpInputRef = useRef(null); const emailInputRef = useRef(null); const demoInputRef = useRef(null); @@ -49,12 +62,22 @@ export default function LoginPage() { const location = useLocation(); const { t } = useTranslation(); const { toast } = useToast(); - const from = (location.state as { from?: { pathname?: string } } | null)?.from?.pathname || '/app'; + const routeState = location.state as LoginRouteState; + // A page another account had open belongs to that account. After a sign-out + // the next person at this browser starts on /app, not where they left off. + const fromThisAccount = !routeState?.fromUserId || routeState.fromUserId === user?.id; + const from = (fromThisAccount && routeState?.from?.pathname) || '/app'; + // Google and the email link reload the page, so the deep link travels in the redirect. + const next = routeState?.fromUserId ? undefined : routeState?.from?.pathname; useEffect(() => { if (user) navigate(consumeWorkspaceInvite() ?? from, { replace: true }); }, [user, navigate, from]); + useEffect(() => { + if (callbackError) clearCallbackError(); + }, [callbackError, clearCallbackError]); + useEffect(() => { if (sessionExpired) clearSessionExpiredNotice(); }, [sessionExpired]); @@ -72,7 +95,7 @@ export default function LoginPage() { const handleGoogleSignIn = async () => { try { - await runRedirect(() => signInWithGoogle()); + await runRedirect(() => signInWithGoogle({ next })); } catch (error) { toast({ title: t('login.failed', 'Could not start sign in'), @@ -102,7 +125,7 @@ export default function LoginPage() { setErrorMessage(''); setAuthStatus('submitting'); try { - await signInWithEmailOtp(normalizedEmail); + await signInWithEmailOtp(normalizedEmail, { next }); setOtp(''); setEmailSent(true); setAuthStatus('code-sent'); @@ -137,7 +160,7 @@ export default function LoginPage() { setErrorMessage(''); setAuthStatus('submitting'); try { - await signInWithEmailOtp(email.trim()); + await signInWithEmailOtp(email.trim(), { next }); setOtp(''); setAuthStatus('code-sent'); } catch (error) { @@ -196,6 +219,11 @@ export default function LoginPage() { {t('login.sessionExpired', 'Your session expired. Sign in again.')}

)} + {linkError && authStatus === 'idle' && ( +

+ {t(CALLBACK_ERROR_KEYS[linkError])} +

+ )} diff --git a/src/prerender/entry.tsx b/src/prerender/entry.tsx index 4df6ec36b..ca646a499 100644 --- a/src/prerender/entry.tsx +++ b/src/prerender/entry.tsx @@ -20,6 +20,8 @@ const SIGNED_OUT: AuthContextValue = { session: null, user: null, loading: false, + callbackError: null, + clearCallbackError: () => undefined, signInWithGoogle: unavailable, signInWithEmailOtp: unavailable, verifyEmailOtp: unavailable, diff --git a/src/test/auth/AuthProvider.behavior.test.tsx b/src/test/auth/AuthProvider.behavior.test.tsx index a05e24866..e7546db77 100644 --- a/src/test/auth/AuthProvider.behavior.test.tsx +++ b/src/test/auth/AuthProvider.behavior.test.tsx @@ -107,7 +107,6 @@ vi.mock('@/lib/supabase/client', () => ({ })); vi.mock('@/lib/native', () => ({ - getOAuthRedirectUrl: () => `${window.location.origin}/app`, openExternalUrl: openExternalUrlMock, })); @@ -142,12 +141,13 @@ import { crossfireStorageKey } from '@/lib/crossfire/storage'; import { useDebateStore } from '@/lib/stores/debate-store'; function AuthConsumer() { - const { loading, user, signInWithGoogle, signOut } = useAuth(); + const { loading, user, callbackError, signInWithGoogle, signOut } = useAuth(); return (
{String(loading)}
{user?.id ?? 'none'}
+
{callbackError ?? 'none'}
@@ -507,7 +507,7 @@ describe('AuthProvider behavior', () => { expect(signInWithOAuthMock).toHaveBeenCalledWith({ provider: 'google', options: { - redirectTo: `${window.location.origin}/app`, + redirectTo: `${window.location.origin}/auth/callback`, skipBrowserRedirect: true, queryParams: undefined, }, @@ -533,7 +533,7 @@ describe('AuthProvider behavior', () => { expect(signInWithOAuthMock).toHaveBeenCalledWith({ provider: 'google', options: { - redirectTo: `${window.location.origin}/app`, + redirectTo: `${window.location.origin}/auth/callback`, skipBrowserRedirect: true, scopes: 'https://www.googleapis.com/auth/drive.file', queryParams: undefined, @@ -554,7 +554,7 @@ describe('AuthProvider behavior', () => { await waitFor(() => { expect(signInWithOtpMock).toHaveBeenCalledWith({ email: 'student@example.com', - options: { shouldCreateUser: true }, + options: { shouldCreateUser: true, emailRedirectTo: `${window.location.origin}/auth/callback` }, }); }); }); @@ -629,8 +629,8 @@ describe('AuthProvider behavior', () => { await waitFor(() => expect(screen.getByTestId('passwordless-error')).toHaveTextContent('invalid session handoff')); }); - it('exchanges PKCE codes from the callback URL and clears the query string', async () => { - window.history.replaceState({}, '', '/app?code=pkce-code'); + it('exchanges the callback code with its own flow verifier and keeps the deep link', async () => { + window.history.replaceState({}, '', '/auth/callback?code=pkce-code&sb_flow_id=0123456789abcdef&next=%2Fapp%2Fhistory'); exchangeCodeForSessionMock.mockResolvedValue({ data: { session: { user: { id: 'pkce-user' } } }, error: null, @@ -646,14 +646,109 @@ describe('AuthProvider behavior', () => { expect(screen.getByTestId('user')).toHaveTextContent('pkce-user'); }); - expect(exchangeCodeForSessionMock).toHaveBeenCalledWith('pkce-code'); + expect(exchangeCodeForSessionMock).toHaveBeenCalledWith('pkce-code', { flowId: '0123456789abcdef' }); + expect(window.location.search).toBe('?next=%2Fapp%2Fhistory'); + }); + + it('exchanges a callback code without a flow id the old way', async () => { + window.history.replaceState({}, '', '/auth/callback?code=pkce-code'); + exchangeCodeForSessionMock.mockResolvedValue({ + data: { session: { user: { id: 'pkce-user' } } }, + error: null, + }); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId('user')).toHaveTextContent('pkce-user'); + }); + expect(exchangeCodeForSessionMock).toHaveBeenCalledWith('pkce-code', undefined); expect(window.location.search).toBe(''); }); - it('fails open when PKCE callback exchange hangs', async () => { + it('stays loading while the callback exchange runs, even when auth reports no session yet', async () => { + window.history.replaceState({}, '', '/auth/callback?code=pkce-code'); + let finishExchange!: (value: unknown) => void; + exchangeCodeForSessionMock.mockReturnValue(new Promise((resolve) => { finishExchange = resolve; })); + + render( + + + + ); + + // auth-js announces the empty stored session while the code is still being exchanged. + act(() => callbackRef.current?.('INITIAL_SESSION', null)); + expect(screen.getByTestId('loading')).toHaveTextContent('true'); + + await act(async () => { + finishExchange({ data: { session: { user: { id: 'pkce-user' }, access_token: 'token' } }, error: null }); + }); + + await waitFor(() => expect(screen.getByTestId('user')).toHaveTextContent('pkce-user')); + expect(screen.getByTestId('loading')).toHaveTextContent('false'); + }); + + it('says why a callback exchange failed and keeps any session the browser already had', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + window.history.replaceState({}, '', '/auth/callback?code=pkce-code&sb_flow_id=0123456789abcdef'); + exchangeCodeForSessionMock.mockResolvedValue({ + data: { session: null }, + error: Object.assign(new Error('PKCE code verifier not found in storage.'), { code: 'pkce_code_verifier_not_found' }), + }); + getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'existing-user' }, access_token: 'token' } }, error: null }); + + render( + + + + ); + + await waitFor(() => expect(screen.getByTestId('loading')).toHaveTextContent('false')); + expect(screen.getByTestId('callback-error')).toHaveTextContent('other-browser'); + expect(screen.getByTestId('user')).toHaveTextContent('existing-user'); + expect(window.location.search).toBe(''); + warnSpy.mockRestore(); + }); + + it('reads an expired-link error from the callback URL without exchanging anything', async () => { + window.history.replaceState({}, '', '/auth/callback?error=access_denied&error_code=otp_expired&error_description=Email+link+is+invalid+or+has+expired'); + + render( + + + + ); + + await waitFor(() => expect(screen.getByTestId('loading')).toHaveTextContent('false')); + expect(screen.getByTestId('callback-error')).toHaveTextContent('link-expired'); + expect(screen.getByTestId('user')).toHaveTextContent('none'); + expect(exchangeCodeForSessionMock).not.toHaveBeenCalled(); + expect(window.location.search).toBe(''); + }); + + it('ignores a code query parameter outside the callback route', async () => { + window.history.replaceState({}, '', '/app/history?code=not-ours'); + + render( + + + + ); + + await waitFor(() => expect(screen.getByTestId('loading')).toHaveTextContent('false')); + expect(exchangeCodeForSessionMock).not.toHaveBeenCalled(); + expect(window.location.search).toBe('?code=not-ours'); + }); + + it('stops waiting and reports a failed sign-in when the callback exchange hangs', async () => { vi.useFakeTimers(); const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - window.history.replaceState({}, '', '/app?code=stalled-code'); + window.history.replaceState({}, '', '/auth/callback?code=stalled-code'); exchangeCodeForSessionMock.mockReturnValue(new Promise(() => {})); render( @@ -662,15 +757,20 @@ describe('AuthProvider behavior', () => { ); + act(() => callbackRef.current?.('INITIAL_SESSION', null)); + await act(async () => { + await vi.advanceTimersByTimeAsync(14_000); + }); + expect(screen.getByTestId('loading')).toHaveTextContent('true'); + await act(async () => { // Keep this in sync with the OAuth code-exchange timeout in AuthProvider. - await vi.advanceTimersByTimeAsync(15_000); + await vi.advanceTimersByTimeAsync(1_000); }); expect(screen.getByTestId('loading')).toHaveTextContent('false'); expect(screen.getByTestId('user')).toHaveTextContent('none'); - expect(warnSpy).toHaveBeenCalledWith('[Auth] init failed:', expect.any(Error)); - expect(window.location.search).toBe('?code=stalled-code'); + expect(screen.getByTestId('callback-error')).toHaveTextContent('failed'); warnSpy.mockRestore(); vi.useRealTimers(); @@ -696,7 +796,7 @@ describe('AuthProvider behavior', () => { }); }); - it('signOut delegates to Supabase and resets both stores', async () => { + it('signOut ends only this device\'s session and resets both stores', async () => { render( @@ -706,7 +806,7 @@ describe('AuthProvider behavior', () => { fireEvent.click(screen.getByRole('button', { name: 'sign-out' })); await waitFor(() => { - expect(signOutMock).toHaveBeenCalled(); + expect(signOutMock).toHaveBeenCalledWith({ scope: 'local' }); }); expect(resetSessionStoreMock).toHaveBeenCalled(); diff --git a/src/test/auth/account-switch-route.test.tsx b/src/test/auth/account-switch-route.test.tsx new file mode 100644 index 000000000..4ba9b9174 --- /dev/null +++ b/src/test/auth/account-switch-route.test.tsx @@ -0,0 +1,64 @@ +import { describe, expect, it, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom'; +import { RequireAuth } from '@/components/auth/RequireAuth'; +import LoginPage from '@/pages/LoginPage'; + +const auth = vi.hoisted(() => ({ user: null as { id: string } | null })); + +vi.mock('@/lib/supabase/AuthProvider', () => ({ + useAuth: () => ({ + user: auth.user, + loading: false, + callbackError: null as null, + clearCallbackError: vi.fn(), + signInWithGoogle: vi.fn(), + signInWithEmailOtp: vi.fn(), + verifyEmailOtp: vi.fn(), + signInWithDemoCode: vi.fn(), + }), +})); + +function Where() { + return
{useLocation().pathname}
; +} + +function App() { + return ( + + + } /> + }> + + + + + + ); +} + +// Signs in as `first` on /app/history, signs out, then signs in as `second` in the same tab. +async function switchAccounts(first: string, second: string) { + auth.user = { id: first }; + const view = render(); + expect(screen.getByTestId('where')).toHaveTextContent('/app/history'); + + auth.user = null; + view.rerender(); + await waitFor(() => expect(screen.getByTestId('where')).toHaveTextContent('/login')); + + auth.user = { id: second }; + view.rerender(); + await waitFor(() => expect(screen.getByTestId('where')).not.toHaveTextContent('/login')); + return screen.getByTestId('where').textContent; +} + +describe('where a sign-in lands after a sign-out in the same tab', () => { + it('opens /app for a different account, not the page the previous account had open', async () => { + expect(await switchAccounts('student-a', 'student-b')).toBe('/app'); + }); + + it('returns the same account to the page it left', async () => { + expect(await switchAccounts('student-a', 'student-a')).toBe('/app/history'); + }); +}); diff --git a/src/test/auth/auth-callback.test.ts b/src/test/auth/auth-callback.test.ts new file mode 100644 index 000000000..786e62ce9 --- /dev/null +++ b/src/test/auth/auth-callback.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest'; +import { authCallbackErrorFor, authCallbackUrl, legacyCallbackPath, readAuthCallbackParams, safeNextPath } from '@/lib/auth-callback'; + +describe('auth callback URLs', () => { + it.each([ + ['/app/history', '/app/history'], + ['/app', '/app'], + ['/app/workspaces/invite', '/app/workspaces/invite'], + [null, '/app'], + ['', '/app'], + ['/login', '/app'], + ['//evil.example/app', '/app'], + ['/app//evil.example', '/app'], + ['/app/\\evil.example', '/app'], + ['https://evil.example/app', '/app'], + ['javascript:alert(1)', '/app'], + ['/app/../login', '/app'], + ['/application', '/app'], + ])('only lets %s return to an in-app page (%s)', (value, expected) => { + expect(safeNextPath(value)).toBe(expected); + }); + + it('carries a deep link through the callback and drops anything else', () => { + expect(authCallbackUrl('https://debate.pi-stack.org')).toBe('https://debate.pi-stack.org/auth/callback'); + expect(authCallbackUrl('https://debate.pi-stack.org', '/app')).toBe('https://debate.pi-stack.org/auth/callback'); + expect(authCallbackUrl('https://debate.pi-stack.org', '/app/history')).toBe('https://debate.pi-stack.org/auth/callback?next=%2Fapp%2Fhistory'); + expect(authCallbackUrl('https://debate.pi-stack.org', '//evil.example')).toBe('https://debate.pi-stack.org/auth/callback'); + }); + + it.each([ + ['/', '?code=abc', '', '/auth/callback?code=abc'], + ['/app', '?code=abc&sb_flow_id=0123456789abcdef', '', '/auth/callback?code=abc&sb_flow_id=0123456789abcdef'], + ['/app/', '?error=access_denied&error_code=otp_expired', '', '/auth/callback?error=access_denied&error_code=otp_expired'], + ['/', '', '#error=access_denied&error_code=otp_expired', '/auth/callback#error=access_denied&error_code=otp_expired'], + ])('moves an old link to %s%s%s onto the callback route', (pathname, search, hash, expected) => { + expect(legacyCallbackPath({ pathname, search, hash })).toBe(expected); + }); + + it.each([ + ['/', '', ''], + ['/', '', '#short-answer'], + ['/app/settings', '?checkout=success', ''], + ['/app/history', '?code=abc', ''], + ['/share/abc', '?code=abc', ''], + ['/auth/callback', '?code=abc', ''], + ])('leaves %s%s%s alone', (pathname, search, hash) => { + expect(legacyCallbackPath({ pathname, search, hash })).toBeNull(); + }); + + it('reads the code, its flow id and any error the auth server returned', () => { + expect(readAuthCallbackParams(new URL('https://x.test/auth/callback?code=c1&sb_flow_id=0123456789abcdef&next=%2Fapp'))).toEqual({ + code: 'c1', + flowId: '0123456789abcdef', + error: null, + }); + expect(readAuthCallbackParams(new URL('https://x.test/auth/callback?error=access_denied&error_code=otp_expired&error_description=Email+link+is+invalid'))).toEqual({ + code: null, + flowId: null, + error: 'link-expired', + }); + expect(readAuthCallbackParams(new URL('https://x.test/auth/callback#error=server_error&error_description=x'))).toEqual({ + code: null, + flowId: null, + error: 'failed', + }); + // A flow id outside auth-js's own shape never reaches a storage key. + expect(readAuthCallbackParams(new URL('https://x.test/auth/callback?code=c1&sb_flow_id=../x')).flowId).toBeNull(); + }); + + it('names why an exchange failed', () => { + expect(authCallbackErrorFor({ code: 'pkce_code_verifier_not_found' })).toBe('other-browser'); + expect(authCallbackErrorFor({ code: 'flow_state_not_found' })).toBe('link-expired'); + expect(authCallbackErrorFor({ code: 'flow_state_expired' })).toBe('link-expired'); + expect(authCallbackErrorFor(new Error('network'))).toBe('failed'); + }); +}); diff --git a/src/test/auth/onboarding-per-account.test.ts b/src/test/auth/onboarding-per-account.test.ts new file mode 100644 index 000000000..24c0efac0 --- /dev/null +++ b/src/test/auth/onboarding-per-account.test.ts @@ -0,0 +1,45 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const STORAGE_KEY = 'debate-trainer-onboarding'; + +async function loadStore() { + vi.resetModules(); + return await import('@/hooks/use-onboarding'); +} + +describe('onboarding tour per account', () => { + beforeEach(() => localStorage.clear()); + + it('shows the tour to every new account on a browser, once each', async () => { + const { useOnboardingStore, hasCompletedOnboarding } = await loadStore(); + useOnboardingStore.getState().setOnboardingComplete('student-a'); + + const state = useOnboardingStore.getState(); + expect(hasCompletedOnboarding(state, 'student-a')).toBe(true); + expect(hasCompletedOnboarding(state, 'student-b')).toBe(false); + expect(JSON.parse(localStorage.getItem(STORAGE_KEY)!)).toEqual({ + state: { completedUserIds: ['student-a'], legacyCompleted: false }, + version: 1, + }); + }); + + it('gives the old browser-wide "done" flag to the next account only', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ state: { hasCompletedOnboarding: true, isTourRunning: false }, version: 0 })); + const { useOnboardingStore, hasCompletedOnboarding } = await loadStore(); + + useOnboardingStore.getState().claimLegacyCompletion('student-a'); + useOnboardingStore.getState().claimLegacyCompletion('student-b'); + + const state = useOnboardingStore.getState(); + expect(hasCompletedOnboarding(state, 'student-a')).toBe(true); + expect(hasCompletedOnboarding(state, 'student-b')).toBe(false); + }); + + it('keeps the tour for a browser whose old flag says it was never finished', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ state: { hasCompletedOnboarding: false, isTourRunning: false }, version: 0 })); + const { useOnboardingStore, hasCompletedOnboarding } = await loadStore(); + + useOnboardingStore.getState().claimLegacyCompletion('student-a'); + expect(hasCompletedOnboarding(useOnboardingStore.getState(), 'student-a')).toBe(false); + }); +}); diff --git a/src/test/vercel-deployment-config.test.ts b/src/test/vercel-deployment-config.test.ts index ebbaf2dea..b1fc0128f 100644 --- a/src/test/vercel-deployment-config.test.ts +++ b/src/test/vercel-deployment-config.test.ts @@ -78,7 +78,7 @@ describe("Vercel frontend deployment configuration", () => { const config = loadConfig(); expect(config.rewrites).toEqual( - ["/app", "/app/:path*", "/login", "/share/:path*", "/expocit/sources"] + ["/app", "/app/:path*", "/login", "/auth/callback", "/share/:path*", "/expocit/sources"] .map((source) => ({ source, destination: "/app-shell.html" })), ); // Rewrite sources match exactly, so /app/settings/ must first lose its slash. @@ -255,7 +255,7 @@ describe("Vercel frontend deployment configuration", () => { } }); - it.each(["/app", "/app/(.*)", "/login", "/share/(.*)", "/embed/debate-timer"])( + it.each(["/app", "/app/(.*)", "/login", "/auth/callback", "/share/(.*)", "/embed/debate-timer"])( "keeps private route %s out of search indexes", (source) => { const config = loadConfig(); diff --git a/supabase/config.toml b/supabase/config.toml index 97021b5c5..c4f15a2c6 100644 --- a/supabase/config.toml +++ b/supabase/config.toml @@ -59,3 +59,10 @@ max_frequency = "1s" [auth.email.template.magic_link] subject = "Your DebateLab sign-in code" content_path = "./supabase/templates/magic_link.html" + +# A first email sign-in creates the account, so Auth sends this template +# instead of magic_link. Same subject and code-only body: the code never goes +# in the subject, and there is no link to open in the wrong browser. +[auth.email.template.confirmation] +subject = "Your DebateLab sign-in code" +content_path = "./supabase/templates/confirmation.html" diff --git a/supabase/templates/confirmation.html b/supabase/templates/confirmation.html new file mode 100644 index 000000000..c4f6e3cd2 --- /dev/null +++ b/supabase/templates/confirmation.html @@ -0,0 +1,6 @@ + + + +

Your DebateLab sign-in code is {{ .Token }}.

+ + diff --git a/vercel.json b/vercel.json index 4adfb3c7f..7d91b36c8 100644 --- a/vercel.json +++ b/vercel.json @@ -105,6 +105,10 @@ "source": "/login", "destination": "/app-shell.html" }, + { + "source": "/auth/callback", + "destination": "/app-shell.html" + }, { "source": "/share/:path*", "destination": "/app-shell.html" @@ -189,6 +193,15 @@ } ] }, + { + "source": "/auth/callback", + "headers": [ + { + "key": "X-Robots-Tag", + "value": "noindex, nofollow" + } + ] + }, { "source": "/share/(.*)", "headers": [ From 3891981992287914f53310cfff7b531abfe9ed3f Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 21:01:59 +0000 Subject: [PATCH 02/11] Address Codex review: hold loading for another account's callback, keep link errors - AuthProvider keeps loading while a callback exchange runs even when the browser's stored session (another account) is announced meanwhile, so the callback page never opens `next` as the wrong account. - A callback error is no longer cleared by any SIGNED_IN (a session restore fires one). When the link fails but another session survives, the callback page opens that account and shows the failure as a toast. - The onboarding store validates its stored copy on every rehydrate (merge), not only on migration. - A tour left running never opens for an account that already finished it. - RequireAuth records the last account in an effect, not during render (docs/standards/async-and-state.md). Co-Authored-By: Claude Opus 5.5 --- src/App.tsx | 3 +- src/components/auth/RequireAuth.tsx | 6 ++-- src/hooks/use-onboarding.ts | 15 +++++++- src/lib/auth-callback.ts | 6 ++++ src/lib/supabase/AuthProvider.tsx | 5 +-- src/pages/AuthCallbackPage.tsx | 15 ++++++-- src/pages/LoginPage.tsx | 10 ++---- src/test/auth/AuthProvider.behavior.test.tsx | 37 ++++++++++++++++++++ src/test/auth/auth-callback-page.test.tsx | 36 +++++++++++++++++++ src/test/auth/onboarding-per-account.test.ts | 9 +++++ 10 files changed, 126 insertions(+), 16 deletions(-) create mode 100644 src/test/auth/auth-callback-page.test.tsx diff --git a/src/App.tsx b/src/App.tsx index ee8980297..05ae4116c 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -238,7 +238,8 @@ function OptionalOnboardingTour() { const completed = useOnboardingStore((state) => hasCompletedOnboarding(state, userId)); const isTourRunning = useOnboardingStore((state) => state.isTourRunning); const claimLegacyCompletion = useOnboardingStore((state) => state.claimLegacyCompletion); - const shouldLoadTour = isTourRunning || (location.pathname === "/app" && !completed); + // A tour left running by the previous account never opens for one that finished it. + const shouldLoadTour = !completed && (isTourRunning || location.pathname === "/app"); useEffect(() => { if (userId) claimLegacyCompletion(userId); diff --git a/src/components/auth/RequireAuth.tsx b/src/components/auth/RequireAuth.tsx index a82ed1c3e..4aa42f6ef 100644 --- a/src/components/auth/RequireAuth.tsx +++ b/src/components/auth/RequireAuth.tsx @@ -1,4 +1,4 @@ -import { useRef } from 'react'; +import { useEffect, useRef } from 'react'; import { Navigate, useLocation } from 'react-router-dom'; import { useAuth } from '@/lib/supabase/AuthProvider'; import { Loader2 } from 'lucide-react'; @@ -10,7 +10,9 @@ export function RequireAuth({ children }: { children: React.ReactNode }) { // The account this layout last showed. A sign-out here stamps the redirect // with it, so LoginPage returns only that account to the page it left. const lastUserId = useRef(undefined); - if (user) lastUserId.current = user.id; + useEffect(() => { + if (user) lastUserId.current = user.id; + }, [user]); if (loading) { return ( diff --git a/src/hooks/use-onboarding.ts b/src/hooks/use-onboarding.ts index ef0bac44c..b9edafd1d 100644 --- a/src/hooks/use-onboarding.ts +++ b/src/hooks/use-onboarding.ts @@ -21,6 +21,17 @@ type PersistedOnboarding = Pick (userIds.includes(userId) ? userIds : [...userIds, userId]); +/** Stored state is browser data: anything malformed counts as nothing stored. */ +function readPersisted(value: unknown): PersistedOnboarding { + const stored = (value ?? {}) as Partial>; + return { + completedUserIds: Array.isArray(stored.completedUserIds) + ? stored.completedUserIds.filter((id): id is string => typeof id === 'string') + : [], + legacyCompleted: stored.legacyCompleted === true, + }; +} + export function hasCompletedOnboarding(state: PersistedOnboarding, userId: string | undefined): boolean { return !userId || state.legacyCompleted || state.completedUserIds.includes(userId); } @@ -51,8 +62,10 @@ export const useOnboardingStore = create()( const legacy = persisted as { hasCompletedOnboarding?: unknown } | null; return { completedUserIds: [], legacyCompleted: legacy?.hasCompletedOnboarding === true }; } - return persisted as PersistedOnboarding; + return readPersisted(persisted); }, + // migrate runs only on a version change; every rehydrate is checked here. + merge: (persisted, current) => ({ ...current, ...readPersisted(persisted) }), } ) ); diff --git a/src/lib/auth-callback.ts b/src/lib/auth-callback.ts index 8c607dbfb..e8bcfa892 100644 --- a/src/lib/auth-callback.ts +++ b/src/lib/auth-callback.ts @@ -5,6 +5,12 @@ export const AUTH_CALLBACK_PATH = '/auth/callback'; /** Why a callback did not sign anyone in. LoginPage shows a message for each. */ export type AuthCallbackError = 'link-expired' | 'other-browser' | 'failed'; +export const AUTH_CALLBACK_ERROR_KEYS: Record = { + 'link-expired': 'login.errors.linkExpired', + 'other-browser': 'login.errors.linkOtherBrowser', + failed: 'login.errors.callbackFailed', +}; + export type AuthCallbackParams = { code: string | null; flowId: string | null; diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index abdf76d7d..c40cfd779 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -287,7 +287,6 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { setJamUserId(nextUserId); setActiveAccessToken(newSession?.access_token ?? null, newSession?.user.id ?? null); setSession(newSession); - if (event === 'SIGNED_IN') setCallbackError(null); // Provider access tokens are short-lived. Keep them only for this browser // session; a later export must reauthorize if no fresh token is available. @@ -328,7 +327,9 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { } else { hydrateStores(nextUserId); } - setLoading(false); + // A session this browser already had is announced while the callback + // exchange runs; the callback page must wait for the account it signs in. + if (!callbackPending) setLoading(false); }); return () => { diff --git a/src/pages/AuthCallbackPage.tsx b/src/pages/AuthCallbackPage.tsx index 63d1d3a5a..ad0bf9b2c 100644 --- a/src/pages/AuthCallbackPage.tsx +++ b/src/pages/AuthCallbackPage.tsx @@ -1,8 +1,10 @@ +import { useEffect } from 'react'; import { Navigate, useSearchParams } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; import { Loader2 } from 'lucide-react'; import { useAuth } from '@/lib/supabase/AuthProvider'; -import { safeNextPath } from '@/lib/auth-callback'; +import { useToast } from '@/hooks/use-toast'; +import { AUTH_CALLBACK_ERROR_KEYS, safeNextPath } from '@/lib/auth-callback'; /** * Where Google and the emailed sign-in link return. AuthProvider exchanges the @@ -10,11 +12,20 @@ import { safeNextPath } from '@/lib/auth-callback'; * /login, which says why the sign-in failed. */ export default function AuthCallbackPage() { - const { user, loading } = useAuth(); + const { user, loading, callbackError, clearCallbackError } = useAuth(); const [params] = useSearchParams(); const { t } = useTranslation(); + const { toast } = useToast(); const next = safeNextPath(params.get('next')); + // The link failed, but this browser is still signed in to another session: + // that account opens, and the failure is still said. + useEffect(() => { + if (loading || !user || !callbackError) return; + toast({ title: t(AUTH_CALLBACK_ERROR_KEYS[callbackError]), variant: 'destructive' }); + clearCallbackError(); + }, [loading, user, callbackError, clearCallbackError, t, toast]); + if (loading) { return (
diff --git a/src/pages/LoginPage.tsx b/src/pages/LoginPage.tsx index 666027501..203f1930d 100644 --- a/src/pages/LoginPage.tsx +++ b/src/pages/LoginPage.tsx @@ -13,7 +13,7 @@ import { useExternalRedirectPending } from '@/hooks/use-external-redirect-pendin import { consumeWorkspaceInvite } from '@/lib/workspace/invite-handoff'; import { localizeAuthError } from '@/lib/auth-errors'; import { clearSessionExpiredNotice, sessionExpiredNotice } from '@/lib/supabase/session-expiry'; -import type { AuthCallbackError } from '@/lib/auth-callback'; +import { AUTH_CALLBACK_ERROR_KEYS } from '@/lib/auth-callback'; type AuthMode = 'email' | 'demo'; type AuthStatus = 'idle' | 'submitting' | 'code-sent' | 'verifying' | 'success' | 'error'; @@ -31,12 +31,6 @@ function isAuthMode(value: string): value is AuthMode { return value === 'email' || value === 'demo'; } -const CALLBACK_ERROR_KEYS: Record = { - 'link-expired': 'login.errors.linkExpired', - 'other-browser': 'login.errors.linkOtherBrowser', - failed: 'login.errors.callbackFailed', -}; - /** RequireAuth's redirect: the page that needed sign-in, and whose session just ended there. */ type LoginRouteState = { from?: { pathname?: string }; fromUserId?: string } | null; @@ -221,7 +215,7 @@ export default function LoginPage() { )} {linkError && authStatus === 'idle' && (

- {t(CALLBACK_ERROR_KEYS[linkError])} + {t(AUTH_CALLBACK_ERROR_KEYS[linkError])}

)}
} /> + + , + ); + + expect(screen.getByText('history')).toBeInTheDocument(); + expect(mocks.toast).toHaveBeenCalledWith(expect.objectContaining({ + title: 'This sign-in link only works in the browser where you asked for it. Send a new code here.', + variant: 'destructive', + })); + expect(mocks.clearCallbackError).toHaveBeenCalled(); + }); +}); diff --git a/src/test/auth/onboarding-per-account.test.ts b/src/test/auth/onboarding-per-account.test.ts index 24c0efac0..242718340 100644 --- a/src/test/auth/onboarding-per-account.test.ts +++ b/src/test/auth/onboarding-per-account.test.ts @@ -42,4 +42,13 @@ describe('onboarding tour per account', () => { useOnboardingStore.getState().claimLegacyCompletion('student-a'); expect(hasCompletedOnboarding(useOnboardingStore.getState(), 'student-a')).toBe(false); }); + + it('ignores a damaged stored copy instead of breaking every protected page', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ state: { completedUserIds: null, legacyCompleted: 'yes' }, version: 1 })); + const { useOnboardingStore, hasCompletedOnboarding } = await loadStore(); + + const state = useOnboardingStore.getState(); + expect(state.completedUserIds).toEqual([]); + expect(hasCompletedOnboarding(state, 'student-a')).toBe(false); + }); }); From 6b26b63c143718bd78ec4b19d0940aa44e8da829 Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 21:12:20 +0000 Subject: [PATCH 03/11] Keep the callback code in the URL until the exchange settles Codex review r2: clearing `code` and `sb_flow_id` before the exchange meant a reload during the spinner found no code and sent the user to /login with an unused code and verifier. The URL is now cleaned once the exchange succeeds, fails or times out; auth errors and empty callbacks are cleaned at once, as before. Co-Authored-By: Claude Opus 5.5 --- src/lib/supabase/AuthProvider.tsx | 12 ++++++++---- src/test/auth/AuthProvider.behavior.test.tsx | 2 ++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index c40cfd779..9bbdf56b2 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -190,12 +190,12 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { const hydrateOAuthCallbackSession = async (): Promise => { if (!callbackPending) return null; const { code, flowId, error } = readAuthCallbackParams(callbackUrl); - window.history.replaceState(window.history.state, '', withoutCallbackParams(callbackUrl)); - if (error) { - setCallbackError(error); + const cleanUrl = () => window.history.replaceState(window.history.state, '', withoutCallbackParams(callbackUrl)); + if (error || !code) { + cleanUrl(); + if (error) setCallbackError(error); return null; } - if (!code) return null; // Finishing a sign-in is as explicit as pressing a sign-in button. allowSessionsAfterSignIn(); @@ -214,6 +214,10 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { console.warn('[Auth] sign-in callback failed:', err); if (mounted) setCallbackError(authCallbackErrorFor(err)); return null; + } finally { + // Only once the exchange settles: a reload during the spinner must + // still find the unused code and its flow id. + if (mounted) cleanUrl(); } }; diff --git a/src/test/auth/AuthProvider.behavior.test.tsx b/src/test/auth/AuthProvider.behavior.test.tsx index 5c125d2e7..6efd19344 100644 --- a/src/test/auth/AuthProvider.behavior.test.tsx +++ b/src/test/auth/AuthProvider.behavior.test.tsx @@ -684,6 +684,8 @@ describe('AuthProvider behavior', () => { // auth-js announces the empty stored session while the code is still being exchanged. act(() => callbackRef.current?.('INITIAL_SESSION', null)); expect(screen.getByTestId('loading')).toHaveTextContent('true'); + // A reload during the spinner must still find the unused code and its flow. + expect(window.location.search).toBe('?code=pkce-code'); await act(async () => { finishExchange({ data: { session: { user: { id: 'pkce-user' }, access_token: 'token' } }, error: null }); From 42eb5a52ac25ef187760095624bb29dff6da49b4 Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 21:14:17 +0000 Subject: [PATCH 04/11] Keep the callback code after an exchange timeout A timeout is not a final answer: the code may still be good, so the URL keeps it and a reload can retry, as main did before. Only a success or a definitive failure cleans it. Co-Authored-By: Claude Opus 5.5 --- src/lib/supabase/AuthProvider.tsx | 9 ++++++--- src/test/auth/AuthProvider.behavior.test.tsx | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index 9bbdf56b2..c2bd43ebe 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -199,6 +199,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { // Finishing a sign-in is as explicit as pressing a sign-in button. allowSessionsAfterSignIn(); + let settled = true; try { // The flow id picks this sign-in's own verifier, so another tab's // sign-in or a failed resend can't break the link. @@ -210,14 +211,16 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { if (exchangeError) throw exchangeError; return data.session; } catch (err) { + // A timeout is not a final answer: the code may still be good. + settled = !(err instanceof AuthTimeoutError); // A session this browser already had still stands; getSession restores it below. console.warn('[Auth] sign-in callback failed:', err); if (mounted) setCallbackError(authCallbackErrorFor(err)); return null; } finally { - // Only once the exchange settles: a reload during the spinner must - // still find the unused code and its flow id. - if (mounted) cleanUrl(); + // Only once the exchange has an answer: a reload during the spinner, + // or after a timeout, must still find the unused code and its flow id. + if (mounted && settled) cleanUrl(); } }; diff --git a/src/test/auth/AuthProvider.behavior.test.tsx b/src/test/auth/AuthProvider.behavior.test.tsx index 6efd19344..2200e2f6b 100644 --- a/src/test/auth/AuthProvider.behavior.test.tsx +++ b/src/test/auth/AuthProvider.behavior.test.tsx @@ -810,6 +810,8 @@ describe('AuthProvider behavior', () => { expect(screen.getByTestId('loading')).toHaveTextContent('false'); expect(screen.getByTestId('user')).toHaveTextContent('none'); expect(screen.getByTestId('callback-error')).toHaveTextContent('failed'); + // A timeout is not a final answer: the code may still be good, so a reload can retry it. + expect(window.location.search).toBe('?code=stalled-code'); warnSpy.mockRestore(); vi.useRealTimers(); From d2b067a2cb3cb2c972b5912d049ac4dad018d91b Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 21:27:10 +0000 Subject: [PATCH 05/11] Offer a retry when the callback exchange times out; bound the user lookup Codex review r3: - After a timeout the provider kept the code, but the callback page still navigated to /login and lost it. A timeout now has its own error kind: the page stays on the callback URL and offers Try again (a reload that reuses the code) and Back to sign-in. A late success still opens the app. The provider stays the only owner of stripping callback parameters. - confirmRestoredSession's user lookup had no deadline, and during a callback nothing else ends loading. It now times out after 4 s and, like any unanswered lookup, keeps the local session. Co-Authored-By: Claude Opus 5.5 --- src/lib/auth-callback.ts | 8 +++- src/lib/supabase/AuthProvider.tsx | 5 ++- src/locales/en.json | 5 ++- src/locales/es.json | 5 ++- src/locales/hu.json | 5 ++- src/pages/AuthCallbackPage.tsx | 20 ++++++++- src/test/auth/AuthProvider.behavior.test.tsx | 26 ++++++++++- src/test/auth/auth-callback-page.test.tsx | 46 +++++++++++++++----- 8 files changed, 99 insertions(+), 21 deletions(-) diff --git a/src/lib/auth-callback.ts b/src/lib/auth-callback.ts index e8bcfa892..12c4427d0 100644 --- a/src/lib/auth-callback.ts +++ b/src/lib/auth-callback.ts @@ -2,13 +2,17 @@ // the code; AuthCallbackPage waits for it and then opens `next` or /login. export const AUTH_CALLBACK_PATH = '/auth/callback'; -/** Why a callback did not sign anyone in. LoginPage shows a message for each. */ -export type AuthCallbackError = 'link-expired' | 'other-browser' | 'failed'; +/** + * Why a callback did not sign anyone in. LoginPage shows a message for each. + * `timed-out` is not a final answer: the code stays in the URL for a retry. + */ +export type AuthCallbackError = 'link-expired' | 'other-browser' | 'failed' | 'timed-out'; export const AUTH_CALLBACK_ERROR_KEYS: Record = { 'link-expired': 'login.errors.linkExpired', 'other-browser': 'login.errors.linkOtherBrowser', failed: 'login.errors.callbackFailed', + 'timed-out': 'login.errors.callbackTimedOut', }; export type AuthCallbackParams = { diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index c2bd43ebe..94f402467 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -144,7 +144,8 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { if (restoredSessionsBlocked()) return false; if (typeof supabase.auth.getUser !== 'function') return true; try { - const { error } = await supabase.auth.getUser(); + // Bounded: during a sign-in callback nothing else ends the wait. + const { error } = await withAuthTimeout(supabase.auth.getUser(), 'User lookup timeout'); if (!isDeadUserLookup(error)) return true; await rejectDeadSession(supabase.auth); return false; @@ -215,7 +216,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { settled = !(err instanceof AuthTimeoutError); // A session this browser already had still stands; getSession restores it below. console.warn('[Auth] sign-in callback failed:', err); - if (mounted) setCallbackError(authCallbackErrorFor(err)); + if (mounted) setCallbackError(settled ? authCallbackErrorFor(err) : 'timed-out'); return null; } finally { // Only once the exchange has an answer: a reload during the spinner, diff --git a/src/locales/en.json b/src/locales/en.json index 96a3cfc99..cf8ce3250 100644 --- a/src/locales/en.json +++ b/src/locales/en.json @@ -540,6 +540,8 @@ "signInWithDemoCode": "Sign in with demo code", "signingIn": "Signing in...", "finishingSignIn": "Signing you in…", + "retrySignIn": "Try again", + "backToSignIn": "Back to sign-in", "invalidEmail": "Enter a valid email address.", "invalidCode": "Enter a valid six-digit code.", "invalidDemoCode": "Enter a valid demo code.", @@ -563,7 +565,8 @@ "network": "Can't reach DebateLab. Check your connection and try again.", "linkExpired": "This sign-in link has expired or was already used. Send a new code.", "linkOtherBrowser": "This sign-in link only works in the browser where you asked for it. Send a new code here.", - "callbackFailed": "Sign-in didn't finish. Try again." + "callbackFailed": "Sign-in didn't finish. Try again.", + "callbackTimedOut": "Signing in is taking longer than it should. Try again." } }, "admin": { diff --git a/src/locales/es.json b/src/locales/es.json index b095af781..d6f4886af 100644 --- a/src/locales/es.json +++ b/src/locales/es.json @@ -540,6 +540,8 @@ "signInWithDemoCode": "Iniciar sesión con código demo", "signingIn": "Iniciando sesión...", "finishingSignIn": "Iniciando sesión…", + "retrySignIn": "Reintentar", + "backToSignIn": "Volver al inicio de sesión", "invalidEmail": "Introduce una dirección de email válida.", "invalidCode": "Introduce un código válido de seis dígitos.", "invalidDemoCode": "Introduce un código demo válido.", @@ -563,7 +565,8 @@ "network": "No se puede conectar con DebateLab. Revisa tu conexión e inténtalo de nuevo.", "linkExpired": "Este enlace de inicio de sesión ha caducado o ya se usó. Solicita un código nuevo.", "linkOtherBrowser": "Este enlace de inicio de sesión solo funciona en el navegador donde lo pediste. Solicita aquí un código nuevo.", - "callbackFailed": "No se completó el inicio de sesión. Inténtalo de nuevo." + "callbackFailed": "No se completó el inicio de sesión. Inténtalo de nuevo.", + "callbackTimedOut": "El inicio de sesión está tardando más de lo normal. Inténtalo de nuevo." } }, "admin": { diff --git a/src/locales/hu.json b/src/locales/hu.json index 5b32e76ab..d20449547 100644 --- a/src/locales/hu.json +++ b/src/locales/hu.json @@ -540,6 +540,8 @@ "signInWithDemoCode": "Bejelentkezés demókóddal", "signingIn": "Bejelentkezés...", "finishingSignIn": "Bejelentkezés…", + "retrySignIn": "Újrapróbálás", + "backToSignIn": "Vissza a bejelentkezéshez", "invalidEmail": "Adj meg érvényes e-mail-címet.", "invalidCode": "Adj meg érvényes, hatjegyű kódot.", "invalidDemoCode": "Adj meg érvényes demókódot.", @@ -563,7 +565,8 @@ "network": "A DebateLab nem érhető el. Ellenőrizd a kapcsolatot, és próbáld újra.", "linkExpired": "Ez a bejelentkezési link lejárt, vagy már felhasználták. Kérj új kódot.", "linkOtherBrowser": "Ez a bejelentkezési link csak abban a böngészőben működik, ahol kérted. Kérj itt új kódot.", - "callbackFailed": "A bejelentkezés nem fejeződött be. Próbáld újra." + "callbackFailed": "A bejelentkezés nem fejeződött be. Próbáld újra.", + "callbackTimedOut": "A bejelentkezés a szokásosnál tovább tart. Próbáld újra." } }, "admin": { diff --git a/src/pages/AuthCallbackPage.tsx b/src/pages/AuthCallbackPage.tsx index ad0bf9b2c..2b84ee328 100644 --- a/src/pages/AuthCallbackPage.tsx +++ b/src/pages/AuthCallbackPage.tsx @@ -1,9 +1,10 @@ import { useEffect } from 'react'; -import { Navigate, useSearchParams } from 'react-router-dom'; +import { Link, Navigate, useSearchParams } from 'react-router-dom'; import { useTranslation } from 'react-i18next'; import { Loader2 } from 'lucide-react'; import { useAuth } from '@/lib/supabase/AuthProvider'; import { useToast } from '@/hooks/use-toast'; +import { Button } from '@/components/ui/button'; import { AUTH_CALLBACK_ERROR_KEYS, safeNextPath } from '@/lib/auth-callback'; /** @@ -36,5 +37,22 @@ export default function AuthCallbackPage() { } if (user) return ; + + // The exchange did not answer in time. This URL still holds the unused code, + // so a reload retries it; a late success still opens the app from here. + if (callbackError === 'timed-out') { + return ( +
+

{t('login.errors.callbackTimedOut')}

+
+ + +
+
+ ); + } + return ; } diff --git a/src/test/auth/AuthProvider.behavior.test.tsx b/src/test/auth/AuthProvider.behavior.test.tsx index 2200e2f6b..5da4090c2 100644 --- a/src/test/auth/AuthProvider.behavior.test.tsx +++ b/src/test/auth/AuthProvider.behavior.test.tsx @@ -717,6 +717,30 @@ describe('AuthProvider behavior', () => { expect(screen.getByTestId('user')).toHaveTextContent('callback-user'); }); + it('ends the callback wait when the stored session\'s user lookup never answers', async () => { + vi.useFakeTimers(); + try { + window.history.replaceState({}, '', '/auth/callback?error=access_denied&error_code=otp_expired'); + getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); + getUserMock.mockReturnValue(new Promise(() => {})); + + render( + + + + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(4_000); + }); + // Auth did not answer: an outage keeps the stored session, and the page moves on. + expect(screen.getByTestId('loading')).toHaveTextContent('false'); + expect(screen.getByTestId('user')).toHaveTextContent('stored-user'); + } finally { + vi.useRealTimers(); + } + }); + it('keeps a callback error when the stored session is restored afterwards', async () => { window.history.replaceState({}, '', '/auth/callback?error=access_denied&error_code=otp_expired'); getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); @@ -809,7 +833,7 @@ describe('AuthProvider behavior', () => { expect(screen.getByTestId('loading')).toHaveTextContent('false'); expect(screen.getByTestId('user')).toHaveTextContent('none'); - expect(screen.getByTestId('callback-error')).toHaveTextContent('failed'); + expect(screen.getByTestId('callback-error')).toHaveTextContent('timed-out'); // A timeout is not a final answer: the code may still be good, so a reload can retry it. expect(window.location.search).toBe('?code=stalled-code'); diff --git a/src/test/auth/auth-callback-page.test.tsx b/src/test/auth/auth-callback-page.test.tsx index c31ca6f67..57ec9edbd 100644 --- a/src/test/auth/auth-callback-page.test.tsx +++ b/src/test/auth/auth-callback-page.test.tsx @@ -1,30 +1,42 @@ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { render, screen } from '@testing-library/react'; import { MemoryRouter, Route, Routes } from 'react-router-dom'; import AuthCallbackPage from '@/pages/AuthCallbackPage'; -const mocks = vi.hoisted(() => ({ toast: vi.fn(), clearCallbackError: vi.fn() })); +const mocks = vi.hoisted(() => ({ + toast: vi.fn(), + clearCallbackError: vi.fn(), + auth: { user: null as { id: string } | null, callbackError: null as string | null }, +})); vi.mock('@/hooks/use-toast', () => ({ useToast: () => ({ toast: mocks.toast }) })); vi.mock('@/lib/supabase/AuthProvider', () => ({ useAuth: () => ({ - user: { id: 'stored-user' }, + user: mocks.auth.user, loading: false, - callbackError: 'other-browser', + callbackError: mocks.auth.callbackError, clearCallbackError: mocks.clearCallbackError, }), })); +function renderCallback() { + render( + + + } /> + history
} /> + login} /> + + , + ); +} + describe('sign-in callback page', () => { + beforeEach(() => vi.clearAllMocks()); + it('still says the link failed when this browser keeps another session', () => { - render( - - - } /> - history} /> - - , - ); + mocks.auth = { user: { id: 'stored-user' }, callbackError: 'other-browser' }; + renderCallback(); expect(screen.getByText('history')).toBeInTheDocument(); expect(mocks.toast).toHaveBeenCalledWith(expect.objectContaining({ @@ -33,4 +45,14 @@ describe('sign-in callback page', () => { })); expect(mocks.clearCallbackError).toHaveBeenCalled(); }); + + it('stays on the callback after a timeout so a reload can retry the unused code', () => { + mocks.auth = { user: null, callbackError: 'timed-out' }; + renderCallback(); + + expect(screen.queryByText('login')).not.toBeInTheDocument(); + expect(screen.getByRole('alert')).toHaveTextContent('Signing in is taking longer than it should. Try again.'); + expect(screen.getByRole('button', { name: 'Try again' })).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Back to sign-in' })).toHaveAttribute('href', '/login'); + }); }); From 17e13f05f6aaa37b333888b43acb088c5b28e0ab Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 22:27:43 +0000 Subject: [PATCH 06/11] Keep a stored account out of a callback; settle late exchange answers Fresh review (Claude, replacing Codex r4, which hit a 429): - While a callback signs an account in, the listener no longer checks or loads a session the browser already had. A dead stored account's lookup could sign out, or block, the account just signed in. init loads the outcome: the new account, or the stored one only if the callback fails. The blocked-session branch also respects callbackPending. - After a timeout the exchange's late answer is final: a success clears the timeout notice and opens the app; a failure shows its real reason instead of a retry that would report a missing verifier. Either way the used code leaves the URL, and only while the callback page is still showing. - The timeout page shows before an existing session would open the app, and its Back to sign-in keeps the deep link. - A dead stored session's sign-out is bounded, so it cannot hold startup. Co-Authored-By: Claude Opus 5.5 --- src/lib/supabase/AuthProvider.tsx | 45 +++++++---- src/pages/AuthCallbackPage.tsx | 10 +-- src/test/auth/AuthProvider.behavior.test.tsx | 85 ++++++++++++++++++++ src/test/auth/auth-callback-page.test.tsx | 9 +++ 4 files changed, 130 insertions(+), 19 deletions(-) diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index 94f402467..25138dd78 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -147,12 +147,13 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { // Bounded: during a sign-in callback nothing else ends the wait. const { error } = await withAuthTimeout(supabase.auth.getUser(), 'User lookup timeout'); if (!isDeadUserLookup(error)) return true; - await rejectDeadSession(supabase.auth); - return false; } catch { // Auth did not answer. Keep the local session; this is an outage, not a dead login. return true; } + // The server refused this user. A sign-out that never answers must not hold startup either. + await withAuthTimeout(rejectDeadSession(supabase.auth), 'Sign-out timeout').catch((): void => undefined); + return false; })(); confirmations.set(accessToken, pending); return pending; @@ -191,7 +192,11 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { const hydrateOAuthCallbackSession = async (): Promise => { if (!callbackPending) return null; const { code, flowId, error } = readAuthCallbackParams(callbackUrl); - const cleanUrl = () => window.history.replaceState(window.history.state, '', withoutCallbackParams(callbackUrl)); + const cleanUrl = () => { + // A late answer can arrive after the user left the callback page. + if (window.location.pathname !== AUTH_CALLBACK_PATH) return; + window.history.replaceState(window.history.state, '', withoutCallbackParams(callbackUrl)); + }; if (error || !code) { cleanUrl(); if (error) setCallbackError(error); @@ -200,15 +205,12 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { // Finishing a sign-in is as explicit as pressing a sign-in button. allowSessionsAfterSignIn(); + // The flow id picks this sign-in's own verifier, so another tab's + // sign-in or a failed resend can't break the link. + const exchange = supabase.auth.exchangeCodeForSession(code, flowId ? { flowId } : undefined); let settled = true; try { - // The flow id picks this sign-in's own verifier, so another tab's - // sign-in or a failed resend can't break the link. - const { data, error: exchangeError } = await withAuthTimeout( - supabase.auth.exchangeCodeForSession(code, flowId ? { flowId } : undefined), - 'OAuth code exchange timeout', - OAUTH_CODE_EXCHANGE_TIMEOUT_MS, - ); + const { data, error: exchangeError } = await withAuthTimeout(exchange, 'OAuth code exchange timeout', OAUTH_CODE_EXCHANGE_TIMEOUT_MS); if (exchangeError) throw exchangeError; return data.session; } catch (err) { @@ -217,6 +219,17 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { // A session this browser already had still stands; getSession restores it below. console.warn('[Auth] sign-in callback failed:', err); if (mounted) setCallbackError(settled ? authCallbackErrorFor(err) : 'timed-out'); + if (!settled) { + // The late answer is final. A success is announced by auth-js and + // opens the app; a failure replaces the timeout with its real reason. + void exchange + .then(({ error: lateError }) => lateError, (lateError: unknown) => lateError ?? new Error('Exchange failed')) + .then((lateError) => { + if (!mounted) return; + setCallbackError(lateError ? authCallbackErrorFor(lateError) : null); + cleanUrl(); + }); + } return null; } finally { // Only once the exchange has an answer: a reload during the spinner, @@ -284,7 +297,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { if (newSession && restoredSessionsBlocked()) { dropRestoredSession(); - setLoading(false); + if (!callbackPending) setLoading(false); return; } @@ -324,6 +337,12 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { useDebateContextStore.getState().resetLocalData(); } + // While a callback signs an account in, init checks and loads the + // outcome: the new account on success, the stored one only if the + // callback fails. Checking a stored account here could sign out, or + // block, the account being signed in. + if (callbackPending) return; + if (newSession?.access_token) { void confirmRestoredSession(newSession.access_token).then((alive) => { if (!mounted || !alive || restoredSessionsBlocked()) { @@ -335,9 +354,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { } else { hydrateStores(nextUserId); } - // A session this browser already had is announced while the callback - // exchange runs; the callback page must wait for the account it signs in. - if (!callbackPending) setLoading(false); + setLoading(false); }); return () => { diff --git a/src/pages/AuthCallbackPage.tsx b/src/pages/AuthCallbackPage.tsx index 2b84ee328..3f4ac83ac 100644 --- a/src/pages/AuthCallbackPage.tsx +++ b/src/pages/AuthCallbackPage.tsx @@ -22,7 +22,7 @@ export default function AuthCallbackPage() { // The link failed, but this browser is still signed in to another session: // that account opens, and the failure is still said. useEffect(() => { - if (loading || !user || !callbackError) return; + if (loading || !user || !callbackError || callbackError === 'timed-out') return; toast({ title: t(AUTH_CALLBACK_ERROR_KEYS[callbackError]), variant: 'destructive' }); clearCallbackError(); }, [loading, user, callbackError, clearCallbackError, t, toast]); @@ -36,10 +36,9 @@ export default function AuthCallbackPage() { ); } - if (user) return ; - // The exchange did not answer in time. This URL still holds the unused code, - // so a reload retries it; a late success still opens the app from here. + // so a reload retries it; a late answer clears this and the page moves on. + // Checked before `user`: a session this browser already had must not hide it. if (callbackError === 'timed-out') { return (
@@ -47,12 +46,13 @@ export default function AuthCallbackPage() {
); } + if (user) return ; return ; } diff --git a/src/test/auth/AuthProvider.behavior.test.tsx b/src/test/auth/AuthProvider.behavior.test.tsx index 5da4090c2..bb01191b1 100644 --- a/src/test/auth/AuthProvider.behavior.test.tsx +++ b/src/test/auth/AuthProvider.behavior.test.tsx @@ -741,6 +741,91 @@ describe('AuthProvider behavior', () => { } }); + it('never lets a dead stored account sign out or block the account a callback signs in', async () => { + window.history.replaceState({}, '', '/auth/callback?code=pkce-code'); + let finishExchange!: (value: unknown) => void; + exchangeCodeForSessionMock.mockReturnValue(new Promise((resolve) => { finishExchange = resolve; })); + // The stored account was revoked elsewhere: its lookup is refused. + getUserMock.mockResolvedValue({ data: { user: null }, error: { status: 401, message: 'revoked' } }); + + render( + + + + ); + + act(() => callbackRef.current?.('INITIAL_SESSION', { user: { id: 'stored-user' }, access_token: 'stored' })); + await act(async () => { + finishExchange({ data: { session: { user: { id: 'callback-user' }, access_token: 'token' } }, error: null }); + }); + + await waitFor(() => expect(screen.getByTestId('loading')).toHaveTextContent('false')); + expect(screen.getByTestId('user')).toHaveTextContent('callback-user'); + expect(signOutMock).not.toHaveBeenCalled(); + expect(prepareSessionStoreMock).toHaveBeenCalledWith('callback-user'); + expect(hydrateSessionStoreMock).toHaveBeenCalled(); + }); + + it.each([ + ['opens the account and drops the timeout notice on a late success', { data: { session: { user: { id: 'late-user' }, access_token: 'token' } }, error: null }, 'none'], + ['gives the real reason on a late failure', { data: { session: null }, error: Object.assign(new Error('expired'), { code: 'flow_state_expired' }) }, 'link-expired'], + ])('after a callback timeout, %s', async (_name, lateResult, expectedError) => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + window.history.replaceState({}, '', '/auth/callback?code=slow-code&sb_flow_id=0123456789abcdef'); + let finishExchange!: (value: unknown) => void; + exchangeCodeForSessionMock.mockReturnValue(new Promise((resolve) => { finishExchange = resolve; })); + + render( + + + + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(15_000); + }); + expect(screen.getByTestId('callback-error')).toHaveTextContent('timed-out'); + expect(window.location.search).toBe('?code=slow-code&sb_flow_id=0123456789abcdef'); + + await act(async () => { + finishExchange(lateResult); + await vi.advanceTimersByTimeAsync(0); + }); + expect(screen.getByTestId('callback-error')).toHaveTextContent(expectedError); + // The answer is final now, so the used code leaves the URL. + expect(window.location.search).toBe(''); + } finally { + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); + + it('does not hold startup on a sign-out of a dead stored session that never answers', async () => { + vi.useFakeTimers(); + try { + getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); + getUserMock.mockResolvedValue({ data: { user: null }, error: { status: 401, message: 'revoked' } }); + signOutMock.mockReturnValue(new Promise(() => {})); + + render( + + + + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(4_000); + }); + expect(screen.getByTestId('loading')).toHaveTextContent('false'); + expect(screen.getByTestId('user')).toHaveTextContent('none'); + } finally { + signOutMock.mockReset(); + vi.useRealTimers(); + } + }); + it('keeps a callback error when the stored session is restored afterwards', async () => { window.history.replaceState({}, '', '/auth/callback?error=access_denied&error_code=otp_expired'); getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); diff --git a/src/test/auth/auth-callback-page.test.tsx b/src/test/auth/auth-callback-page.test.tsx index 57ec9edbd..c647f07d2 100644 --- a/src/test/auth/auth-callback-page.test.tsx +++ b/src/test/auth/auth-callback-page.test.tsx @@ -55,4 +55,13 @@ describe('sign-in callback page', () => { expect(screen.getByRole('button', { name: 'Try again' })).toBeInTheDocument(); expect(screen.getByRole('link', { name: 'Back to sign-in' })).toHaveAttribute('href', '/login'); }); + + it('keeps the retry after a timeout even when this browser has another session', () => { + mocks.auth = { user: { id: 'stored-user' }, callbackError: 'timed-out' }; + renderCallback(); + + expect(screen.queryByText('history')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Try again' })).toBeInTheDocument(); + expect(mocks.toast).not.toHaveBeenCalled(); + }); }); From 7f1fc05968fae6f7f3ffc40fb89136556454f4a4 Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 22:40:17 +0000 Subject: [PATCH 07/11] Let only the exchange's answer sign an account in during a callback Follow-up review: after a timeout, init restored and checked the stored account while the late exchange could still sign someone else in. A late success during that check got overwritten by the stored account (and the new account's rows loaded under the old owner), and a dead stored account could block the late success. While the code exchange has no answer, even past the timeout, the listener now ignores auth-js's events. init applies the answer itself: the new account on success, or the stored account through the usual server check once the exchange definitively failed. After a timeout nothing is restored until the late answer arrives. A late failure sets its error only while the callback page is still showing. applySession now also does what the listener did for these sign-ins: keep Google's provider token for Drive export and drop the previous account's Crossfire draft. Co-Authored-By: Claude Opus 5.5 --- src/lib/supabase/AuthProvider.tsx | 120 +++++++++++++------ src/test/auth/AuthProvider.behavior.test.tsx | 92 ++++++++++++++ 2 files changed, 175 insertions(+), 37 deletions(-) diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index 25138dd78..41498435d 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -88,6 +88,9 @@ class AuthTimeoutError extends Error { } } +/** What a code exchange that outlived its timeout finally answered. */ +type LateAnswer = { session: Session | null; lateError: unknown }; + function withAuthTimeout(operation: Promise, message: string, timeoutMs = AUTH_OPERATION_TIMEOUT_MS): Promise { return Promise.race([ operation, @@ -188,6 +191,55 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { // while the code is still being exchanged, and that must not end loading. const callbackUrl = new URL(window.location.href); let callbackPending = callbackUrl.pathname === AUTH_CALLBACK_PATH; + // True while the callback's code exchange has no answer, even past the + // timeout. auth-js keeps announcing sessions meanwhile (the stored + // account, then the exchange's own); only the exchange's answer may + // sign an account in, so the listener ignores them all. + let exchangeInFlight = false; + // The first account auth-js announced while the exchange ran: the one + // this browser had before the callback ('' for none). + let accountBeforeCallback: string | undefined; + + const applySession = (session: Session, previousUserId = currentUserIdRef.current) => { + // What the listener does for any other sign-in. + clearCrossfireDraftOnAuthChange(previousUserId || undefined, session.user?.id); + if (session.provider_token) sessionStorage.setItem('google_provider_token', session.provider_token); + setActiveAccessToken(session.access_token, session.user.id); + setSession(session); + currentUserIdRef.current = session.user?.id; + setJamUserId(session.user?.id); + hydrateStores(session.user?.id); + }; + + // The session this browser already had, checked with the server first. + const restoreStoredSession = async () => { + // In some browser states (stalled locks, blocked storage, extensions), + // getSession() can hang long enough to look like a frozen app. If it + // doesn't resolve quickly, fail open to the login route and rely on + // onAuthStateChange to populate the session when available. + const getSessionResult = await Promise.race([ + supabase.auth.getSession(), + new Promise((_, reject) => setTimeout(() => reject(new Error('getSession timeout')), AUTH_OPERATION_TIMEOUT_MS)), + ]); + + if (!mounted) return; + const { data, error } = getSessionResult; + if (error) console.warn('[Auth] getSession error:', error.message); + if (data.session?.access_token && !(await confirmRestoredSession(data.session.access_token))) { + if (!mounted) return; + dropRestoredSession(); + return; + } + if (!mounted) return; + if (data.session) { + applySession(data.session); + } else { + setActiveAccessToken(null); + setSession(null); + currentUserIdRef.current = undefined; + setJamUserId(undefined); + } + }; const hydrateOAuthCallbackSession = async (): Promise => { if (!callbackPending) return null; @@ -207,28 +259,44 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { allowSessionsAfterSignIn(); // The flow id picks this sign-in's own verifier, so another tab's // sign-in or a failed resend can't break the link. + exchangeInFlight = true; const exchange = supabase.auth.exchangeCodeForSession(code, flowId ? { flowId } : undefined); let settled = true; try { const { data, error: exchangeError } = await withAuthTimeout(exchange, 'OAuth code exchange timeout', OAUTH_CODE_EXCHANGE_TIMEOUT_MS); + exchangeInFlight = false; if (exchangeError) throw exchangeError; return data.session; } catch (err) { // A timeout is not a final answer: the code may still be good. settled = !(err instanceof AuthTimeoutError); - // A session this browser already had still stands; getSession restores it below. + if (settled) exchangeInFlight = false; + // A session this browser already had still stands; init restores it. console.warn('[Auth] sign-in callback failed:', err); if (mounted) setCallbackError(settled ? authCallbackErrorFor(err) : 'timed-out'); if (!settled) { - // The late answer is final. A success is announced by auth-js and - // opens the app; a failure replaces the timeout with its real reason. + // The late answer is final: a success signs that account in, a + // failure gives its real reason and restores the stored session. void exchange - .then(({ error: lateError }) => lateError, (lateError: unknown) => lateError ?? new Error('Exchange failed')) - .then((lateError) => { + .then( + ({ data, error: lateError }): LateAnswer => (lateError || !data.session ? { session: null, lateError } : { session: data.session, lateError: null }), + (lateError: unknown): LateAnswer => ({ session: null, lateError }), + ) + .then(async ({ session, lateError }) => { + exchangeInFlight = false; if (!mounted) return; - setCallbackError(lateError ? authCallbackErrorFor(lateError) : null); + const onCallbackPage = window.location.pathname === AUTH_CALLBACK_PATH; cleanUrl(); - }); + if (session) { + setCallbackError(null); + applySession(session, accountBeforeCallback); + return; + } + // Only the callback page still shows this attempt's outcome. + if (onCallbackPage) setCallbackError(authCallbackErrorFor(lateError)); + await restoreStoredSession(); + }) + .catch((lateError: unknown) => console.warn('[Auth] restoring the stored session failed:', lateError)); } return null; } finally { @@ -244,39 +312,13 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { if (!mounted) return; if (oauthSession) { - setActiveAccessToken(oauthSession.access_token, oauthSession.user.id); - setSession(oauthSession); - currentUserIdRef.current = oauthSession.user?.id; - setJamUserId(oauthSession.user?.id); - hydrateStores(oauthSession.user?.id); + applySession(oauthSession, accountBeforeCallback); return; } + // Timed out: the late answer decides which account, if any, opens. + if (exchangeInFlight) return; - // In some browser states (stalled locks, blocked storage, extensions), - // getSession() can hang long enough to look like a frozen app. If it - // doesn't resolve quickly, fail open to the login route and rely on - // onAuthStateChange to populate the session when available. - const getSessionResult = await Promise.race([ - supabase.auth.getSession(), - new Promise((_, reject) => setTimeout(() => reject(new Error('getSession timeout')), AUTH_OPERATION_TIMEOUT_MS)), - ]); - - if (!mounted) return; - const { data, error } = getSessionResult; - if (error) console.warn('[Auth] getSession error:', error.message); - if (data.session?.access_token && !(await confirmRestoredSession(data.session.access_token))) { - if (!mounted) return; - dropRestoredSession(); - return; - } - if (!mounted) return; - setActiveAccessToken(data.session?.access_token ?? null, data.session?.user.id ?? null); - setSession(data.session ?? null); - currentUserIdRef.current = data.session?.user?.id; - setJamUserId(data.session?.user?.id); - if (data.session) { - hydrateStores(data.session.user?.id); - } + await restoreStoredSession(); } catch (err) { console.warn('[Auth] init failed:', err); if (!mounted) return; @@ -294,6 +336,10 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { const { data: authListener } = supabase.auth.onAuthStateChange((event, newSession) => { if (import.meta.env.DEV) console.log('[Auth] Event:', event, 'Provider Token:', newSession?.provider_token ? 'present' : 'null'); + if (exchangeInFlight) { + accountBeforeCallback ??= newSession?.user?.id ?? ''; + return; + } if (newSession && restoredSessionsBlocked()) { dropRestoredSession(); diff --git a/src/test/auth/AuthProvider.behavior.test.tsx b/src/test/auth/AuthProvider.behavior.test.tsx index bb01191b1..9f8257c26 100644 --- a/src/test/auth/AuthProvider.behavior.test.tsx +++ b/src/test/auth/AuthProvider.behavior.test.tsx @@ -650,6 +650,29 @@ describe('AuthProvider behavior', () => { expect(window.location.search).toBe('?next=%2Fapp%2Fhistory'); }); + it('keeps Google\'s provider token and drops the previous account\'s draft after a callback sign-in', async () => { + sessionStorage.removeItem('google_provider_token'); + localStorage.setItem(crossfireStorageKey('stored-user'), '{"draft":true}'); + window.history.replaceState({}, '', '/auth/callback?code=pkce-code'); + let finishExchange!: (value: unknown) => void; + exchangeCodeForSessionMock.mockReturnValue(new Promise((resolve) => { finishExchange = resolve; })); + + render( + + + + ); + + act(() => callbackRef.current?.('INITIAL_SESSION', { user: { id: 'stored-user' }, access_token: 'stored' })); + await act(async () => { + finishExchange({ data: { session: { user: { id: 'google-user' }, access_token: 'token', provider_token: 'google-access' } }, error: null }); + }); + + await waitFor(() => expect(screen.getByTestId('user')).toHaveTextContent('google-user')); + expect(sessionStorage.getItem('google_provider_token')).toBe('google-access'); + expect(localStorage.getItem(crossfireStorageKey('stored-user'))).toBeNull(); + }); + it('exchanges a callback code without a flow id the old way', async () => { window.history.replaceState({}, '', '/auth/callback?code=pkce-code'); exchangeCodeForSessionMock.mockResolvedValue({ @@ -802,6 +825,75 @@ describe('AuthProvider behavior', () => { } }); + it('after a timeout, leaves the stored account alone until the exchange answers, then opens the new one', async () => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + window.history.replaceState({}, '', '/auth/callback?code=slow-code'); + getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); + let finishExchange!: (value: unknown) => void; + exchangeCodeForSessionMock.mockReturnValue(new Promise((resolve) => { finishExchange = resolve; })); + + render( + + + + ); + + act(() => callbackRef.current?.('INITIAL_SESSION', { user: { id: 'stored-user' }, access_token: 'stored' })); + await act(async () => { + await vi.advanceTimersByTimeAsync(15_000); + }); + // Nothing is restored or checked while the slow exchange may still sign someone in. + expect(screen.getByTestId('loading')).toHaveTextContent('false'); + expect(screen.getByTestId('user')).toHaveTextContent('none'); + expect(getSessionMock).not.toHaveBeenCalled(); + expect(getUserMock).not.toHaveBeenCalled(); + + await act(async () => { + finishExchange({ data: { session: { user: { id: 'callback-user' }, access_token: 'token' } }, error: null }); + await vi.advanceTimersByTimeAsync(0); + }); + expect(screen.getByTestId('user')).toHaveTextContent('callback-user'); + expect(prepareSessionStoreMock).toHaveBeenCalledWith('callback-user'); + expect(prepareSessionStoreMock).not.toHaveBeenCalledWith('stored-user'); + } finally { + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); + + it('after a timeout and a late failure, restores the stored account through the usual check', async () => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + window.history.replaceState({}, '', '/auth/callback?code=slow-code'); + getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); + let finishExchange!: (value: unknown) => void; + exchangeCodeForSessionMock.mockReturnValue(new Promise((resolve) => { finishExchange = resolve; })); + + render( + + + + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(15_000); + }); + await act(async () => { + finishExchange({ data: { session: null }, error: Object.assign(new Error('expired'), { code: 'flow_state_expired' }) }); + await vi.advanceTimersByTimeAsync(0); + }); + expect(screen.getByTestId('callback-error')).toHaveTextContent('link-expired'); + expect(getUserMock).toHaveBeenCalled(); + expect(screen.getByTestId('user')).toHaveTextContent('stored-user'); + } finally { + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); + it('does not hold startup on a sign-out of a dead stored session that never answers', async () => { vi.useFakeTimers(); try { From 074c9feea4a7ce945f529189e3c87b382d46634b Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 22:47:57 +0000 Subject: [PATCH 08/11] After a callback timeout, let the user act; guard the stored-session restore Follow-up review of 7f1fc059: - The listener ignored auth events until the slow exchange answered, so after the timeout a sign-in on /login was never seen (and never, if the request stalled for good). It now ignores them only until the exchange answers or times out. A late success arrives like any sign-in; it is applied directly only if a blocked restore dropped it. A late failure restores the stored session only if nobody signed in meanwhile. - restoreStoredSession drops its result when a different account was applied while the stored one was being checked. Co-Authored-By: Claude Opus 5.5 --- src/lib/supabase/AuthProvider.tsx | 33 +++++++---- src/test/auth/AuthProvider.behavior.test.tsx | 60 ++++++++++++++++++++ 2 files changed, 81 insertions(+), 12 deletions(-) diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index 41498435d..b09c42d9b 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -191,11 +191,13 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { // while the code is still being exchanged, and that must not end loading. const callbackUrl = new URL(window.location.href); let callbackPending = callbackUrl.pathname === AUTH_CALLBACK_PATH; - // True while the callback's code exchange has no answer, even past the - // timeout. auth-js keeps announcing sessions meanwhile (the stored - // account, then the exchange's own); only the exchange's answer may - // sign an account in, so the listener ignores them all. + // True until the callback's code exchange answers or times out. auth-js + // announces sessions meanwhile (the stored account, then the exchange's + // own); only the exchange's answer may sign an account in, so the + // listener ignores them. After a timeout the user can act again, so the + // listener does too. let exchangeInFlight = false; + let callbackTimedOut = false; // The first account auth-js announced while the exchange ran: the one // this browser had before the callback ('' for none). let accountBeforeCallback: string | undefined; @@ -231,6 +233,9 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { return; } if (!mounted) return; + // Someone else signed in while this was checked: theirs stands. + const current = currentUserIdRef.current; + if (current && current !== data.session?.user.id) return; if (data.session) { applySession(data.session); } else { @@ -270,31 +275,34 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { } catch (err) { // A timeout is not a final answer: the code may still be good. settled = !(err instanceof AuthTimeoutError); - if (settled) exchangeInFlight = false; + exchangeInFlight = false; + callbackTimedOut = !settled; // A session this browser already had still stands; init restores it. console.warn('[Auth] sign-in callback failed:', err); if (mounted) setCallbackError(settled ? authCallbackErrorFor(err) : 'timed-out'); if (!settled) { - // The late answer is final: a success signs that account in, a - // failure gives its real reason and restores the stored session. + // The late answer is final. A success is announced by auth-js like + // any sign-in; a failure gives its real reason and, if nobody signed + // in meanwhile, restores the stored session. void exchange .then( ({ data, error: lateError }): LateAnswer => (lateError || !data.session ? { session: null, lateError } : { session: data.session, lateError: null }), (lateError: unknown): LateAnswer => ({ session: null, lateError }), ) .then(async ({ session, lateError }) => { - exchangeInFlight = false; if (!mounted) return; const onCallbackPage = window.location.pathname === AUTH_CALLBACK_PATH; cleanUrl(); if (session) { setCallbackError(null); - applySession(session, accountBeforeCallback); + allowSessionsAfterSignIn(); + // The listener applies it, unless a blocked restore dropped it. + if (currentUserIdRef.current !== session.user.id) applySession(session); return; } // Only the callback page still shows this attempt's outcome. if (onCallbackPage) setCallbackError(authCallbackErrorFor(lateError)); - await restoreStoredSession(); + if (!currentUserIdRef.current) await restoreStoredSession(); }) .catch((lateError: unknown) => console.warn('[Auth] restoring the stored session failed:', lateError)); } @@ -315,8 +323,9 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { applySession(oauthSession, accountBeforeCallback); return; } - // Timed out: the late answer decides which account, if any, opens. - if (exchangeInFlight) return; + // Timed out: nothing is restored while the exchange may still sign + // someone in. Its late answer, or the user, decides. + if (callbackTimedOut) return; await restoreStoredSession(); } catch (err) { diff --git a/src/test/auth/AuthProvider.behavior.test.tsx b/src/test/auth/AuthProvider.behavior.test.tsx index 9f8257c26..85beb0389 100644 --- a/src/test/auth/AuthProvider.behavior.test.tsx +++ b/src/test/auth/AuthProvider.behavior.test.tsx @@ -894,6 +894,66 @@ describe('AuthProvider behavior', () => { } }); + it('after a timeout, still takes a sign-in made on /login, and a late failure does not undo it', async () => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + window.history.replaceState({}, '', '/auth/callback?code=slow-code'); + getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); + let finishExchange!: (value: unknown) => void; + exchangeCodeForSessionMock.mockReturnValue(new Promise((resolve) => { finishExchange = resolve; })); + + render( + + + + ); + + await act(async () => { + await vi.advanceTimersByTimeAsync(15_000); + }); + // The user gave up on the link and signed in with a code instead. + await act(async () => { + callbackRef.current?.('SIGNED_IN', { user: { id: 'code-user' }, access_token: 'code-token' }); + await vi.advanceTimersByTimeAsync(0); + }); + expect(screen.getByTestId('user')).toHaveTextContent('code-user'); + + await act(async () => { + finishExchange({ data: { session: null }, error: Object.assign(new Error('expired'), { code: 'flow_state_expired' }) }); + await vi.advanceTimersByTimeAsync(0); + }); + expect(screen.getByTestId('user')).toHaveTextContent('code-user'); + expect(prepareSessionStoreMock).not.toHaveBeenCalledWith('stored-user'); + } finally { + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); + + it('does not restore the stored account over one signed in during its check', async () => { + getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); + let answerLookup!: (value: unknown) => void; + getUserMock.mockImplementationOnce(() => new Promise((resolve) => { answerLookup = resolve; })); + + render( + + + + ); + + await waitFor(() => expect(getUserMock).toHaveBeenCalled()); + await act(async () => { + callbackRef.current?.('SIGNED_IN', { user: { id: 'new-user' }, access_token: 'new-token' }); + }); + await act(async () => { + answerLookup({ data: { user: { id: 'stored-user' } }, error: null }); + }); + + await waitFor(() => expect(screen.getByTestId('loading')).toHaveTextContent('false')); + expect(screen.getByTestId('user')).toHaveTextContent('new-user'); + }); + it('does not hold startup on a sign-out of a dead stored session that never answers', async () => { vi.useFakeTimers(); try { From f291919fda89f09f4e20144158294582925d4749 Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 22:51:40 +0000 Subject: [PATCH 09/11] Keep a newer sign-in when the restored stored account turns out dead Follow-up review of 074c9fee (Low): the guard that keeps an account signed in during the stored session's check ran only for a live stored account. It now runs before the dead branch's drop as well. Co-Authored-By: Claude Opus 5.5 --- src/lib/supabase/AuthProvider.tsx | 12 +++++----- src/test/auth/AuthProvider.behavior.test.tsx | 23 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/lib/supabase/AuthProvider.tsx b/src/lib/supabase/AuthProvider.tsx index b09c42d9b..7a3e72ec5 100644 --- a/src/lib/supabase/AuthProvider.tsx +++ b/src/lib/supabase/AuthProvider.tsx @@ -227,15 +227,15 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { if (!mounted) return; const { data, error } = getSessionResult; if (error) console.warn('[Auth] getSession error:', error.message); - if (data.session?.access_token && !(await confirmRestoredSession(data.session.access_token))) { - if (!mounted) return; - dropRestoredSession(); - return; - } + const alive = data.session?.access_token ? await confirmRestoredSession(data.session.access_token) : true; if (!mounted) return; - // Someone else signed in while this was checked: theirs stands. + // Someone else signed in while this was checked: theirs stands, alive or not. const current = currentUserIdRef.current; if (current && current !== data.session?.user.id) return; + if (!alive) { + dropRestoredSession(); + return; + } if (data.session) { applySession(data.session); } else { diff --git a/src/test/auth/AuthProvider.behavior.test.tsx b/src/test/auth/AuthProvider.behavior.test.tsx index 85beb0389..bccb1bff2 100644 --- a/src/test/auth/AuthProvider.behavior.test.tsx +++ b/src/test/auth/AuthProvider.behavior.test.tsx @@ -954,6 +954,29 @@ describe('AuthProvider behavior', () => { expect(screen.getByTestId('user')).toHaveTextContent('new-user'); }); + it('does not drop an account signed in while a dead stored account was being checked', async () => { + getSessionMock.mockResolvedValue({ data: { session: { user: { id: 'stored-user' }, access_token: 'stored' } }, error: null }); + let answerLookup!: (value: unknown) => void; + getUserMock.mockImplementationOnce(() => new Promise((resolve) => { answerLookup = resolve; })); + + render( + + + + ); + + await waitFor(() => expect(getUserMock).toHaveBeenCalled()); + await act(async () => { + callbackRef.current?.('SIGNED_IN', { user: { id: 'new-user' }, access_token: 'new-token' }); + }); + await act(async () => { + answerLookup({ data: { user: null }, error: { status: 401, message: 'revoked' } }); + }); + + await waitFor(() => expect(screen.getByTestId('loading')).toHaveTextContent('false')); + expect(screen.getByTestId('user')).toHaveTextContent('new-user'); + }); + it('does not hold startup on a sign-out of a dead stored session that never answers', async () => { vi.useFakeTimers(); try { From 9acb8e22930d78e6d1019a24d42cfe1c36ccc38a Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 23:36:55 +0000 Subject: [PATCH 10/11] Add callback fields to the SharedPage auth mock after merging main --- src/test/pages/SharedPage.test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/pages/SharedPage.test.tsx b/src/test/pages/SharedPage.test.tsx index 7497efc9d..adc24ff3d 100644 --- a/src/test/pages/SharedPage.test.tsx +++ b/src/test/pages/SharedPage.test.tsx @@ -16,6 +16,8 @@ const signedOut: AuthContextValue = { session: null, user: null, loading: false, + callbackError: null, + clearCallbackError: () => undefined, signInWithGoogle: async () => undefined, signInWithEmailOtp: async () => undefined, verifyEmailOtp: async () => undefined, From d2c9c994ee41bdec084b6f2916dfa5a5237ece49 Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 22 Oct 2026 23:52:19 +0000 Subject: [PATCH 11/11] Blur the paused timer button before the reset shortcut in the timer recipe (#172) --- .claude/skills/verify-debatelab/recipes/account.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/skills/verify-debatelab/recipes/account.spec.ts b/.claude/skills/verify-debatelab/recipes/account.spec.ts index dd2467476..ae9c62616 100644 --- a/.claude/skills/verify-debatelab/recipes/account.spec.ts +++ b/.claude/skills/verify-debatelab/recipes/account.spec.ts @@ -82,6 +82,8 @@ test("timer-run: the speech timer starts, pauses, and resets", async ({ signedIn await page.waitForTimeout(1_500); expect(await display.textContent()).toBe(paused); await evidence.shot("01-paused"); + // Timer shortcuts ignore a focused button (#172), and Pause still has focus. + await page.evaluate(() => (document.activeElement as HTMLElement | null)?.blur()); await page.keyboard.press("r"); await expect(display).toHaveText(initial!); await evidence.shot("02-reset");