From 15796c148b36f8a9c2e59f1c307e71df184acd61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 10 May 2026 21:29:17 +0200 Subject: [PATCH] fix(auth): move onboarding redirect from AuthShell useEffect to proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-hydration redirect for users with `onboardingCompletedAt === null` caused a brief dashboard flash before the client-side effect could fire. The redirect now lives in `src/proxy.ts` so it lands on the first server response. The proxy runs in the Edge runtime and can't reach Prisma, so the auth routes (login / passkey-verify / register / password / /api/auth/me) mirror the DB `onboardingCompletedAt` column into a non-httpOnly `hl_onboarding` cookie. The cookie is a UX hint, not a security signal — a user editing it locally only skips the dashboard flash; they still can't bypass any server check. `/api/onboarding/complete` clears the cookie so the next navigation drops the redirect immediately, and `destroySession` + expired-session purge wipe it alongside the session cookie. The e2e onboarding-flicker spec can return to testing the actual incomplete-onboarding case: the proxy reads the cookie (absent for the fully-onboarded storage-state user), not the mocked `/api/auth/me` response, so the test now sets `onboardingCompletedAt: null` honestly instead of pinning it to a non-null value as a workaround. Co-Authored-By: Claude Opus 4.7 (1M context) --- e2e/onboarding-flicker.spec.ts | 15 ++- e2e/setup/global-setup.ts | 8 +- .../proxy-onboarding-redirect.test.ts | 104 ++++++++++++++++++ .../auth/login/__tests__/native-token.test.ts | 1 + src/app/api/auth/login/route.ts | 5 +- src/app/api/auth/me/route.ts | 7 ++ .../__tests__/native-token.test.ts | 1 + .../api/auth/passkey/login-verify/route.ts | 5 +- src/app/api/auth/password/route.ts | 9 +- src/app/api/auth/register/route.ts | 6 +- src/app/api/onboarding/complete/route.ts | 5 + src/components/layout/auth-shell.tsx | 26 +---- src/lib/auth/session.ts | 33 ++++++ src/proxy.ts | 15 +++ 14 files changed, 208 insertions(+), 32 deletions(-) create mode 100644 src/__tests__/proxy-onboarding-redirect.test.ts diff --git a/e2e/onboarding-flicker.spec.ts b/e2e/onboarding-flicker.spec.ts index 55be4014d..e8d9150d4 100644 --- a/e2e/onboarding-flicker.spec.ts +++ b/e2e/onboarding-flicker.spec.ts @@ -139,10 +139,15 @@ test.describe("dashboard onboarding card flicker guard", () => { // card mounts collapsed: header + progress meter visible, full row // list hidden until the user clicks the chevron. // - // We intentionally pin `onboardingCompletedAt` to a non-null value: - // `` redirects users with a null timestamp straight to - // `/onboarding`, so the dashboard never paints. The checklist still - // renders here because `summaries: {}` keeps `measurementCount = 0`. + // v1.4.22 C4 — the onboarding redirect now lives in `proxy.ts`, + // not in ``. The proxy reads the `hl_onboarding` cookie + // (which the auth routes mirror from the DB). Our storageState + // belongs to a fully-onboarded user (no `hl_onboarding` cookie), + // so the proxy passes the dashboard through even when the mocked + // `/api/auth/me` reports `onboardingCompletedAt: null`. That's the + // exact case this test wants to cover — the checklist mounts + // because `measurementCount < 5` AND `onboardingCompletedAt` is + // null, but the page itself still paints. await page.route("**/api/auth/me", (route) => route.fulfill({ status: 200, @@ -157,7 +162,7 @@ test.describe("dashboard onboarding card flicker guard", () => { dateOfBirth: null, gender: null, timezone: "Europe/Berlin", - onboardingCompletedAt: "2025-01-01T00:00:00.000Z", + onboardingCompletedAt: null, // Suppress the spotlight tour overlay during this spec so // the onboarding-card visibility probe is not occluded by // the tour's z-200 dialog. The tour itself is exercised by diff --git a/e2e/setup/global-setup.ts b/e2e/setup/global-setup.ts index 0d6daf02a..e57e6897f 100644 --- a/e2e/setup/global-setup.ts +++ b/e2e/setup/global-setup.ts @@ -176,10 +176,12 @@ export default async function globalSetup(config: FullConfig): Promise { } const state = await ctx.storageState(); // Strip any cookies we don't recognise so storageState only carries - // what the auth shell needs. (Mostly defensive — the proxy currently - // only sets `healthlog_session` and `healthlog_locale`.) + // what the auth shell needs. v1.4.22 C4 added `hl_onboarding` — the + // login route sets it to "pending" for new users and DELETES it for + // already-onboarded ones, so for our pre-seeded completed e2e user + // the cookie is absent and the proxy passes the dashboard through. state.cookies = state.cookies.filter((c) => - ["healthlog_session", "healthlog_locale"].includes(c.name), + ["healthlog_session", "healthlog_locale", "hl_onboarding"].includes(c.name), ); await writeFile(STORAGE_STATE_PATH, JSON.stringify(state, null, 2)); await ctx.dispose(); diff --git a/src/__tests__/proxy-onboarding-redirect.test.ts b/src/__tests__/proxy-onboarding-redirect.test.ts new file mode 100644 index 000000000..bd8edb9ae --- /dev/null +++ b/src/__tests__/proxy-onboarding-redirect.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +/** + * v1.4.22 C4 — onboarding redirect moved from `` useEffect + * to `src/proxy.ts` so it lands on the first server response instead + * of post-hydration. The previous client-side approach caused a brief + * dashboard flash for users with `onboardingCompletedAt === null`. + * + * The proxy reads the `hl_onboarding` cookie (which the auth routes + * mirror from the DB `onboardingCompletedAt` column) without a DB + * roundtrip — Edge runtime can't reach Prisma. The cookie is a UX + * hint, not a security signal: a user editing it locally only skips + * the dashboard flash, they still can't bypass any server check. + */ + +vi.mock("@/lib/process-type", () => ({ + shouldRunWeb: () => true, +})); + +import { proxy } from "../proxy"; + +function makeRequest( + pathname: string, + cookies: Record = {}, +): NextRequest { + const cookieHeader = Object.entries(cookies) + .map(([k, v]) => `${k}=${v}`) + .join("; "); + return new NextRequest(`http://localhost${pathname}`, { + headers: cookieHeader ? { cookie: cookieHeader } : {}, + }); +} + +describe("proxy.ts onboarding redirect (v1.4.22 C4)", () => { + it("307s a session with hl_onboarding=pending from / to /onboarding", () => { + const res = proxy( + makeRequest("/", { + healthlog_session: "sess-1", + hl_onboarding: "pending", + }), + ); + // NextResponse.redirect returns a 307 by default in Next 16. + expect([307, 308]).toContain(res.status); + expect(res.headers.get("location")).toMatch(/\/onboarding$/); + }); + + it("307s a pending session from /insights to /onboarding", () => { + const res = proxy( + makeRequest("/insights", { + healthlog_session: "sess-1", + hl_onboarding: "pending", + }), + ); + expect([307, 308]).toContain(res.status); + expect(res.headers.get("location")).toMatch(/\/onboarding$/); + }); + + it("does NOT redirect from /onboarding itself", () => { + const res = proxy( + makeRequest("/onboarding", { + healthlog_session: "sess-1", + hl_onboarding: "pending", + }), + ); + // /onboarding is in PUBLIC_PATHS so the proxy passes through; the + // response is a NextResponse.next() with security headers applied. + expect(res.status).toBe(200); + expect(res.headers.get("location")).toBeNull(); + }); + + it("does NOT redirect from /auth/login", () => { + const res = proxy( + makeRequest("/auth/login", { hl_onboarding: "pending" }), + ); + expect(res.status).toBe(200); + expect(res.headers.get("location")).toBeNull(); + }); + + it("passes through when hl_onboarding cookie is absent (completed users)", () => { + const res = proxy(makeRequest("/", { healthlog_session: "sess-1" })); + expect(res.status).toBe(200); + expect(res.headers.get("location")).toBeNull(); + }); + + it("passes through API routes even when onboarding is pending", () => { + // API routes do their own getSession() checks — the proxy must + // never redirect them or every legitimate /api/* call would 307. + const res = proxy( + makeRequest("/api/measurements", { + healthlog_session: "sess-1", + hl_onboarding: "pending", + }), + ); + expect(res.status).toBe(200); + expect(res.headers.get("location")).toBeNull(); + }); + + it("still redirects unauthenticated users to /auth/login (existing contract)", () => { + const res = proxy(makeRequest("/")); + expect([307, 308]).toContain(res.status); + expect(res.headers.get("location")).toMatch(/\/auth\/login$/); + }); +}); diff --git a/src/app/api/auth/login/__tests__/native-token.test.ts b/src/app/api/auth/login/__tests__/native-token.test.ts index 389956c30..5597ef3f2 100644 --- a/src/app/api/auth/login/__tests__/native-token.test.ts +++ b/src/app/api/auth/login/__tests__/native-token.test.ts @@ -17,6 +17,7 @@ vi.mock("@/lib/auth/password", () => ({ vi.mock("@/lib/auth/session", () => ({ createSession: vi.fn().mockResolvedValue("session-id"), + setOnboardingPendingCookie: vi.fn().mockResolvedValue(undefined), })); vi.mock("@/lib/auth/audit", () => ({ diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index a6f820a2d..ba8c969e6 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -1,7 +1,7 @@ import { prisma } from "@/lib/db"; import { loginPasswordSchema } from "@/lib/validations/auth"; import { verifyPassword } from "@/lib/auth/password"; -import { createSession } from "@/lib/auth/session"; +import { createSession, setOnboardingPendingCookie } from "@/lib/auth/session"; import { auditLog } from "@/lib/auth/audit"; import { apiSuccess, @@ -77,6 +77,9 @@ export const POST = apiHandler(async (request: NextRequest) => { const ua = request.headers.get("user-agent"); await createSession(user.id, ip, ua); + // v1.4.22 C4 — mirror the user's onboarding status into a cookie so + // the proxy can short-circuit the redirect before the page hydrates. + await setOnboardingPendingCookie(user.onboardingCompletedAt == null); await auditLog("auth.login.password", { userId: user.id, diff --git a/src/app/api/auth/me/route.ts b/src/app/api/auth/me/route.ts index 3c31b6246..0a09736dd 100644 --- a/src/app/api/auth/me/route.ts +++ b/src/app/api/auth/me/route.ts @@ -2,12 +2,19 @@ import { apiSuccess } from "@/lib/api-response"; import { getGravatarUrl } from "@/lib/gravatar"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; +import { setOnboardingPendingCookie } from "@/lib/auth/session"; export const dynamic = "force-dynamic"; export const GET = apiHandler(async () => { const { user } = await requireAuth(); + // v1.4.22 C4 — keep the proxy-readable onboarding cookie in sync + // with the DB state on every /me roundtrip. Old sessions that + // predate the cookie pick it up on first dashboard load; the + // /onboarding/complete handler always wins on the cleared edge. + await setOnboardingPendingCookie(user.onboardingCompletedAt == null); + annotate({ action: { name: "auth.me" } }); return apiSuccess({ diff --git a/src/app/api/auth/passkey/login-verify/__tests__/native-token.test.ts b/src/app/api/auth/passkey/login-verify/__tests__/native-token.test.ts index 026f17940..b4f5f4030 100644 --- a/src/app/api/auth/passkey/login-verify/__tests__/native-token.test.ts +++ b/src/app/api/auth/passkey/login-verify/__tests__/native-token.test.ts @@ -15,6 +15,7 @@ vi.mock("@/lib/auth/passkey", () => ({ vi.mock("@/lib/auth/session", () => ({ createSession: vi.fn().mockResolvedValue("session-id"), + setOnboardingPendingCookie: vi.fn().mockResolvedValue(undefined), })); vi.mock("@/lib/auth/audit", () => ({ diff --git a/src/app/api/auth/passkey/login-verify/route.ts b/src/app/api/auth/passkey/login-verify/route.ts index a15a56f73..9e3783041 100644 --- a/src/app/api/auth/passkey/login-verify/route.ts +++ b/src/app/api/auth/passkey/login-verify/route.ts @@ -1,5 +1,5 @@ import { verifyAuthentication } from "@/lib/auth/passkey"; -import { createSession } from "@/lib/auth/session"; +import { createSession, setOnboardingPendingCookie } from "@/lib/auth/session"; import { auditLog } from "@/lib/auth/audit"; import { apiSuccess, @@ -67,6 +67,9 @@ export const POST = apiHandler(async (request: NextRequest) => { const ua = request.headers.get("user-agent"); await createSession(user.id, ip, ua); + // v1.4.22 C4 — mirror onboarding status into the proxy-readable cookie + // so the redirect short-circuits before the page hydrates. + await setOnboardingPendingCookie(user.onboardingCompletedAt == null); await auditLog("auth.login.passkey", { userId: user.id, diff --git a/src/app/api/auth/password/route.ts b/src/app/api/auth/password/route.ts index 0267d24b2..53ea9a206 100644 --- a/src/app/api/auth/password/route.ts +++ b/src/app/api/auth/password/route.ts @@ -16,7 +16,11 @@ import { import { checkRateLimit } from "@/lib/rate-limit"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; -import { destroyAllSessions, createSession } from "@/lib/auth/session"; +import { + destroyAllSessions, + createSession, + setOnboardingPendingCookie, +} from "@/lib/auth/session"; import { resolveServerLocale } from "@/lib/i18n/server-locale"; export const POST = apiHandler(async (request: NextRequest) => { @@ -82,6 +86,9 @@ export const POST = apiHandler(async (request: NextRequest) => { getClientIp(request), request.headers.get("user-agent"), ); + // v1.4.22 C4 — re-anchor the onboarding cookie to the user's real + // status after the session rebuild. + await setOnboardingPendingCookie(user.onboardingCompletedAt == null); await auditLog("auth.password.change", { userId: user.id, diff --git a/src/app/api/auth/register/route.ts b/src/app/api/auth/register/route.ts index d41413195..78a1c4dd5 100644 --- a/src/app/api/auth/register/route.ts +++ b/src/app/api/auth/register/route.ts @@ -1,7 +1,7 @@ import { prisma } from "@/lib/db"; import { registerSchema } from "@/lib/validations/auth"; import { hashPassword, checkPasswordStrength } from "@/lib/auth/password"; -import { createSession } from "@/lib/auth/session"; +import { createSession, setOnboardingPendingCookie } from "@/lib/auth/session"; import { auditLog } from "@/lib/auth/audit"; import { apiSuccess, @@ -97,6 +97,10 @@ export const POST = apiHandler(async (request: NextRequest) => { // Create session immediately const ua = request.headers.get("user-agent"); await createSession(user.id, ip, ua); + // v1.4.22 C4 — fresh users are always onboarding-pending. The cookie + // lets the proxy redirect on first navigation instead of waiting for + // `` to hydrate and trigger a useEffect bounce. + await setOnboardingPendingCookie(true); await auditLog("auth.register", { userId: user.id, diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index 99285fc88..bef7c65d3 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -2,6 +2,7 @@ import { prisma } from "@/lib/db"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; import { apiSuccess, apiError, safeJson } from "@/lib/api-response"; +import { setOnboardingPendingCookie } from "@/lib/auth/session"; import { NextRequest } from "next/server"; import { z } from "zod/v4"; @@ -56,5 +57,9 @@ export const POST = apiHandler(async (request: NextRequest) => { data, }); + // v1.4.22 C4 — clear the proxy-readable onboarding cookie so the + // next navigation drops the /onboarding redirect immediately. + await setOnboardingPendingCookie(false); + return apiSuccess({ completed: true }); }); diff --git a/src/components/layout/auth-shell.tsx b/src/components/layout/auth-shell.tsx index 4b1ceefb9..d2419d662 100644 --- a/src/components/layout/auth-shell.tsx +++ b/src/components/layout/auth-shell.tsx @@ -41,26 +41,12 @@ export function AuthShell({ children }: { children: React.ReactNode }) { } }, [isLoading, isAuthenticated, isAdminPage, user?.role, router]); - // Redirect to onboarding if not completed - useEffect(() => { - if ( - !isLoading && - isAuthenticated && - !isOnboardingPage && - !isPublicPage && - user && - !user.onboardingCompletedAt - ) { - router.replace("/onboarding"); - } - }, [ - isLoading, - isAuthenticated, - isOnboardingPage, - isPublicPage, - user, - router, - ]); + // v1.4.22 C4 — onboarding redirect moved to proxy.ts so it lands on + // the first server response instead of post-hydration. The proxy + // reads the `hl_onboarding` cookie (mirrored from the DB by the auth + // routes) and 307s to /onboarding before this shell ever paints. The + // previous `useEffect`-based redirect caused a brief dashboard flash + // for users with `onboardingCompletedAt === null`. // Loading state if (isLoading) { diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index 70177a150..6b433d5dc 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -7,6 +7,37 @@ import { getEvent } from "@/lib/logging/context"; const SESSION_COOKIE = "healthlog_session"; const SESSION_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000; // 30 days +/** + * v1.4.22 C4 — flag cookie mirroring the user's `onboardingCompletedAt` + * status. Set to `"pending"` while the field is null and cleared once + * onboarding completes. The proxy reads this cookie to short-circuit + * the post-hydration redirect that previously caused a dashboard flash + * before the client-side `` effect could fire. + * + * NOT httpOnly: the cookie is a UX hint, not a security signal — the + * real gate stays the server-side onboarding-complete check in + * `/api/onboarding/complete`. A user editing the cookie locally just + * skips the dashboard flash; they still can't bypass any data check. + */ +const ONBOARDING_COOKIE = "hl_onboarding"; + +export async function setOnboardingPendingCookie( + pending: boolean, +): Promise { + const cookieStore = await cookies(); + if (pending) { + cookieStore.set(ONBOARDING_COOKIE, "pending", { + httpOnly: false, + secure: process.env.NODE_ENV === "production", + sameSite: "lax", + maxAge: SESSION_MAX_AGE_MS / 1000, + path: "/", + }); + } else { + cookieStore.delete(ONBOARDING_COOKIE); + } +} + export async function createSession( userId: string, ipAddress?: string | null, @@ -63,6 +94,7 @@ export async function getSession(): Promise<{ await prisma.session.delete({ where: { id: sessionId } }); } cookieStore.delete(SESSION_COOKIE); + cookieStore.delete(ONBOARDING_COOKIE); return null; } @@ -99,6 +131,7 @@ export async function destroySession(): Promise { await prisma.session.delete({ where: { id: sessionId } }).catch(() => {}); } cookieStore.delete(SESSION_COOKIE); + cookieStore.delete(ONBOARDING_COOKIE); } export async function destroyAllSessions(userId: string): Promise { diff --git a/src/proxy.ts b/src/proxy.ts index 37309433a..a4f4a3e90 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -137,6 +137,21 @@ export function proxy(request: NextRequest) { if (!hasSession) { return NextResponse.redirect(new URL("/auth/login", request.url)); } + + // v1.4.22 C4 — 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. + const onboardingPending = + request.cookies.get("hl_onboarding")?.value === "pending"; + if (onboardingPending && !pathname.startsWith("/onboarding")) { + return NextResponse.redirect(new URL("/onboarding", request.url)); + } } // Generate or propagate x-request-id for request correlation