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/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"); 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 e37d5b931..66bd7c3b1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -132,6 +132,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 2247c956a..049b005b9 100644 --- a/public/theme-init.js +++ b/public/theme-init.js @@ -21,9 +21,9 @@ try { var pathname = window.location.pathname; var privateRoute = pathname === '/app' || pathname.indexOf('/app/') === 0 - || pathname === '/login' + || pathname === '/login' || pathname === '/auth/callback' || pathname === '/share' || pathname.indexOf('/share/') === 0; - 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'); } // The shell HTML is noindex. Private routes keep that. The sources page is diff --git a/src/App.tsx b/src/App.tsx index 432da506b..05ae4116c 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,29 @@ 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); + // A tour left running by the previous account never opens for one that finished it. + const shouldLoadTour = !completed && (isTourRunning || location.pathname === "/app"); - 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 +291,7 @@ function AppRoutes() { } /> } /> } /> + } /> } /> } /> } /> diff --git a/src/components/auth/RequireAuth.tsx b/src/components/auth/RequireAuth.tsx index 22e876317..4aa42f6ef 100644 --- a/src/components/auth/RequireAuth.tsx +++ b/src/components/auth/RequireAuth.tsx @@ -1,3 +1,4 @@ +import { useEffect, useRef } from 'react'; import { Navigate, useLocation } from 'react-router-dom'; import { useAuth } from '@/lib/supabase/AuthProvider'; import { Loader2 } from 'lucide-react'; @@ -6,6 +7,12 @@ 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); + useEffect(() => { + if (user) lastUserId.current = user.id; + }, [user]); if (loading) { return ( @@ -17,7 +24,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]); + +/** 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); +} + 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 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 new file mode 100644 index 000000000..12c4427d0 --- /dev/null +++ b/src/lib/auth-callback.ts @@ -0,0 +1,89 @@ +// 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. + * `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 = { + 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 640723f85..dedebc9df 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..7a3e72ec5 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'; @@ -80,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, @@ -104,6 +115,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); @@ -135,14 +147,16 @@ 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; } 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; @@ -173,39 +187,131 @@ 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; + // 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; + + 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); + }; - const { data, error } = result; - if (error) throw error; - return data.session; + // 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); + 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, 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 { + setActiveAccessToken(null); + setSession(null); + currentUserIdRef.current = undefined; + setJamUserId(undefined); + } + }; + + const hydrateOAuthCallbackSession = async (): Promise => { + if (!callbackPending) return null; + const { code, flowId, error } = readAuthCallbackParams(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); + return null; } - return null; + // 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. + 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); + 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 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 }) => { + if (!mounted) return; + const onCallbackPage = window.location.pathname === AUTH_CALLBACK_PATH; + cleanUrl(); + if (session) { + setCallbackError(null); + 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)); + if (!currentUserIdRef.current) await restoreStoredSession(); + }) + .catch((lateError: unknown) => console.warn('[Auth] restoring the stored session failed:', lateError)); + } + return null; + } finally { + // 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(); + } }; const init = async () => { @@ -214,39 +320,14 @@ 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: nothing is restored while the exchange may still sign + // someone in. Its late answer, or the user, decides. + if (callbackTimedOut) 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; @@ -255,6 +336,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { currentUserIdRef.current = undefined; setJamUserId(undefined); } finally { + callbackPending = false; if (mounted) setLoading(false); } }; @@ -263,10 +345,14 @@ 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(); - setLoading(false); + if (!callbackPending) setLoading(false); return; } @@ -293,7 +379,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { resetDebateStore(); useDebateContextStore.getState().resetLocalData(); clearGoogleTokens(); - setLoading(false); + if (!callbackPending) setLoading(false); return; } @@ -306,6 +392,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()) { @@ -350,7 +442,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 +454,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 +466,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 +513,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 +531,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 d17bc57c9..e0d4edbbb 100644 --- a/src/lib/supabase/client.ts +++ b/src/lib/supabase/client.ts @@ -76,6 +76,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 ac74c0682..ad12179f1 100644 --- a/src/locales/en.json +++ b/src/locales/en.json @@ -546,6 +546,9 @@ "demoCodePlaceholder": "DEMO-AB12-CD34", "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.", @@ -566,7 +569,11 @@ "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.", + "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 aa77d04c7..2168cca84 100644 --- a/src/locales/es.json +++ b/src/locales/es.json @@ -546,6 +546,9 @@ "demoCodePlaceholder": "DEMO-AB12-CD34", "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.", @@ -566,7 +569,11 @@ "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.", + "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 70623599e..abc15bf14 100644 --- a/src/locales/hu.json +++ b/src/locales/hu.json @@ -546,6 +546,9 @@ "demoCodePlaceholder": "DEMO-AB12-CD34", "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.", @@ -566,7 +569,11 @@ "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.", + "callbackTimedOut": "A bejelentkezés a szokásosnál tovább tart. 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..3f4ac83ac --- /dev/null +++ b/src/pages/AuthCallbackPage.tsx @@ -0,0 +1,58 @@ +import { useEffect } from 'react'; +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'; + +/** + * 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, 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 || callbackError === 'timed-out') return; + toast({ title: t(AUTH_CALLBACK_ERROR_KEYS[callbackError]), variant: 'destructive' }); + clearCallbackError(); + }, [loading, user, callbackError, clearCallbackError, t, toast]); + + if (loading) { + return ( +
+
+ ); + } + + // The exchange did not answer in time. This URL still holds the unused code, + // 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 ( +
+

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

