From a8b0365ca5b53257b71e73becfa13b017c5e4049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 19 Jul 2026 11:21:01 +0200 Subject: [PATCH] fix(auth): carry a generated secret in the session cookie, not the row id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cookie was the Session row's cuid. cuid is built for collision- resistant identity across distributed writers, not for unguessability — it encodes a millisecond timestamp and a process-local counter and never touches a CSPRNG. A session cookie is a bearer credential and has to be unguessable, so it now carries 32 random bytes and only the HMAC of that value is stored, matching how api_tokens and refresh_tokens already hold their secrets. Compatibility is passive, and deliberately so. A backfill cannot help: the cookie already sitting in a browser is the cuid, and nothing we generate server-side can be pushed into it — backfilling would sign everyone out at deploy. Upgrading a row in place on first use loses to the parallel requests a single page load fires: each mints its own secret, one write wins, and the losers cannot re-emit the winner's secret because only a hash is stored, so the browser can end up holding a value the row does not honour. Instead, rows minted before the column keep a NULL token_hash and stay resolvable by id, and are denied the sliding-expiry extension so they drain within the 30-day session lifetime. Every login from this release forward is on a secret at once, and an id stops authenticating the moment its row holds one. Note for operators: API_TOKEN_HMAC_KEY now also keys session cookies, so rotating it signs browser sessions out as well as invalidating tokens. It is already Core-required, so no deployment gains a new prerequisite. --- docs/self-hosting/getting-started.md | 10 +- .../0254_session_token_hash/migration.sql | 21 ++ prisma/schema.prisma | 10 + scripts/env-manifest.json | 2 +- src/lib/auth/__tests__/session.test.ts | 233 ++++++++++++++++-- src/lib/auth/session.ts | 146 +++++++++-- tests/integration/auth-flow.test.ts | 132 ++++++++++ tests/integration/setup.ts | 10 + 8 files changed, 516 insertions(+), 48 deletions(-) create mode 100644 prisma/migrations/0254_session_token_hash/migration.sql diff --git a/docs/self-hosting/getting-started.md b/docs/self-hosting/getting-started.md index c24150fd4..644d3ed8b 100644 --- a/docs/self-hosting/getting-started.md +++ b/docs/self-hosting/getting-started.md @@ -47,11 +47,11 @@ echo "ENCRYPTION_KEY=$(openssl rand -hex 32)" >> .env echo "API_TOKEN_HMAC_KEY=$(openssl rand -hex 32)" >> .env ``` -| Variable | Purpose | Rotation cost | -| -------------------- | -------------------------------------------------------------------------------------- | ------------------------------------------------------ | -| `POSTGRES_PASSWORD` | Bundled Postgres password — used by both the `db` service and `DATABASE_URL` | Database restart + connection-string update | -| `ENCRYPTION_KEY` | AES-256-GCM key for at-rest secrets (Withings tokens, AI provider keys, VAPID secrets) | Coordinated; see `docs/ops/encryption-key-rotation.md` | -| `API_TOKEN_HMAC_KEY` | HMAC-SHA256 key for hashing Bearer API tokens before storage | Invalidates every issued `hlk_*` token | +| Variable | Purpose | Rotation cost | +| -------------------- | -------------------------------------------------------------------------------------- | -------------------------------------------------------------------------- | +| `POSTGRES_PASSWORD` | Bundled Postgres password — used by both the `db` service and `DATABASE_URL` | Database restart + connection-string update | +| `ENCRYPTION_KEY` | AES-256-GCM key for at-rest secrets (Withings tokens, AI provider keys, VAPID secrets) | Coordinated; see `docs/ops/encryption-key-rotation.md` | +| `API_TOKEN_HMAC_KEY` | HMAC-SHA256 key for hashing Bearer API tokens and web session cookies before storage | Invalidates every issued `hlk_*` token and signs out every browser session | You do **not** edit `DATABASE_URL` on the bundled `docker-compose.yml` stack: compose constructs the connection string inside the `app` diff --git a/prisma/migrations/0254_session_token_hash/migration.sql b/prisma/migrations/0254_session_token_hash/migration.sql new file mode 100644 index 000000000..30ea82b9d --- /dev/null +++ b/prisma/migrations/0254_session_token_hash/migration.sql @@ -0,0 +1,21 @@ +-- Session cookies carry a purpose-generated secret rather than the row's cuid +-- primary key. +-- +-- A cuid is built for collision-resistant identity, not for unguessability: it +-- embeds a timestamp and a monotonic counter and does not come from a CSPRNG. +-- A session cookie is a bearer credential and must. The cookie now carries a +-- 32-byte random token (`hls_`) and only its HMAC-SHA256 hash is stored, +-- matching how api_tokens and refresh_tokens already hold their secrets. +-- +-- Additive and nullable ON PURPOSE: every session row that exists at deploy +-- time keeps NULL here and stays resolvable by its id, so the release signs +-- nobody out. Those rows cannot have their sliding expiry extended, so the +-- id-as-credential path drains itself within the 30-day session lifetime with +-- no operator action. Postgres permits many NULLs under a unique index, so the +-- constraint below does not conflict with the backfill-free transition. + +-- AlterTable +ALTER TABLE "sessions" ADD COLUMN "token_hash" TEXT; + +-- CreateIndex +CREATE UNIQUE INDEX "sessions_token_hash_key" ON "sessions"("token_hash"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index f525464f7..ffaaff10a 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -1023,6 +1023,16 @@ model Passkey { model Session { id String @id @default(cuid()) userId String @map("user_id") + // v1.30.32 — HMAC-SHA256 of the session cookie's secret. The cookie carries + // a 32-byte CSPRNG token (`hls_`), never this row's cuid: a cuid is + // collision-resistant, not unguessable (embedded timestamp + counter), and a + // session identifier is a bearer credential. Hash-at-rest mirrors the + // ApiToken / RefreshToken posture — a database read yields no usable cookie. + // + // Nullable for the transition only: rows minted before this column existed + // carry NULL and are still resolvable by id until they expire. See the + // resolution comment in `src/lib/auth/session.ts`. + tokenHash String? @unique @map("token_hash") expiresAt DateTime @map("expires_at") createdAt DateTime @default(now()) @map("created_at") ipAddress String? @map("ip_address") diff --git a/scripts/env-manifest.json b/scripts/env-manifest.json index f65d9e554..dd45eee0f 100644 --- a/scripts/env-manifest.json +++ b/scripts/env-manifest.json @@ -31,7 +31,7 @@ }, { "name": "API_TOKEN_HMAC_KEY", - "purpose": "HMAC-SHA256 key for hashing external Bearer tokens. 64 hex chars." + "purpose": "HMAC-SHA256 key for hashing external Bearer tokens AND web session cookies. 64 hex chars. Rotating it signs every browser session out on top of invalidating every issued token." } ] }, diff --git a/src/lib/auth/__tests__/session.test.ts b/src/lib/auth/__tests__/session.test.ts index ee27a18aa..b7b169611 100644 --- a/src/lib/auth/__tests__/session.test.ts +++ b/src/lib/auth/__tests__/session.test.ts @@ -6,23 +6,48 @@ const cookieState = vi.hoisted(() => ({ set: vi.fn(), })); +interface SessionRow { + id: string; + userId: string; + tokenHash: string | null; + expiresAt: Date; + lastActiveAt: Date | null; + user: { id: string; locale?: string | null }; +} + +const store = vi.hoisted(() => ({ sessions: [] as unknown[] })); + vi.mock("@/lib/db", () => ({ prisma: { session: { - findUnique: vi.fn(), + // Dispatches on the `where` shape so the two resolution paths — by + // secret hash and by primary key — stay distinguishable in assertions. + findUnique: vi.fn( + async ({ where }: { where: { id?: string; tokenHash?: string } }) => { + const rows = store.sessions as SessionRow[]; + if (where.tokenHash !== undefined) { + return rows.find((r) => r.tokenHash === where.tokenHash) ?? null; + } + return rows.find((r) => r.id === where.id) ?? null; + }, + ), + create: vi.fn(async ({ data }: { data: Record }) => { + const row = { + id: `sess_cuid_${(store.sessions as SessionRow[]).length + 1}`, + lastActiveAt: null, + user: { id: data.userId as string }, + ...data, + } as unknown as SessionRow; + (store.sessions as SessionRow[]).push(row); + return row; + }), delete: vi.fn(), deleteMany: vi.fn(), update: vi.fn(), }, - apiToken: { - updateMany: vi.fn(), - }, - refreshToken: { - updateMany: vi.fn(), - }, - trustedDevice: { - deleteMany: vi.fn(), - }, + apiToken: { updateMany: vi.fn() }, + refreshToken: { updateMany: vi.fn() }, + trustedDevice: { deleteMany: vi.fn() }, $transaction: vi.fn(async (ops: Promise[]) => Promise.all(ops)), }, })); @@ -35,6 +60,12 @@ vi.mock("@/lib/logging/context", () => ({ getEvent: () => undefined, })); +// Deterministic stand-in for the HMAC so tests assert the *shape* of the +// stored value rather than re-deriving a keyed digest. +vi.mock("@/lib/auth/hmac", () => ({ + hashToken: (raw: string) => `hash:${raw}`, +})); + vi.mock("next/headers", () => ({ cookies: vi.fn(async () => ({ get: (name: string) => @@ -47,20 +78,165 @@ vi.mock("next/headers", () => ({ })); import { prisma } from "@/lib/db"; -import { getSession, destroyAllSessions, destroySession } from "../session"; +import { + getSession, + createSession, + destroyAllSessions, + destroySession, +} from "../session"; + +const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; + +function seed(row: Partial & { id: string }): SessionRow { + const full: SessionRow = { + userId: "user-1", + tokenHash: null, + expiresAt: new Date(Date.now() + THIRTY_DAYS_MS), + lastActiveAt: new Date(), + user: { id: row.userId ?? "user-1" }, + ...row, + } as SessionRow; + (store.sessions as SessionRow[]).push(full); + return full; +} beforeEach(() => { - vi.resetAllMocks(); + vi.clearAllMocks(); + store.sessions = []; cookieState.sessionId = "sess-expired"; + // `clearAllMocks` drops recorded calls but keeps implementations, so a + // rejection staged by one test would otherwise leak into the next. Re-arm + // the write mocks with a benign default each time. + vi.mocked(prisma.session.delete).mockResolvedValue({} as never); + vi.mocked(prisma.session.deleteMany).mockResolvedValue({ count: 1 } as never); + vi.mocked(prisma.session.update).mockResolvedValue({} as never); +}); + +describe("createSession", () => { + it("puts a CSPRNG secret in the cookie, never the row's primary key", async () => { + cookieState.sessionId = undefined; + + await createSession("user-1", false); + + const row = (store.sessions as SessionRow[])[0]; + const cookieValue = cookieState.set.mock.calls.find( + (c) => c[0] === "healthlog_session", + )![1] as string; + + // A cuid is collision-resistant, not unguessable. The cookie must not be + // the primary key under any encoding. + expect(cookieValue).not.toBe(row.id); + expect(cookieValue).not.toContain(row.id); + // 32 bytes of randomness, hex-encoded, behind a greppable prefix. + expect(cookieValue).toMatch(/^hls_[0-9a-f]{64}$/); + // Only the hash is persisted — a table dump yields no usable cookie. + expect(row.tokenHash).toBe(`hash:${cookieValue}`); + expect(row.tokenHash).not.toContain(row.id); + }); + + it("mints a distinct secret per session", async () => { + cookieState.sessionId = undefined; + await createSession("user-1", false); + await createSession("user-1", false); + + const [first, second] = cookieState.set.mock.calls + .filter((c) => c[0] === "healthlog_session") + .map((c) => c[1] as string); + + expect(first).not.toBe(second); + }); +}); + +describe("getSession — cookie resolution", () => { + it("resolves a modern cookie by its hash, not by primary key", async () => { + const row = seed({ id: "sess_cuid_1", tokenHash: "hash:hls_abc" }); + cookieState.sessionId = "hls_abc"; + + const result = await getSession(); + + expect(result?.session.id).toBe(row.id); + // The lookup went through the hash column. + expect(prisma.session.findUnique).toHaveBeenCalledWith( + expect.objectContaining({ where: { tokenHash: "hash:hls_abc" } }), + ); + }); + + it("refuses a modern-shaped cookie whose hash matches nothing", async () => { + seed({ id: "sess_cuid_1", tokenHash: "hash:hls_real" }); + cookieState.sessionId = "hls_forged"; + + await expect(getSession()).resolves.toBeNull(); + }); + + it("still accepts a pre-upgrade cookie carrying the row id (no forced logout)", async () => { + // The compatibility path. A session minted before the secret existed has + // a NULL token_hash and the browser holds its cuid; the deploy must not + // sign that user out. + const row = seed({ id: "sess_legacy", tokenHash: null }); + cookieState.sessionId = "sess_legacy"; + + const result = await getSession(); + + expect(result?.session.id).toBe(row.id); + expect(result?.user.id).toBe("user-1"); + }); + + it("retires the row id once that row holds a secret", async () => { + // Same cuid, but the row has been issued a secret. The id is an internal + // identifier again and must not authenticate anything — otherwise the + // guessable-credential surface never actually closes. + seed({ id: "sess_upgraded", tokenHash: "hash:hls_xyz" }); + cookieState.sessionId = "sess_upgraded"; + + await expect(getSession()).resolves.toBeNull(); + }); +}); + +describe("getSession — sliding expiry", () => { + it("extends a modern session and re-emits the same secret", async () => { + seed({ + id: "sess_cuid_1", + tokenHash: "hash:hls_abc", + expiresAt: new Date(Date.now() + 2 * 24 * 60 * 60 * 1000), + }); + cookieState.sessionId = "hls_abc"; + + await getSession(); + + expect(prisma.session.update).toHaveBeenCalledWith( + expect.objectContaining({ where: { id: "sess_cuid_1" } }), + ); + const cookieValue = cookieState.set.mock.calls.find( + (c) => c[0] === "healthlog_session", + )![1]; + expect(cookieValue).toBe("hls_abc"); + }); + + it("withholds the extension from a legacy session so the id path drains", async () => { + // A legacy row must not renew itself indefinitely, or an active user could + // ride a cuid-as-credential session forever. Capped at the expiry it + // already carries, the path is gone within the session lifetime. + seed({ + id: "sess_legacy", + tokenHash: null, + expiresAt: new Date(Date.now() + 2 * 24 * 60 * 60 * 1000), + }); + cookieState.sessionId = "sess_legacy"; + + const result = await getSession(); + + expect(result).not.toBeNull(); + expect(prisma.session.update).not.toHaveBeenCalled(); + }); }); describe("getSession", () => { it("swallows expired-session delete races and clears cookies", async () => { - vi.mocked(prisma.session.findUnique).mockResolvedValue({ + seed({ id: "sess-expired", + tokenHash: null, expiresAt: new Date(Date.now() - 1000), - user: { id: "user-1" }, - } as never); + }); vi.mocked(prisma.session.deleteMany).mockRejectedValue( new Error("already deleted") as never, ); @@ -76,7 +252,33 @@ describe("getSession", () => { }); describe("destroySession", () => { + it("deletes the row the secret resolves to, not the cookie value", async () => { + // The cookie is no longer the primary key, so a logout that deletes by + // cookie value would no-op and leave the session alive. + seed({ id: "sess_cuid_1", tokenHash: "hash:hls_abc" }); + cookieState.sessionId = "hls_abc"; + + await destroySession(); + + expect(prisma.session.delete).toHaveBeenCalledWith({ + where: { id: "sess_cuid_1" }, + }); + expect(cookieState.delete).toHaveBeenCalledWith("healthlog_session"); + }); + + it("logs a legacy cookie out too", async () => { + seed({ id: "sess_legacy", tokenHash: null }); + cookieState.sessionId = "sess_legacy"; + + await destroySession(); + + expect(prisma.session.delete).toHaveBeenCalledWith({ + where: { id: "sess_legacy" }, + }); + }); + it("treats an already-deleted session row (P2025) as an idempotent logout", async () => { + seed({ id: "sess-gone", tokenHash: null }); cookieState.sessionId = "sess-gone"; vi.mocked(prisma.session.delete).mockRejectedValue( Object.assign(new Error("record not found"), { code: "P2025" }) as never, @@ -92,6 +294,7 @@ describe("destroySession", () => { }); it("clears the cookie even when the row delete fails on a transient fault", async () => { + seed({ id: "sess-live", tokenHash: null }); cookieState.sessionId = "sess-live"; vi.mocked(prisma.session.delete).mockRejectedValue( Object.assign(new Error("connection reset"), { code: "P1001" }) as never, diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index bcddcc777..88820489b 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -1,6 +1,8 @@ +import { randomBytes } from "node:crypto"; import { prisma } from "@/lib/db"; import { cookies } from "next/headers"; import type { User } from "@/generated/prisma/client"; +import { hashToken } from "@/lib/auth/hmac"; import { ensureDbCompatibility } from "@/lib/db-compat"; import { getEvent } from "@/lib/logging/context"; import { shouldEmitSecureCookie } from "@/lib/auth/secure-cookie"; @@ -11,6 +13,77 @@ import { LOCALE_COOKIE, setLocaleCookie } from "@/lib/i18n/locale-cookie"; const SESSION_COOKIE = "healthlog_session"; const SESSION_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000; // 30 days +/** + * Marks a cookie value as a v1.30.32 session secret. Doubles as the branch + * between the two resolution paths below, so a cookie is only ever looked up + * one way and a raw cuid can never be tried as a secret (or vice versa). + */ +const SESSION_TOKEN_PREFIX = "hls_"; +const SESSION_SECRET_BYTES = 32; + +/** + * Mint a session cookie secret: 32 CSPRNG bytes, hex-encoded. + * + * This exists because the cookie used to carry `Session.id` — a `cuid()`. + * cuid targets collision-resistant identity across distributed writers, not + * unguessability: it encodes a millisecond timestamp and a process-local + * counter and never touches a CSPRNG. That is a fine primary key and an unfit + * bearer credential. Only `hashToken(secret)` reaches the database, so a + * leaked table dump yields no usable cookie — the same posture ApiToken and + * RefreshToken already hold. + */ +function mintSessionSecret(): string { + return `${SESSION_TOKEN_PREFIX}${randomBytes(SESSION_SECRET_BYTES).toString("hex")}`; +} + +/** + * Resolve the session row a cookie value refers to, or null. + * + * COMPATIBILITY DECISION (v1.30.32). Two paths, and the transition is + * deliberately passive: + * + * 1. `hls_…` — the modern secret. Looked up by `hashToken(value)`. + * 2. anything else — a pre-upgrade cookie carrying the row's cuid. Accepted + * ONLY while that row's `tokenHash` is still NULL. + * + * A migration-time backfill cannot work: the cookie already in the user's + * browser is the cuid, and no token we generate server-side can be pushed into + * it. Backfilling would sign out every logged-in user at deploy — the one + * outcome this change must not cause. + * + * Upgrading a legacy row in place on next use was the other candidate and was + * rejected as unsafe. A browser fires several requests in parallel, all + * carrying the same legacy cookie; each would mint its own secret and race to + * claim the row. Exactly one write can win, and the losers cannot learn the + * winner's secret to re-emit it — the row stores only a hash. Whichever + * response reached the browser last would decide the cookie, and if that was a + * loser's, the user is signed out. A guarded update narrows the write but not + * the outcome: a loser that declines to re-cookie leaves the browser holding a + * legacy id the row no longer honours. The race has no correct resolution, and + * its failure mode is precisely the forced logout we are avoiding. + * + * So legacy rows are left alone and simply live out the expiry they already + * have — `getSession` withholds the sliding-expiry extension from them (see + * there). That bounds the id-as-credential path to the 30-day session lifetime + * from the deploy, self-draining, with no operator step and no backfill. Every + * login from the deploy onward is on a secret immediately. + */ +async function findSessionByCookie(cookieValue: string) { + if (cookieValue.startsWith(SESSION_TOKEN_PREFIX)) { + return prisma.session.findUnique({ + where: { tokenHash: hashToken(cookieValue) }, + include: { user: true }, + }); + } + + const legacy = await prisma.session.findUnique({ + where: { id: cookieValue }, + include: { user: true }, + }); + // A row that already holds a secret has retired its id as a credential. + return legacy && legacy.tokenHash === null ? legacy : null; +} + // v1.23 — throttle window for the user-facing active-session list's "last // seen" stamp. A write only happens when the prior stamp is older than this, // so the high-churn `getSession` path costs at most one extra UPDATE per @@ -73,9 +146,13 @@ export async function createSession( // "single-factor session" and never satisfies step-up. mfaVerifiedAt?: Date | null, ): Promise { + // The secret is the cookie; the row id stays an internal identifier and is + // never handed to the client. + const secret = mintSessionSecret(); const session = await prisma.session.create({ data: { userId, + tokenHash: hashToken(secret), expiresAt: new Date(Date.now() + SESSION_MAX_AGE_MS), ipAddress: ipAddress ?? null, userAgent: userAgent ?? null, @@ -84,7 +161,7 @@ export async function createSession( }); const cookieStore = await cookies(); - cookieStore.set(SESSION_COOKIE, session.id, { + cookieStore.set(SESSION_COOKIE, secret, { httpOnly: true, secure: shouldEmitSecureCookie(), // OAuth callbacks (e.g. Withings) arrive via top-level cross-site redirect. @@ -117,18 +194,15 @@ export async function getSession(): Promise<{ } const cookieStore = await cookies(); - const sessionId = cookieStore.get(SESSION_COOKIE)?.value; - if (!sessionId) return null; + const cookieValue = cookieStore.get(SESSION_COOKIE)?.value; + if (!cookieValue) return null; - const session = await prisma.session.findUnique({ - where: { id: sessionId }, - include: { user: true }, - }); + const session = await findSessionByCookie(cookieValue); if (!session || session.expiresAt < new Date()) { if (session) { await prisma.session - .deleteMany({ where: { id: sessionId } }) + .deleteMany({ where: { id: session.id } }) .catch(() => {}); } cookieStore.delete(SESSION_COOKIE); @@ -136,18 +210,26 @@ export async function getSession(): Promise<{ return null; } - // Sliding expiry: refresh if more than 1 day old + // Sliding expiry: refresh if more than 1 day old. + // + // Withheld from legacy (`tokenHash === null`) rows on purpose. Extending + // those would let a cuid-as-credential session renew itself indefinitely for + // an active user, and the whole point of the passive transition is that the + // id-resolvable path drains on its own. Capping them at the expiry they + // already carry bounds it to at most 30 days past the deploy; the user then + // logs in once and lands on a secret. Re-emitting the cookie is likewise + // pointless for a legacy row — its value cannot be improved in place. const oneDayMs = 24 * 60 * 60 * 1000; if ( - session.expiresAt.getTime() - Date.now() < - SESSION_MAX_AGE_MS - oneDayMs + session.tokenHash !== null && + session.expiresAt.getTime() - Date.now() < SESSION_MAX_AGE_MS - oneDayMs ) { const newExpiry = new Date(Date.now() + SESSION_MAX_AGE_MS); await prisma.session.update({ where: { id: session.id }, data: { expiresAt: newExpiry }, }); - cookieStore.set(SESSION_COOKIE, session.id, { + cookieStore.set(SESSION_COOKIE, cookieValue, { httpOnly: true, secure: shouldEmitSecureCookie(), sameSite: "lax", @@ -211,33 +293,43 @@ export async function getSession(): Promise<{ */ export async function getSessionUserLocale(): Promise { const cookieStore = await cookies(); - const sessionId = cookieStore.get(SESSION_COOKIE)?.value; - if (!sessionId) return null; + const cookieValue = cookieStore.get(SESSION_COOKIE)?.value; + if (!cookieValue) return null; - const session = await prisma.session.findUnique({ - where: { id: sessionId }, - select: { expiresAt: true, user: { select: { locale: true } } }, - }); + // Same two-path resolution as `getSession`, read-only: this runs in the root + // layout where the cookie store cannot be written, so there is nothing to + // refresh or clear here. + const session = await findSessionByCookie(cookieValue); if (!session || session.expiresAt < new Date()) return null; return session.user.locale; } export async function destroySession(): Promise { const cookieStore = await cookies(); - const sessionId = cookieStore.get(SESSION_COOKIE)?.value; - if (sessionId) { + const cookieValue = cookieStore.get(SESSION_COOKIE)?.value; + if (cookieValue) { + // Resolve the cookie to its row before deleting — the cookie is a secret, + // not the primary key, so `delete({ where: { id: cookieValue } })` would + // silently no-op on every modern session and leave the row alive after a + // logout. + // // Logout is idempotent: a session row that no longer exists (P2025) is a // no-op. Any other failure is a real delete error — record it on the wide // event rather than swallowing it silently, but never block the cookie // clear below: a transient DB fault must not leave the client logged in // with the cookie intact. - await prisma.session.delete({ where: { id: sessionId } }).catch((err) => { - if (!isP2025(err)) { - getEvent()?.addWarning( - `destroySession delete failed: ${err instanceof Error ? err.message : String(err)}`, - ); - } - }); + const session = await findSessionByCookie(cookieValue).catch(() => null); + if (session) { + await prisma.session + .delete({ where: { id: session.id } }) + .catch((err) => { + if (!isP2025(err)) { + getEvent()?.addWarning( + `destroySession delete failed: ${err instanceof Error ? err.message : String(err)}`, + ); + } + }); + } } cookieStore.delete(SESSION_COOKIE); cookieStore.delete(ONBOARDING_COOKIE); diff --git a/tests/integration/auth-flow.test.ts b/tests/integration/auth-flow.test.ts index 5f269a6d9..e496fe0ea 100644 --- a/tests/integration/auth-flow.test.ts +++ b/tests/integration/auth-flow.test.ts @@ -132,6 +132,138 @@ describe("session lifecycle (real Postgres)", () => { expect(cookieJar.get("hl_onboarding")).toBeUndefined(); }); + /** + * v1.30.32 — the cookie carries a CSPRNG secret, not the row's cuid. + * These run against real Postgres because the thing under test is the + * migration plus the unique index plus the two lookup paths; a mocked + * client can assert the shape but not that the column and constraint + * actually exist on a migrated database. + */ + describe("session cookie secret (migration 0254)", () => { + it("issues a secret cookie and stores only its hash", async () => { + const { createSession } = await import("@/lib/auth/session"); + const { hashToken } = await import("@/lib/auth/hmac"); + + const user = await getPrismaClient().user.create({ + data: { username: "secret-cookie", email: "secret@example.test" }, + }); + const sessionId = await createSession(user.id, false); + + const cookie = cookieJar.get("healthlog_session")!; + expect(cookie).toMatch(/^hls_[0-9a-f]{64}$/); + expect(cookie).not.toBe(sessionId); + + const row = await getPrismaClient().session.findUnique({ + where: { id: sessionId }, + }); + // The raw secret is nowhere on the row. + expect(row?.tokenHash).toBe(hashToken(cookie)); + expect(row?.tokenHash).not.toBe(cookie); + }); + + it("authenticates by the secret and refuses the row id", async () => { + const { createSession, getSession } = await import("@/lib/auth/session"); + + const user = await getPrismaClient().user.create({ + data: { username: "id-retired", email: "retired@example.test" }, + }); + const sessionId = await createSession(user.id, false); + + // The secret works. + expect((await getSession())?.session.id).toBe(sessionId); + + // The primary key does not. This is the whole point: knowing or + // guessing a cuid must no longer authenticate anything. + cookieJar.set("healthlog_session", sessionId); + expect(await getSession()).toBeNull(); + }); + + it("keeps a pre-upgrade session alive on its row id", async () => { + // The compatibility contract. This row is exactly what the migration + // leaves behind: a real session with a NULL token_hash whose cookie in + // the user's browser is the cuid. The deploy must not sign them out. + const { getSession } = await import("@/lib/auth/session"); + + const user = await getPrismaClient().user.create({ + data: { username: "legacy-session", email: "legacy@example.test" }, + }); + const legacy = await getPrismaClient().session.create({ + data: { + userId: user.id, + expiresAt: new Date(Date.now() + 20 * 24 * 60 * 60 * 1000), + }, + }); + expect(legacy.tokenHash).toBeNull(); + + cookieJar.set("healthlog_session", legacy.id); + const result = await getSession(); + + expect(result).not.toBeNull(); + expect(result?.session.id).toBe(legacy.id); + expect(result?.user.id).toBe(user.id); + }); + + it("does not extend a pre-upgrade session's expiry", async () => { + // Withholding the sliding refresh is what bounds the transition: the + // id-resolvable path drains within the session lifetime instead of + // renewing forever under an active user. + const { getSession } = await import("@/lib/auth/session"); + + const user = await getPrismaClient().user.create({ + data: { username: "legacy-drain", email: "drain@example.test" }, + }); + const expiresAt = new Date(Date.now() + 2 * 24 * 60 * 60 * 1000); + const legacy = await getPrismaClient().session.create({ + data: { userId: user.id, expiresAt }, + }); + + cookieJar.set("healthlog_session", legacy.id); + expect(await getSession()).not.toBeNull(); + + const after = await getPrismaClient().session.findUnique({ + where: { id: legacy.id }, + }); + expect(after?.expiresAt.getTime()).toBe(expiresAt.getTime()); + }); + + it("gives concurrent sessions distinct secrets under the unique index", async () => { + const { createSession } = await import("@/lib/auth/session"); + + const user = await getPrismaClient().user.create({ + data: { username: "many-secrets", email: "many@example.test" }, + }); + await createSession(user.id, false); + const first = cookieJar.get("healthlog_session")!; + await createSession(user.id, false); + const second = cookieJar.get("healthlog_session")!; + + expect(first).not.toBe(second); + const rows = await getPrismaClient().session.findMany({ + where: { userId: user.id }, + }); + expect(rows).toHaveLength(2); + expect(new Set(rows.map((r) => r.tokenHash)).size).toBe(2); + }); + + it("logs out by resolving the secret to its row", async () => { + const { createSession, destroySession } = + await import("@/lib/auth/session"); + + const user = await getPrismaClient().user.create({ + data: { username: "secret-logout", email: "logout@example.test" }, + }); + const sessionId = await createSession(user.id, false); + + await destroySession(); + + // A logout that deleted by cookie value would leave this row behind. + const row = await getPrismaClient().session.findUnique({ + where: { id: sessionId }, + }); + expect(row).toBeNull(); + }); + }); + it("destroySession clears the hl_onboarding cookie alongside the session cookie", async () => { const { createSession, destroySession } = await import("@/lib/auth/session"); diff --git a/tests/integration/setup.ts b/tests/integration/setup.ts index 0b4240f3d..56fdd8094 100644 --- a/tests/integration/setup.ts +++ b/tests/integration/setup.ts @@ -24,6 +24,16 @@ import type { PrismaClient } from "@/generated/prisma/client"; process.env.ENCRYPTION_KEY ??= "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; +// v1.30.32 — session cookies now carry a CSPRNG secret whose HMAC is what +// lands in `sessions.token_hash`, so `createSession` needs this key exactly +// like token issuance already did. It is a Core-required variable in +// `scripts/env-manifest.json` (the app refuses to boot without it), so +// defaulting it here matches production rather than papering over a gap. +// Same `??=` treatment as the encryption key above: files that set their own +// still win. +process.env.API_TOKEN_HMAC_KEY ??= + "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210"; + /** * The application's Prisma singleton. Tests use this so any code * imported via `await import("@/lib/...")` shares the exact same client