Implement completeAuth chokepoint + caller migration

Implements all of unify-auth-completion (12/14 tasks done; manual
smoke + archive-time spec sync remain).

Design refinement during implementation: completeAuth supports two
response shapes via a `mode` parameter:
- mode: 'redirect' (loaders / direct browser navigation; auth.verify.tsx)
- mode: 'json' (action handlers called by imperative fetch from
  client forms; api.auth.login, api.auth.register)

Both modes share createSession + safeReturnTo + Set-Cookie. JSON mode
carries `{ ok: true, step: "done", redirectTo }` (the `step` field
preserves the existing client-form check).

Why two modes: passkey ceremonies are inherently imperative
(start → browser WebAuthn API → finish), so action handlers can't
move to <Form>/useFetcher. Picking option (B) from the design grill —
the chokepoint owns destination selection while clients navigate —
required this dual shape. The 3 hardcoded client-side targets
(returnTo ?? "/", "/", "/?add-passkey=1") collapse into 1 server-side
sanitization pass (safeReturnTo) inside completeAuth.

New module:
- apps/journal/app/lib/auth/session.ts: cookie session storage
  (sessionStorage, createSession, getSessionUser, destroySession)
  moved from auth.server.ts. Legacy import path kept via re-exports
  with @deprecated JSDoc.
- apps/journal/app/lib/auth/completion.ts: completeAuth + safeReturnTo.
- apps/journal/app/lib/auth/completion.test.ts: 10 contract tests
  covering both modes, returnTo sanitization (path-relative, protocol-
  relative, absolute-URL, malformed), Set-Cookie attachment, redirect
  status, JSON shape.

Caller migration:
- api.auth.register.ts passkey-finish → completeAuth(json)
- api.auth.login.ts finish-passkey → completeAuth(json)
- api.auth.login.ts verify-code → completeAuth(json)
- auth.verify.tsx magic-link consumer → completeAuth(redirect)

Client form updates:
- auth.login.tsx: pass returnTo in fetch body, read result.redirectTo
  on done.
- auth.register.tsx: pass returnTo: "/?add-passkey=1" for the magic-
  link verify-code path (preserves the post-register passkey prompt
  via the chokepoint's safeReturnTo, instead of hardcoding it
  client-side).

Verified:
- pnpm typecheck && pnpm lint: green across all 15 workspaces.
- pnpm --filter @trails-cool/journal test: 126 passed.
- pnpm test:e2e auth: 4/4 passed without modification — confirms the
  refactor is behaviour-preserving for the user-facing flows that
  matter most (passkey register + login).

Spec delta in openspec/changes/unify-auth-completion/specs/ applies at
/opsx:archive time.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ullrich Schäfer 2026-05-08 02:38:15 +02:00
parent 7e22f1260b
commit d64c47614d
No known key found for this signature in database
GPG key ID: A32FF691A0F752D9
12 changed files with 304 additions and 80 deletions

View file