+
+ + +
+
+ ); + } + + if (user) return ; + return ; +} diff --git a/src/pages/LoginPage.tsx b/src/pages/LoginPage.tsx index 10ddc1eb3..beaee465d 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 { AUTH_CALLBACK_ERROR_KEYS } from '@/lib/auth-callback'; type AuthMode = 'email' | 'demo'; type AuthStatus = 'idle' | 'submitting' | 'code-sent' | 'verifying' | 'success' | 'error'; @@ -30,8 +31,11 @@ function isAuthMode(value: string): value is AuthMode { return value === 'email' || value === 'demo'; } +/** 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 +46,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 +56,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 +89,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 +119,7 @@ export default function LoginPage() { setErrorMessage(''); setAuthStatus('submitting'); try { - await signInWithEmailOtp(normalizedEmail); + await signInWithEmailOtp(normalizedEmail, { next }); setOtp(''); setEmailSent(true); setAuthStatus('code-sent'); @@ -137,7 +154,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 +213,11 @@ export default function LoginPage() { {t('login.sessionExpired', 'Your session expired. Sign in again.')}

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

+ {t(AUTH_CALLBACK_ERROR_KEYS[linkError])} +

+ )} diff --git a/src/prerender/entry.tsx b/src/prerender/entry.tsx index 91db11976..5f27282f7 100644 --- a/src/prerender/entry.tsx +++ b/src/prerender/entry.tsx @@ -21,6 +21,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..bccb1bff2 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,432 @@ 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('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({ + 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'); + // 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 }); + }); + + await waitFor(() => expect(screen.getByTestId('user')).toHaveTextContent('pkce-user')); + expect(screen.getByTestId('loading')).toHaveTextContent('false'); + }); + + it('stays loading through the stored session\'s events until another account\'s callback exchange settles', async () => { + window.history.replaceState({}, '', '/auth/callback?code=pkce-code'); + let finishExchange!: (value: unknown) => void; + exchangeCodeForSessionMock.mockReturnValue(new Promise((resolve) => { finishExchange = resolve; })); + + render( + + + + ); + + // auth-js restores the account already stored in this browser while B's code is exchanged. + act(() => callbackRef.current?.('INITIAL_SESSION', { user: { id: 'stored-user' }, access_token: 'stored' })); + expect(screen.getByTestId('loading')).toHaveTextContent('true'); + + 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'); + }); + + 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('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('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(() => {}); - window.history.replaceState({}, '', '/app?code=stalled-code'); + 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('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 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 { + 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 }); + + render( + + + + ); + + await waitFor(() => expect(screen.getByTestId('loading')).toHaveTextContent('false')); + act(() => callbackRef.current?.('SIGNED_IN', { user: { id: 'stored-user' }, access_token: 'stored' })); + expect(screen.getByTestId('callback-error')).toHaveTextContent('link-expired'); + }); + + 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({}, '', '/auth/callback?code=stalled-code'); exchangeCodeForSessionMock.mockReturnValue(new Promise(() => {})); render( @@ -662,14 +1080,21 @@ 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(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'); warnSpy.mockRestore(); @@ -696,7 +1121,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 +1131,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-page.test.tsx b/src/test/auth/auth-callback-page.test.tsx new file mode 100644 index 000000000..c647f07d2 --- /dev/null +++ b/src/test/auth/auth-callback-page.test.tsx @@ -0,0 +1,67 @@ +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(), + 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: mocks.auth.user, + loading: false, + 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', () => { + mocks.auth = { user: { id: 'stored-user' }, callbackError: 'other-browser' }; + renderCallback(); + + 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(); + }); + + 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'); + }); + + 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(); + }); +}); 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..242718340 --- /dev/null +++ b/src/test/auth/onboarding-per-account.test.ts @@ -0,0 +1,54 @@ +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); + }); + + 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); + }); +}); 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, diff --git a/src/test/vercel-deployment-config.test.ts b/src/test/vercel-deployment-config.test.ts index 7c395953e..be91ad859 100644 --- a/src/test/vercel-deployment-config.test.ts +++ b/src/test/vercel-deployment-config.test.ts @@ -83,7 +83,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. @@ -277,7 +277,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 b8838e71f..5dbaefb7d 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": [