From 6c5a6eacbb37d37c13816caa65f4d3e5018737dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 21:14:04 +0100 Subject: [PATCH] fix(auth): bring a deep link back to its page after signing in A home-screen shortcut, a notification tap or a bookmark opened without a session landed on the sign-in page with a bare URL, and after signing in the person was on the dashboard instead of the page they asked for. The sign-in page already honoured a sanitised `next`; neither redirect set one. The proxy now passes the request's own path and query as `next` (the bare start page goes without one). The shell does the same when the account answer ends a session on a protected page, except after a deliberate sign-out: the marker a confirmed session leaves is gone by then, so the next person on the device starts from a bare sign-in page rather than the page the previous one left. --- .../baselines/version-stamp-comments.json | 4 +- .../proxy-early-exit-headers.test.ts | 2 +- .../proxy-onboarding-redirect.test.ts | 4 +- src/__tests__/proxy-sign-in-next.test.ts | 40 ++++++++++ .../auth-shell-session-end.browser.test.tsx | 74 +++++++++++++++++++ src/components/layout/auth-shell.tsx | 44 ++++++----- src/proxy.ts | 21 +++--- 7 files changed, 152 insertions(+), 37 deletions(-) create mode 100644 src/__tests__/proxy-sign-in-next.test.ts diff --git a/src/__tests__/baselines/version-stamp-comments.json b/src/__tests__/baselines/version-stamp-comments.json index fc64b4a07..89b8b678b 100644 --- a/src/__tests__/baselines/version-stamp-comments.json +++ b/src/__tests__/baselines/version-stamp-comments.json @@ -1,7 +1,7 @@ { "$comment": "Comment lines that open with a release version, per file under src/. Maintained by src/__tests__/version-stamp-ratchet.test.ts.", "count": 2892, - "total": 9220, + "total": 9217, "entries": { "__tests__/account-payload-consumer-guard.test.ts": 2, "__tests__/acting-account-boundary-guard.test.ts": 7, @@ -2892,7 +2892,7 @@ "lib/withings/sync.ts": 6, "lib/withings/webhook-handler.ts": 3, "lib/workouts/list-read.ts": 1, - "proxy.ts": 27, + "proxy.ts": 26, "types/analytics.ts": 8, "types/backups.ts": 1 } diff --git a/src/__tests__/proxy-early-exit-headers.test.ts b/src/__tests__/proxy-early-exit-headers.test.ts index bfa02f8c5..fed0da4d9 100644 --- a/src/__tests__/proxy-early-exit-headers.test.ts +++ b/src/__tests__/proxy-early-exit-headers.test.ts @@ -87,7 +87,7 @@ describe("proxy early exits carry the baseline security headers", () => { it("unauthenticated page redirect to /auth/login", () => { const res = proxy(makeRequest("/dashboard")); - expect(res.headers.get("location")).toMatch(/\/auth\/login$/); + expect(res.headers.get("location")).toMatch(/\/auth\/login(\?|$)/); expectBaselineHeaders(res); }); diff --git a/src/__tests__/proxy-onboarding-redirect.test.ts b/src/__tests__/proxy-onboarding-redirect.test.ts index ee609e893..68ea922fa 100644 --- a/src/__tests__/proxy-onboarding-redirect.test.ts +++ b/src/__tests__/proxy-onboarding-redirect.test.ts @@ -82,7 +82,7 @@ describe("proxy.ts onboarding redirect (v1.4.22 C4)", () => { expect(res.status).toBe(200); expect(res.headers.get("location")).toBeNull(); const anonymous = proxy(makeRequest("/insights")); - expect(anonymous.headers.get("location")).toMatch(/\/auth\/login$/); + expect(anonymous.headers.get("location")).toMatch(/\/auth\/login(\?|$)/); }); it("passes through when hl_onboarding cookie is absent (completed users)", () => { @@ -123,7 +123,7 @@ describe("proxy.ts onboarding redirect (v1.4.22 C4)", () => { // were still in place it would short-circuit through public-path // handling and 200 instead. expect([307, 308]).toContain(res.status); - expect(res.headers.get("location")).toMatch(/\/auth\/login$/); + expect(res.headers.get("location")).toMatch(/\/auth\/login(\?|$)/); }); it("still treats subroutes /onboarding/ as the onboarding surface", () => { diff --git a/src/__tests__/proxy-sign-in-next.test.ts b/src/__tests__/proxy-sign-in-next.test.ts new file mode 100644 index 000000000..d9740c6f4 --- /dev/null +++ b/src/__tests__/proxy-sign-in-next.test.ts @@ -0,0 +1,40 @@ +/** + * The edge sends a visitor without a session to the sign-in page with the + * page they asked for in `next`, so a home-screen shortcut or a notification + * tap still arrives there after signing in. The sign-in page re-checks the + * value (`sanitizeSameOriginPath`); the proxy only ever puts in the + * request's own path and query. + */ +import { describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/process-type", () => ({ + shouldRunWeb: () => true, +})); + +import { proxy } from "../proxy"; + +function signInRedirect(pathAndQuery: string): URL { + const res = proxy(new NextRequest(`http://localhost${pathAndQuery}`)); + expect([307, 308]).toContain(res.status); + return new URL(res.headers.get("location")!, "http://localhost"); +} + +describe("proxy.ts sign-in redirect", () => { + it.each([ + "/measurements?add=WEIGHT&source=shortcut", + "/mood?add=1", + "/medications", + "/?source=pwa", + ])("carries %s to the sign-in page", (target) => { + const location = signInRedirect(target); + expect(location.pathname).toBe("/auth/login"); + expect(location.searchParams.get("next")).toBe(target); + }); + + it("sends the bare start page to a bare sign-in page", () => { + const location = signInRedirect("/"); + expect(location.pathname).toBe("/auth/login"); + expect(location.search).toBe(""); + }); +}); diff --git a/src/components/layout/__tests__/auth-shell-session-end.browser.test.tsx b/src/components/layout/__tests__/auth-shell-session-end.browser.test.tsx index 7e2d7578d..5a5e5ebd0 100644 --- a/src/components/layout/__tests__/auth-shell-session-end.browser.test.tsx +++ b/src/components/layout/__tests__/auth-shell-session-end.browser.test.tsx @@ -5,8 +5,15 @@ * so a session that ended without a sign-out (an expired cookie the edge sent * straight to the sign-in page) leaves nothing on the device for the next * account. It never clears on an answer that is not a verdict. + * + * On a protected page it sends the visitor to the sign-in page, carrying the + * page along when the session ended on its own, and not after a deliberate + * sign-out: the next person on the device starts from a bare sign-in page. */ import { afterEach, describe, expect, it, vi } from "vitest"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { I18nProvider } from "@/lib/i18n/context"; import { authUser, renderWithApp } from "../../../../tests/browser/render"; import { stubFetch } from "../../../../tests/browser/fetch-stub"; @@ -42,6 +49,8 @@ vi.mock("@/components/command-palette/command-palette.lazy", () => ({ })); vi.mock("../global-shortcuts", () => ({ GlobalShortcuts: () => null })); +import { clearCachesForSessionEnd } from "@/hooks/use-auth"; + import { AuthShell } from "../auth-shell"; const ME = "/api/auth/me"; @@ -50,6 +59,19 @@ function openAt(path: string): void { window.history.replaceState(null, "", path); } +/** The tree `renderWithApp` mounts, for rendering it again. */ +function shellTree(queryClient: QueryClient) { + return ( + + + +
+ + + + ); +} + async function mountShell(user: Parameters[0] | null) { return renderWithApp( @@ -125,3 +147,55 @@ describe("AuthShell on a sign-in page", () => { expect(clearOffline).not.toHaveBeenCalled(); }); }); + +describe("AuthShell on a protected page", () => { + /** Signed in on `path`, with the marker a confirmed session leaves. */ + async function signedInAt(path: string) { + let session = true; + stubFetch().on("GET", ME, () => + session + ? { data: authUser({ id: "signed-in" }) } + : { status: 401, error: "Unauthorized" }, + ); + openAt(path); + const view = await mountShell(null); + await expect + .poll(() => document.querySelector('[data-slot="page"]')) + .toBeTruthy(); + return { + ...view, + endSession: () => { + session = false; + }, + }; + } + + it("returns to the page after a session that ended on its own", async () => { + // Signed in once on this browser; the session has since expired while + // the cookie stayed, so the next visit gets past the edge and the + // account answer is the first to say so. + const first = await signedInAt("/"); + first.endSession(); + await first.screen.unmount(); + openAt("/measurements?add=WEIGHT"); + await mountShell(null); + + await expect.poll(() => window.location.pathname).toBe("/auth/login"); + expect(new URLSearchParams(window.location.search).get("next")).toBe( + "/measurements?add=WEIGHT", + ); + }); + + it("does not carry the page past a deliberate sign-out", async () => { + const { screen, queryClient, endSession } = + await signedInAt("/medications"); + // What the sign-out does once the server confirmed it; the navigation + // it starts renders the shell again, which asks for the account anew. + endSession(); + clearCachesForSessionEnd(queryClient); + await screen.rerender(shellTree(queryClient)); + + await expect.poll(() => window.location.pathname).toBe("/auth/login"); + expect(window.location.search).toBe(""); + }); +}); diff --git a/src/components/layout/auth-shell.tsx b/src/components/layout/auth-shell.tsx index 9610fc3cc..e6b519dc7 100644 --- a/src/components/layout/auth-shell.tsx +++ b/src/components/layout/auth-shell.tsx @@ -10,9 +10,14 @@ import { MaintainershipBanner } from "@/components/i18n/maintainership-banner"; import { LayoutCoachFab } from "@/components/insights/layout-coach-fab"; import { LayoutCoachMount } from "@/components/insights/layout-coach-mount"; import { TourLauncher } from "@/components/onboarding/tour-launcher"; -import { clearCachesForSessionEnd, useAuth } from "@/hooks/use-auth"; +import { + clearCachesForSessionEnd, + readWasAuthenticated, + useAuth, +} from "@/hooks/use-auth"; import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { useRecordSessionTransition } from "@/hooks/use-record-session-transition"; +import { signInHref } from "@/lib/api/read-failure"; import { useTranslations } from "@/lib/i18n/context"; import { CoachLaunchProvider } from "@/lib/insights/coach-launch-context"; import { clearOfflineCachesForSessionEnd } from "@/lib/pwa/query-persister"; @@ -243,27 +248,26 @@ export function AuthShell({ window.scrollTo(0, 0); }, [pathname]); + // Session END on a protected page (`/api/auth/me` answered 401/403): wipe + // every client cache, since the QueryClient outlives navigation and health + // data is not user-scoped, then go to the sign-in page. Logout does the same + // from `useLogout`; this covers the expiry path it never runs through. No + // verdict (`isAuthUnknown`: offline, 5xx, no marker) never wipes: a wipe on + // a network failure is what used to destroy the offline caches. + // + // The target is fixed once per signed-out stretch, before the wipe clears + // the marker it reads (the refetch that follows runs this again). A session + // that ended on its own returns here after the next sign-in; a deliberate + // sign-out already cleared the marker, so the next person gets a bare page. + const signInTarget = useRef(null); useEffect(() => { + if (isAuthenticated || isPublicPage) signInTarget.current = null; if (!isLoading && !isAuthenticated && !isPublicPage) { - // Session END (expired / invalidated cookie → `/api/auth/me` 401/403). - // Wipe the in-memory query cache (the cross-user guard — the - // QueryClient outlives the navigation on this SPA and the health-data - // families are not user-scoped) plus the persisted IndexedDB + SW - // caches, so nothing leaks to the next account on a shared browser. - // Logout does the same from `useLogout`; this covers the expiry path it - // never runs through. Best-effort; the redirect proceeds regardless. - // - // `isAuthUnknown` = the auth probe failed WITHOUT a session verdict - // (offline relaunch, network blip, 5xx) and no prior-session marker - // exists. Nothing to protect and nothing to render — send the visitor - // to the login gate, but never run the wipe off a non-verdict: the - // wipe on a network failure is exactly what used to destroy the - // offline caches. (With the marker present, `isAuthenticated` holds - // the last-known state and this branch never runs.) - if (!isAuthUnknown) { - clearCachesForSessionEnd(queryClient); - } - router.replace("/auth/login"); + signInTarget.current ??= readWasAuthenticated() + ? signInHref() + : "/auth/login"; + if (!isAuthUnknown) clearCachesForSessionEnd(queryClient); + router.replace(signInTarget.current); } }, [ isLoading, diff --git a/src/proxy.ts b/src/proxy.ts index 4fc8219cb..1d337dc3e 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -565,20 +565,17 @@ export function proxy(request: NextRequest) { const isSessionGatedPage = !isApiRoute && !isStaticFile && !isPublic; if (isSessionGatedPage) { if (!readSessionCookie(request.cookies)) { - return applyBaselineSecurityHeaders( - NextResponse.redirect(new URL("/auth/login", request.url)), - ); + // Back here after signing in; the sign-in page re-checks `next`. + const signIn = new URL("/auth/login", request.url); + const here = `${pathname}${request.nextUrl.search}`; + if (here !== "/") signIn.searchParams.set("next", here); + return applyBaselineSecurityHeaders(NextResponse.redirect(signIn)); } - // v1.4.22 — server-side onboarding redirect. Previously the - // `` ran this in a post-hydration `useEffect`, so a user - // with `onboardingCompletedAt === null` briefly saw the dashboard - // flash before the redirect fired. The auth flows (login, passkey, - // register, /api/auth/me) keep an `hl_onboarding` cookie in sync - // with the DB state; the proxy reads it without a DB roundtrip so - // the redirect lands on the first server response. The /onboarding - // page itself is in PUBLIC_PATHS above, so this branch only runs - // for the surfaces that need to redirect away. + // The onboarding redirect, here so it lands on the first response with no + // dashboard flash. The auth flows (login, passkey, register, + // /api/auth/me) keep the `hl_onboarding` cookie in step with the DB, so no + // DB round trip; `/onboarding` itself is public above. const onboardingPending = request.cookies.get("hl_onboarding")?.value === "pending"; // v1.4.22 — exact match the redirect