@ -397,22 +397,19 @@ export async function verifyEmailChange(token: string, userId: string): Promise<
}
// --- Sessions ---
//
// Cookie session storage moved to `./auth/session.ts` so the post-verify
// chokepoint (`./auth/completion.ts`, see ADR-0004) can compose it. The
// re-exports below preserve the legacy `~/lib/auth.server` import path —
// new code should import from `~/lib/auth/session` directly.
import { createCookieSessionStorage } from "react-router";
const sessionSecret = process.env.SESSION_SECRET ?? "dev-secret-change-in-production";
export const sessionStorage = createCookieSessionStorage({
cookie: {
name: "__session",
httpOnly: true,
secure: process.env.NODE_ENV === "production",
sameSite: "lax",
path: "/",
maxAge: 60 * 60 * 24 * 30, // 30 days
secrets: [sessionSecret],
},
});
/** @deprecated Import from `~/lib/auth/session` instead. */
export {
sessionStorage,
createSession,
getSessionUser,
destroySession,
} from "./auth/session.ts";
/**
* A row that carries the minimum a visibility check needs.
@ -466,23 +463,3 @@ export async function recordTermsAcceptance(userId: string, termsVersion: string
.where(eq(users.id, userId));
}
export async function createSession(userId: string, request: Request) {
const session = await sessionStorage.getSession(request.headers.get("Cookie"));
session.set("userId", userId);
return sessionStorage.commitSession(session);
}
export async function getSessionUser(request: Request) {
const db = getDb();
const session = await sessionStorage.getSession(request.headers.get("Cookie"));
const userId = session.get("userId");
if (!userId) return null;
const [user] = await db.select().from(users).where(eq(users.id, userId));
return user ?? null;
}
export async function destroySession(request: Request) {
const session = await sessionStorage.getSession(request.headers.get("Cookie"));
return sessionStorage.destroySession(session);
}

View file

@ -0,0 +1,112 @@
// Contract tests for the completeAuth chokepoint. See ADR-0004 +
// docs/adr/0005-no-authmethod-polymorphism.md for why this exists.
import { describe, it, expect } from "vitest";
import { completeAuth } from "./completion.ts";
function reqWith(headers: Record<string, string> = {}) {
return new Request("https://localhost/whatever", { headers });
}
describe("completeAuth (mode=redirect)", () => {
it("redirects to returnTo when it's a same-origin path", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
returnTo: "/profile",
});
expect(res.status).toBe(302);
expect(res.headers.get("Location")).toBe("/profile");
});
it("redirects to / when returnTo is missing", async () => {
const res = await completeAuth({ userId: "u1", request: reqWith() });
expect(res.headers.get("Location")).toBe("/");
});
it("redirects to / when returnTo is null", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
returnTo: null,
});
expect(res.headers.get("Location")).toBe("/");
});
it("rejects protocol-relative returnTo (//evil.com/x) → /", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
returnTo: "//evil.com/x",
});
expect(res.headers.get("Location")).toBe("/");
});
it("rejects absolute-URL returnTo (https://evil.com) → /", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
returnTo: "https://evil.com",
});
expect(res.headers.get("Location")).toBe("/");
});
it("rejects returnTo that doesn't start with / → /", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
returnTo: "javascript:alert(1)",
});
expect(res.headers.get("Location")).toBe("/");
});
it("attaches a __session Set-Cookie carrying the userId", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
returnTo: "/",
});
const setCookie = res.headers.get("Set-Cookie");
expect(setCookie).not.toBeNull();
expect(setCookie!).toMatch(/^__session=/);
// HttpOnly is required for a session cookie.
expect(setCookie!.toLowerCase()).toContain("httponly");
});
});
describe("completeAuth (mode=json)", () => {
it("returns 200 JSON with sanitized redirectTo and Set-Cookie", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
returnTo: "/profile",
mode: "json",
});
expect(res.status).toBe(200);
expect(res.headers.get("Content-Type")).toContain("application/json");
expect(res.headers.get("Set-Cookie")).toMatch(/^__session=/);
const body = (await res.json()) as { ok: boolean; step: string; redirectTo: string };
expect(body).toEqual({ ok: true, step: "done", redirectTo: "/profile" });
});
it("falls back to / when returnTo is unsafe (json mode)", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
returnTo: "https://evil.com",
mode: "json",
});
const body = (await res.json()) as { ok: boolean; redirectTo: string };
expect(body.redirectTo).toBe("/");
});
it("redirectTo defaults to / when no returnTo (json mode)", async () => {
const res = await completeAuth({
userId: "u1",
request: reqWith(),
mode: "json",
});
const body = (await res.json()) as { ok: boolean; redirectTo: string };
expect(body.redirectTo).toBe("/");
});
});

View file

@ -0,0 +1,76 @@
// completeAuth — the single chokepoint that every successful web auth
// flow uses to mint the cookie session and produce the response. See
// ADR-0004 + CONTEXT.md (Authentication section).
//
// Two response shapes, picked via `mode`:
// - 'redirect' (loader callers / direct browser navigation):
// 302 redirect to the sanitized target, Set-Cookie attached.
// - 'json' (action callers from imperative fetch in client forms):
// 200 JSON { ok: true, redirectTo } with Set-Cookie. Client reads
// redirectTo and does window.location = redirectTo.
//
// Both modes share identity-agnostic behaviour: createSession + same-
// origin sanitization + Set-Cookie. Per ADR-0005, this function knows
// nothing about how identity was proved (passkey, magic-link, etc.) —
// the caller does its own per-method verification first.
//
// Terms recording is NOT here: both registration paths (finishRegistration
// for passkey, registerWithMagicLink for magic-link) record terms at
// user-creation time, before any path can reach completeAuth.
import { redirect } from "react-router";
import { createSession } from "./session.ts";
export type CompleteAuthMode = "redirect" | "json";
export interface CompleteAuthInput {
userId: string;
request: Request;
/**
* Optional caller-supplied redirect target. Validated as a same-origin
* path; anything else falls back to "/". Pass through whatever you
* received from the client sanitization is centralized here.
*/
returnTo?: string | null;
/**
* Response shape:
* - 'redirect' (default): 302 redirect Response; right for loaders
* (direct browser navigation, e.g. auth.verify.tsx loader).
* - 'json': 200 JSON `{ ok: true, redirectTo }`; right for action
* handlers called by imperative fetch from client form code
* (e.g. api.auth.register, api.auth.login). Client navigates
* using `data.redirectTo`.
*/
mode?: CompleteAuthMode;
}
/**
* Same-origin path check. Accepts only paths that start with a single
* "/" rejects:
* - Empty / null / undefined
* - Protocol-relative ("//evil.com/x")
* - Absolute URLs ("https://evil.com")
* - Anything not starting with "/"
*/
function safeReturnTo(value: string | null | undefined): string | null {
if (typeof value !== "string" || value.length === 0) return null;
if (!value.startsWith("/")) return null;
if (value.startsWith("//")) return null;
return value;
}
export async function completeAuth(input: CompleteAuthInput): Promise<Response> {
const cookie = await createSession(input.userId, input.request);
const target = safeReturnTo(input.returnTo) ?? "/";
const headers = { "Set-Cookie": cookie };
if (input.mode === "json") {
// `step: "done"` retained for compatibility with existing client
// form checks (auth.register.tsx, auth.login.tsx). New code should
// read `redirectTo` and navigate there.
return Response.json(
{ ok: true, step: "done", redirectTo: target },
{ headers },
);
}
return redirect(target, { headers });
}

View file

@ -0,0 +1,46 @@
// Cookie session storage. Lives here (separate from auth.server.ts) so
// the post-verify chokepoint (./completion.ts) can compose it without
// dragging the entire auth surface in.
//
// The legacy import path `~/lib/auth.server` continues to re-export
// these symbols for backwards compat — see auth.server.ts.
import { createCookieSessionStorage } from "react-router";
import { eq } from "drizzle-orm";
import { users } from "@trails-cool/db/schema/journal";
import { getDb } from "../db.ts";
const sessionSecret = process.env.SESSION_SECRET ?? "dev-secret-change-in-production";
export const sessionStorage = createCookieSessionStorage({
cookie: {
name: "__session",
httpOnly: true,
secure: process.env.NODE_ENV === "production",
sameSite: "lax",
path: "/",
maxAge: 60 * 60 * 24 * 30, // 30 days
secrets: [sessionSecret],
},
});
export async function createSession(userId: string, request: Request) {
const session = await sessionStorage.getSession(request.headers.get("Cookie"));
session.set("userId", userId);
return sessionStorage.commitSession(session);
}
export async function getSessionUser(request: Request) {
const db = getDb();
const session = await sessionStorage.getSession(request.headers.get("Cookie"));
const userId = session.get("userId");
if (!userId) return null;
const [user] = await db.select().from(users).where(eq(users.id, userId));
return user ?? null;
}
export async function destroySession(request: Request) {
const session = await sessionStorage.getSession(request.headers.get("Cookie"));
return sessionStorage.destroySession(session);
}

View file

@ -1,11 +1,12 @@
import { data } from "react-router";
import type { Route } from "./+types/api.auth.login";
import { startAuthentication, finishAuthentication, createMagicToken, verifyLoginCode, createSession } from "~/lib/auth.server";
import { startAuthentication, finishAuthentication, createMagicToken, verifyLoginCode } from "~/lib/auth.server";
import { completeAuth } from "~/lib/auth/completion";
import { sendMagicLink } from "~/lib/email.server";
export async function action({ request }: Route.ActionArgs) {
const body = await request.json();
const { step, response, challenge, email, code } = body;
const { step, response, challenge, email, code, returnTo } = body;
try {
if (step === "start-passkey") {
@ -15,8 +16,7 @@ export async function action({ request }: Route.ActionArgs) {
if (step === "finish-passkey") {
const userId = await finishAuthentication(response, challenge);
const cookie = await createSession(userId, request);
return data({ step: "done" }, { headers: { "Set-Cookie": cookie } });
return completeAuth({ userId, request, returnTo, mode: "json" });
}
if (step === "magic-link") {
@ -38,8 +38,7 @@ export async function action({ request }: Route.ActionArgs) {
return data({ error: "Email and code are required" }, { status: 400 });
}
const userId = await verifyLoginCode(email, code);
const cookie = await createSession(userId, request);
return data({ step: "done" }, { headers: { "Set-Cookie": cookie } });
return completeAuth({ userId, request, returnTo, mode: "json" });
}
return data({ error: "Invalid step" }, { status: 400 });

View file

@ -1,12 +1,13 @@
import { data } from "react-router";
import type { Route } from "./+types/api.auth.register";
import { startRegistration, finishRegistration, createSession, addPasskeyStart, addPasskeyFinish, registerWithMagicLink } from "~/lib/auth.server";
import { startRegistration, finishRegistration, addPasskeyStart, addPasskeyFinish, registerWithMagicLink } from "~/lib/auth.server";
import { completeAuth } from "~/lib/auth/completion";
import { sendWelcome, sendMagicLink } from "~/lib/email.server";
import { logger } from "~/lib/logger.server";
export async function action({ request }: Route.ActionArgs) {
const body = await request.json();
const { step, email, username, response, challenge, userId, termsAccepted, termsVersion } = body;
const { step, email, username, response, challenge, userId, termsAccepted, termsVersion, returnTo } = body;
const origin = process.env.ORIGIN ?? `http://localhost:3000`;
// Registration steps require terms acceptance + the version the client
@ -27,11 +28,10 @@ export async function action({ request }: Route.ActionArgs) {
if (step === "finish") {
const newUserId = await finishRegistration(userId, email, username, response, challenge, termsVersion);
const cookie = await createSession(newUserId, request);
sendWelcome(email, username).catch((err) =>
logger.error({ err }, "Failed to send welcome email"),
);
return data({ step: "done" }, { headers: { "Set-Cookie": cookie } });
return completeAuth({ userId: newUserId, request, returnTo, mode: "json" });
}
if (step === "register-magic-link") {

View file

@ -50,6 +50,7 @@ export default function LoginPage() {
step: "finish-passkey",
response: webAuthnResp,
challenge: startData.options.challenge,
returnTo,
}),
});
@ -57,7 +58,8 @@ export default function LoginPage() {
if (finishData.error) {
setError(finishData.error);
} else if (finishData.step === "done") {
window.location.href = returnTo ?? "/";
// Server-sanitized destination (see completeAuth in auth/completion.ts).
window.location.href = finishData.redirectTo ?? "/";
}
} catch (err) {
const message = (err as Error).message;
@ -80,14 +82,14 @@ export default function LoginPage() {
const resp = await fetch("/api/auth/login", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ step: "verify-code", email, code: loginCode }),
body: JSON.stringify({ step: "verify-code", email, code: loginCode, returnTo }),
});
const result = await resp.json();
if (result.error) {
setError(result.error);
} else if (result.step === "done") {
window.location.href = returnTo ?? "/";
window.location.href = result.redirectTo ?? "/";
}
} catch (err) {
setError((err as Error).message);

View file

@ -75,7 +75,7 @@ export default function RegisterPage() {
if (finishData.error) {
setError(finishData.error);
} else if (finishData.step === "done") {
window.location.href = "/";
window.location.href = finishData.redirectTo ?? "/";
}
} catch (err) {
setError((err as Error).message);
@ -134,13 +134,21 @@ export default function RegisterPage() {
const resp = await fetch("/api/auth/login", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ step: "verify-code", email, code: verifyCode }),
body: JSON.stringify({
step: "verify-code",
email,
code: verifyCode,
// Just-registered user: prompt to set up a passkey so next time
// they can use it directly. completeAuth's safeReturnTo accepts
// this as a same-origin path.
returnTo: "/?add-passkey=1",
}),
});
const result = await resp.json();
if (result.error) {
setError(result.error);
} else if (result.step === "done") {
window.location.href = "/?add-passkey=1";
window.location.href = result.redirectTo ?? "/?add-passkey=1";
}
} catch (err) {
setError((err as Error).message);

View file

@ -1,6 +1,7 @@
import { redirect, data } from "react-router";
import type { Route } from "./+types/auth.verify";
import { verifyMagicToken, verifyEmailChange, createSession, getSessionUser } from "~/lib/auth.server";
import { verifyMagicToken, verifyEmailChange, getSessionUser } from "~/lib/auth.server";
import { completeAuth } from "~/lib/auth/completion";
export async function loader({ request }: Route.LoaderArgs) {
const url = new URL(request.url);
@ -20,10 +21,13 @@ export async function loader({ request }: Route.LoaderArgs) {
}
const userId = await verifyMagicToken(token);
const cookie = await createSession(userId, request);
const returnTo = url.searchParams.get("returnTo");
const destination = returnTo?.startsWith("/") ? returnTo : "/?add-passkey=1";
return redirect(destination, { headers: { "Set-Cookie": cookie } });
// Default destination after magic-link sign-in is "/?add-passkey=1"
// (prompt to set up a passkey now that they're in). If the link
// carried a returnTo, completeAuth's safeReturnTo will honor any
// same-origin path and otherwise fall back to "/" — handle the
// add-passkey default before delegating.
const returnTo = url.searchParams.get("returnTo") ?? "/?add-passkey=1";
return completeAuth({ userId, request, returnTo, mode: "redirect" });
} catch (e) {
return data({ error: (e as Error).message }, { status: 400 });
}