diff --git a/CHANGELOG.md b/CHANGELOG.md index 57ee097ef..5b27a7b90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,38 @@ ## [Unreleased] +## [1.30.33] — 2026-07-19 + +Security release. It ships on its own because it changes how a browser +session is recognised. + +- **The session cookie is a purpose-generated secret.** It used to be the + session record's own database key — an identifier built to avoid + collisions, not to be unguessable, and one that appears wherever that + record is referenced. The cookie now carries 32 bytes from the system's + random source, and only its hash is stored, so the value in your + browser exists nowhere on the server. +- **You are not signed out by this.** Sessions from before the change + keep working and are simply not renewed, so they run out their normal + 30 days and disappear. Every sign-in from this release onward is on the + new scheme. +- **The device list no longer hands out working credentials.** It + identified each device by that same record key — which, for a session + predating this release, is that device's cookie. It now shows an opaque + handle that is useless as a login, and the same handle is what signs a + device out. +- **A refresh token cannot be rotated from a different device**, and an + expired one is refused. The test that was supposed to prove the expiry + passed a token that had never been issued, so it proved nothing: with + the expiry check deleted, the suite still went green. It is a real test + now. +- **A Nightscout address is checked when you save it**, not only when it + happens to be probed first. The check itself already refused private, + loopback and cloud-metadata addresses. + +Operators: rotating `API_TOKEN_HMAC_KEY` now also signs out every browser +session, not just API tokens. One migration (0255), additive. + ## [1.30.32] — 2026-07-19 - **A withdrawn AI consent stays withdrawn.** Opening the AI settings diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 8e17c3fd7..1b8fc1dc1 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: HealthLog API - version: 1.30.32 + version: 1.30.33 description: >- Self-hosted personal-health-tracking PWA — public API surface for the iOS native client and external ingest. 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/package.json b/package.json index b1653770f..413e256e8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "healthlog", - "version": "1.30.32", + "version": "1.30.33", "description": "Self-hosted personal-health-tracking PWA with Withings integration, AI insights, and doctor-report PDF export.", "license": "PolyForm-Noncommercial-1.0.0", "homepage": "https://healthlog.dev", diff --git a/prisma/migrations/0255_session_token_hash/migration.sql b/prisma/migrations/0255_session_token_hash/migration.sql new file mode 100644 index 000000000..30ea82b9d --- /dev/null +++ b/prisma/migrations/0255_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/public/sw.js b/public/sw.js index d35934c1b..a175c4fc5 100644 --- a/public/sw.js +++ b/public/sw.js @@ -36,7 +36,7 @@ try { // v1.4.38.4 → v1.4.42. Do not hand-edit; bump `package.json` and rebuild. const CACHE_VERSION = (typeof self !== "undefined" && self.__APP_VERSION__) || - /* @sw-version-fallback */ "v1.30.32"; + /* @sw-version-fallback */ "v1.30.33"; const STATIC_CACHE = `healthlog-static-${CACHE_VERSION}`; const PAGE_CACHE = `healthlog-pages-${CACHE_VERSION}`; // v1.18.6 — read-only data cache for a curated allowlist of safe GET `/api/*` 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/app/api/auth/me/sessions/__tests__/route.test.ts b/src/app/api/auth/me/sessions/__tests__/route.test.ts index 57ad51f46..575dca26f 100644 --- a/src/app/api/auth/me/sessions/__tests__/route.test.ts +++ b/src/app/api/auth/me/sessions/__tests__/route.test.ts @@ -1,15 +1,25 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest } from "next/server"; +// The handle is an HMAC, so the route needs the key the real helper reads. +process.env.API_TOKEN_HMAC_KEY ??= "a".repeat(64); + vi.mock("@/lib/db", () => ({ prisma: { session: { findMany: vi.fn() } }, })); -vi.mock("@/lib/auth/session", () => ({ - getSession: vi.fn(), - destroyOtherSessions: vi.fn(), - destroySessionById: vi.fn(), -})); +// `sessionHandle` stays REAL: the point of the handle test is that the route +// emits the actual derivation, and a stubbed one would let the route return +// row ids while the assertion still passed. +vi.mock("@/lib/auth/session", async (importOriginal) => { + const actual = await importOriginal(); + return { + getSession: vi.fn(), + destroyOtherSessions: vi.fn(), + destroySessionById: vi.fn(), + sessionHandle: actual.sessionHandle, + }; +}); vi.mock("@/lib/geo", () => ({ lookupIpLocation: vi.fn(async () => "Berlin, DE"), @@ -39,6 +49,7 @@ import { getSession, destroyOtherSessions, destroySessionById, + sessionHandle, } from "@/lib/auth/session"; const SESSION_OK = { @@ -90,7 +101,6 @@ describe("GET /api/auth/me/sessions", () => { const list = body.data.sessions; expect(list).toHaveLength(2); expect(list[0]).toMatchObject({ - id: "sess-current", device: "Firefox on macOS", ipMasked: "203.0.x.x", location: "Berlin, DE", @@ -100,6 +110,42 @@ describe("GET /api/auth/me/sessions", () => { // The full IP never appears in the response body. expect(JSON.stringify(body)).not.toContain("203.0.113.7"); }); + + it("never puts a session row id in the response body", async () => { + // A row id created before the secret cookie landed IS that session's + // cookie value, so listing it hands the client working logins for the + // account's other devices. The list carries an opaque handle instead; + // it is what the revoke route accepts. + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(prisma.session.findMany).mockResolvedValue([ + { + id: "sess-current", + ipAddress: "203.0.113.7", + userAgent: "Mozilla/5.0 (Macintosh) Firefox/120.0", + lastActiveAt: new Date(), + createdAt: new Date(), + }, + { + id: "sess-other", + ipAddress: "198.51.100.4", + userAgent: "Mozilla/5.0 (Windows NT 10.0) Chrome/119.0.0.0", + lastActiveAt: null, + createdAt: new Date(), + }, + ] as never); + + const res = await GET(); + const body = await res.json(); + const serialised = JSON.stringify(body); + + expect(serialised).not.toContain("sess-current"); + expect(serialised).not.toContain("sess-other"); + // Still identified well enough to act on: distinct, stable handles. + const [a, b] = body.data.sessions.map((s: { id: string }) => s.id); + expect(a).toBeTruthy(); + expect(a).not.toBe(b); + expect(a).toBe(sessionHandle("sess-current")); + }); }); describe("DELETE /api/auth/me/sessions (sign out everywhere)", () => { diff --git a/src/app/api/auth/me/sessions/route.ts b/src/app/api/auth/me/sessions/route.ts index bb3fcd05e..440f4d44a 100644 --- a/src/app/api/auth/me/sessions/route.ts +++ b/src/app/api/auth/me/sessions/route.ts @@ -18,7 +18,7 @@ import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; import { getClientIp } from "@/lib/api-response"; import { prisma } from "@/lib/db"; -import { destroyOtherSessions } from "@/lib/auth/session"; +import { destroyOtherSessions, sessionHandle } from "@/lib/auth/session"; import { lookupIpLocation } from "@/lib/geo"; import { coarseDeviceLabel, maskIp } from "@/lib/auth/device-fingerprint"; @@ -68,7 +68,7 @@ export const GET = apiHandler(async () => { ); const result: SessionDTO[] = sessions.map((s) => ({ - id: s.id, + id: sessionHandle(s.id), device: coarseDeviceLabel(s.userAgent), ipMasked: maskIp(s.ipAddress), location: s.ipAddress ? (locationByIp.get(s.ipAddress) ?? null) : null, diff --git a/src/app/api/nightscout/connect/__tests__/route.test.ts b/src/app/api/nightscout/connect/__tests__/route.test.ts index 811a09f72..115bfb8a3 100644 --- a/src/app/api/nightscout/connect/__tests__/route.test.ts +++ b/src/app/api/nightscout/connect/__tests__/route.test.ts @@ -140,6 +140,63 @@ describe("POST /api/nightscout/connect", () => { expect(userUpdate).not.toHaveBeenCalled(); }); + // Input-time SSRF floor. Every case below leaves the default happy-path + // `fetchSgvEntries` mock in place — it RESOLVES — so the only thing that + // can produce a 422 is the check that runs before the probe. Asserting the + // probe was never called is what separates this from the fetch-time guard: + // a URL that never reaches the egress layer cannot have been refused by it. + describe("refuses private / non-public hosts before the probe runs", () => { + const cases: Array<[string, string]> = [ + ["loopback", "http://127.0.0.1:1337"], + ["loopback by name", "http://localhost:1337"], + ["link-local", "http://169.254.1.1"], + ["cloud metadata endpoint", "http://169.254.169.254/latest/meta-data/"], + ["RFC1918 class A", "http://10.0.0.5"], + ["RFC1918 class C", "http://192.168.1.5"], + ["CGNAT", "http://100.64.0.1"], + ["IPv4-mapped IPv6 loopback", "http://[::ffff:127.0.0.1]"], + ["IPv6 loopback", "http://[::1]:1337"], + ["IPv6 unique-local", "http://[fd00::1]"], + ["octal IPv4 loopback", "http://0177.0.0.1"], + ["hex IPv4 loopback", "http://0x7f000001"], + ["decimal IPv4 loopback", "http://2130706433"], + [".internal suffix", "http://cgm.internal"], + [".local suffix", "http://cgm.local"], + ]; + + it.each(cases)("refuses %s", async (_label, url) => { + const res = await post(req({ url, token: "tok" })); + + expect(res.status).toBe(422); + expect(String(res.error)).toMatch(/private network/i); + expect(fetchSgvEntriesMock).not.toHaveBeenCalled(); + expect(userUpdate).not.toHaveBeenCalled(); + }); + + it("still lets the explicit self-hoster opt-in through to the probe", async () => { + // The floor is gated, not absolute: a LAN instance stays reachable for + // the operator who asked for it. Without this the opt-in is dead code. + const res = await post( + req({ + url: "http://192.168.1.5:1337", + token: "tok", + allowPrivateHost: true, + }), + ); + + expect(res.status).toBe(200); + expect(fetchSgvEntriesMock).toHaveBeenCalledTimes(1); + expect(userUpdate).toHaveBeenCalledTimes(1); + }); + + it("lets an ordinary public instance through", async () => { + const res = await post(req({ url: "https://ns.example.com" })); + + expect(res.status).toBe(200); + expect(fetchSgvEntriesMock).toHaveBeenCalledTimes(1); + }); + }); + it("rate-limits the connect surface", async () => { rateLimitMock.mockResolvedValue({ allowed: false, diff --git a/src/app/api/nightscout/connect/route.ts b/src/app/api/nightscout/connect/route.ts index 39af2550a..50c7ad821 100644 --- a/src/app/api/nightscout/connect/route.ts +++ b/src/app/api/nightscout/connect/route.ts @@ -12,6 +12,7 @@ import { markReconnected } from "@/lib/integrations/status"; import { SafeFetchError } from "@/lib/safe-fetch"; import { fetchSgvEntries, NightscoutApiError } from "@/lib/nightscout/client"; import { nightscoutConnectSchema } from "@/lib/validations/nightscout"; +import { isPublicUrl } from "@/lib/validations/notifications"; /** * Connect (or update) the user's Nightscout instance (v1.17.0). @@ -49,6 +50,25 @@ export const POST = apiHandler(async (request: NextRequest) => { const { url, token, allowPrivateHost } = result.data; + // SSRF floor at INPUT time, not merely as a side effect of the live probe + // below. `fetchSgvEntries` does route through `safeFetch` with the same + // public-host pin, so today both paths refuse the same URLs — but that + // makes the guard on the value we PERSIST contingent on the probe staying + // exactly where it is. Reorder it, make it best-effort, or add a "save + // without testing" affordance and the stored URL is suddenly unchecked. + // Assert it here so the thing that gets encrypted onto the row is the + // thing that was validated. + // + // Gated on the same explicit opt-in the fetch path uses: a self-hoster who + // deliberately points at a LAN instance has asked for the private host and + // must still be able to connect. + if (!allowPrivateHost && !isPublicUrl(url)) { + return apiError( + "That instance is on a private network. Enable the private-network option to connect to it.", + 422, + ); + } + // Live validation: pull a single SGV entry. A connection that can't even // fetch one row is not worth storing. try { diff --git a/src/lib/auth/__tests__/refresh-token.test.ts b/src/lib/auth/__tests__/refresh-token.test.ts index 1ee9835cc..b19d98a4b 100644 --- a/src/lib/auth/__tests__/refresh-token.test.ts +++ b/src/lib/auth/__tests__/refresh-token.test.ts @@ -394,21 +394,95 @@ describe("rotateRefreshToken", () => { expect(liveDev1).toHaveLength(0); }); - it("rejects expired refresh tokens", async () => { - await issueAccessAndRefresh({ + it("rejects an expired refresh token by its own secret", async () => { + // The token presented here is the REAL one, so the row is found and the + // expiry branch is what refuses it. Presenting an unknown string instead + // would pass on `not_found` and prove nothing about expiry at all. + const initial = await issueAccessAndRefresh({ userId: "u1", policy: NATIVE_POLICY, source: "login.password", }); - // Manually expire dbState.refreshTokens[0].expiresAt = new Date(Date.now() - 1000); + const result = await rotateRefreshToken({ - refreshToken: "hlr_doesnt_matter", + refreshToken: initial.refreshToken, policy: NATIVE_POLICY, }); + expect(result.ok).toBe(false); if (result.ok) return; - expect(result.reason).toBe("not_found"); + expect(result.reason).toBe("expired"); + // An expired token must not mint a replacement pair. + expect(dbState.refreshTokens).toHaveLength(1); + }); + + it("accepts the same token one millisecond before it expires", async () => { + // Boundary companion: proves the expiry refusal above is driven by the + // clock and not by the rotation path refusing everything. + const initial = await issueAccessAndRefresh({ + userId: "u1", + policy: NATIVE_POLICY, + source: "login.password", + }); + dbState.refreshTokens[0].expiresAt = new Date(Date.now() + 60_000); + + const result = await rotateRefreshToken({ + refreshToken: initial.refreshToken, + policy: NATIVE_POLICY, + }); + + expect(result.ok).toBe(true); + }); + + it("refuses a live token presented under a different device id", async () => { + // Device A's token replayed from device B. The token is valid and unused, + // so only the device binding can refuse it. + const dev1 = await issueAccessAndRefresh({ + userId: "u1", + policy: NATIVE_POLICY, + source: "login.password", + deviceId: "dev-1", + }); + + const result = await rotateRefreshToken({ + refreshToken: dev1.refreshToken, + policy: NATIVE_POLICY, + deviceId: "dev-2", + }); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.reason).toBe("device_mismatch"); + // Refused, not consumed: the legitimate device can still rotate it. + expect(dbState.refreshTokens[0].usedAt).toBeNull(); + expect(dbState.refreshTokens).toHaveLength(1); + + const legitimate = await rotateRefreshToken({ + refreshToken: dev1.refreshToken, + policy: NATIVE_POLICY, + deviceId: "dev-1", + }); + expect(legitimate.ok).toBe(true); + }); + + it("still rotates when the caller sends no device id at all", async () => { + // An older client that never sends `X-Device-Id` must not be locked out + // by the device binding — a presented null is unattributable, not a + // mismatch. + const dev1 = await issueAccessAndRefresh({ + userId: "u1", + policy: NATIVE_POLICY, + source: "login.password", + deviceId: "dev-1", + }); + + const result = await rotateRefreshToken({ + refreshToken: dev1.refreshToken, + policy: NATIVE_POLICY, + }); + + expect(result.ok).toBe(true); }); it("rejects revoked refresh tokens", async () => { 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/refresh-token.ts b/src/lib/auth/refresh-token.ts index 812588f57..37997964f 100644 --- a/src/lib/auth/refresh-token.ts +++ b/src/lib/auth/refresh-token.ts @@ -83,7 +83,7 @@ export async function issueAccessAndRefresh( } export type RotationFailureReason = - "not_found" | "expired" | "already_used" | "revoked"; + "not_found" | "expired" | "already_used" | "revoked" | "device_mismatch"; export type RotationResult = | { ok: true; bundle: IssuedRefreshBundle } @@ -158,6 +158,26 @@ export async function rotateRefreshToken(input: { return { ok: false, reason: "already_used" }; } + // A live token belongs to the device it was issued to. The M-4 escalation + // above already treats a mismatched deviceId on a REPLAY as a spoofing + // signal; the same mismatch on a still-live token is refused outright + // rather than silently rotated. Without this the reuse-detection scope is + // only as honest as the caller: a thief could rotate a stolen token under + // their own fabricated device id, and the resulting family — keyed on the + // attacker's id, because `issueAccessAndRefresh` below inherits + // `row.deviceId ?? input.deviceId` — would sit outside the victim device's + // containment blast radius. + // + // Both ids must be present to compare. A stored null means the row predates + // per-device issuance and cannot be attributed; a presented null means the + // caller sent no `X-Device-Id` header. Neither is treated as a mismatch, so + // this cannot lock out an older client that never sends the header. + const presentedId = input.deviceId ?? null; + const storedId = row.deviceId ?? null; + if (presentedId !== null && storedId !== null && presentedId !== storedId) { + return { ok: false, reason: "device_mismatch" }; + } + // Issue the new pair first, THEN mark old consumed in a transaction. // (Race: if two concurrent refreshes hit the same row, both will try to // mark `usedAt`. We use updateMany with a `usedAt: null` guard so only diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index bcddcc777..4cb7839b2 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); @@ -309,16 +401,48 @@ export async function destroyOtherSessions( } /** - * v1.23 — revoke a single web session by id, scoped to the owning user so a - * caller can never delete another user's session row. Returns whether a row - * was actually removed (false → not found or not owned). + * The public handle for a session row, used by the session-management list + * and its revoke route. + * + * The list needs a per-row identifier the client can send back to revoke one + * device. It used to send the row id — which, for every session created + * before the secret cookie landed, IS that session's cookie value. The list + * was therefore handing the client live credentials for all of the account's + * other devices, and anything that captured one response body (an error + * reporter, a proxy log, a screenshot of devtools) captured usable logins. + * + * The handle is an HMAC of the row id: stable within a deployment so the + * client can round-trip it, and structurally useless as a cookie because the + * legacy lookup matches a cuid primary key and a 64-char hex digest is not + * one. Nothing is stored — the handle is recomputed per request, so this + * needs no column and no migration. + */ +export function sessionHandle(sessionId: string): string { + return hashToken(sessionId); +} + +/** + * v1.23 — revoke a single web session, scoped to the owning user so a caller + * can never delete another user's session row. Returns whether a row was + * actually removed (false → not found or not owned). + * + * Takes the public handle rather than the row id. The account's own rows are + * a handful, so resolving the handle by walking them costs one indexed read + * and a few HMACs — cheaper than a column, and it keeps the id server-side. */ export async function destroySessionById( userId: string, - sessionId: string, + handle: string, ): Promise { + const own = await prisma.session.findMany({ + where: { userId }, + select: { id: true }, + }); + const match = own.find((s) => sessionHandle(s.id) === handle); + if (!match) return false; + const result = await prisma.session.deleteMany({ - where: { id: sessionId, userId }, + where: { id: match.id, userId }, }); return result.count > 0; } diff --git a/src/lib/validations/nightscout.ts b/src/lib/validations/nightscout.ts index 8b0e5a77d..010f2d753 100644 --- a/src/lib/validations/nightscout.ts +++ b/src/lib/validations/nightscout.ts @@ -7,11 +7,17 @@ import { z } from "zod/v4"; * on `User` (`nightscoutUrlEncrypted` / `nightscoutTokenEncrypted`); the * private-host opt-in maps to `nightscoutAllowPrivateHost`. * - * The URL is validated to be a parseable http(s) origin at input time. The - * SSRF floor (public-host requirement, unless `allowPrivateHost` is set) is - * enforced at fetch time by the client through `safeFetch` — input-shape - * validation here only rejects garbage early so the connect route can return - * a clear 422 instead of letting a malformed string reach the egress layer. + * The URL is validated to be a parseable http(s) origin here. The SSRF floor + * (public-host requirement, unless `allowPrivateHost` is set) is asserted + * twice on purpose: the connect route runs `isPublicUrl` before it persists + * anything, and `safeFetch` re-checks plus pins the resolved IP at connect + * time. The shape check here only rejects garbage early so the route can + * return a clear 422 instead of letting a malformed string reach either. + * + * The floor is NOT expressed as a Zod refinement because it depends on the + * sibling `allowPrivateHost` field and needs its own error message; folding + * it in here would collapse "malformed URL" and "private host, opt in to + * reach it" into one opaque 422. */ export const nightscoutConnectSchema = z.object({ // Trimmed: a trailing space / newline from a copy button reaches the diff --git a/tests/integration/auth-flow.test.ts b/tests/integration/auth-flow.test.ts index 5f269a6d9..eb878779e 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 0255)", () => { + 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/auth-password-change.test.ts b/tests/integration/auth-password-change.test.ts index d3000ff69..44bc9f7bb 100644 --- a/tests/integration/auth-password-change.test.ts +++ b/tests/integration/auth-password-change.test.ts @@ -21,6 +21,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { cookieJar, headerJar } from "./mock-next-headers"; import { getPrismaClient, truncateAllTables } from "./setup"; +import { hashToken } from "@/lib/auth/hmac"; vi.mock("next/headers", async () => { const { cookieJar, headerJar } = await import("./mock-next-headers"); @@ -140,7 +141,14 @@ describe("POST /api/auth/password (real Postgres)", () => { expect(remainingSessions).toHaveLength(1); expect(remainingSessions[0].id).not.toBe(sessionId); expect(remainingSessions[0].id).not.toBe(siblingSession.id); - expect(cookieJar.get("healthlog_session")).toBe(remainingSessions[0].id); + + // v1.30.32 — the cookie carries the session secret, not the row id, so + // bind them through the stored hash instead. Strictly stronger than the + // old identity check: it proves the cookie actually authenticates the + // surviving row, and that the row id never leaves the server. + const rotatedCookie = cookieJar.get("healthlog_session")!; + expect(rotatedCookie).not.toBe(remainingSessions[0].id); + expect(remainingSessions[0].tokenHash).toBe(hashToken(rotatedCookie)); // The API token is flipped to revoked rather than deleted so the // audit trail survives. diff --git a/tests/integration/sessions-and-known-devices.test.ts b/tests/integration/sessions-and-known-devices.test.ts index 2ea8d6743..fb9cbc7c5 100644 --- a/tests/integration/sessions-and-known-devices.test.ts +++ b/tests/integration/sessions-and-known-devices.test.ts @@ -5,12 +5,17 @@ * Covers the DB-layer contracts the unit mocks can't pin: * - destroyOtherSessions keeps the current session, deletes the rest, revokes * native refresh tokens, and leaves API tokens untouched. - * - destroySessionById is scoped to the owning user (no cross-user delete). + * - destroySessionById is scoped to the owning user (no cross-user delete) + * and takes the public handle, never the row id. * - the (userId, deviceHash) unique index enforces the login-alert dedupe. */ import { beforeEach, describe, expect, it } from "vitest"; -import { destroyOtherSessions, destroySessionById } from "@/lib/auth/session"; +import { + destroyOtherSessions, + destroySessionById, + sessionHandle, +} from "@/lib/auth/session"; import { getPrismaClient, truncateAllTables } from "./setup"; beforeEach(async () => { @@ -85,14 +90,26 @@ describe("destroySessionById", () => { data: { userId: other.id, expiresAt: new Date(Date.now() + 1e6) }, }); - // Cross-user attempt: owner tries to delete the intruder's session. - expect(await destroySessionById(owner.id, otherSession.id)).toBe(false); + // Cross-user attempt: owner presents the intruder's handle. + expect( + await destroySessionById(owner.id, sessionHandle(otherSession.id)), + ).toBe(false); expect( await prisma.session.findUnique({ where: { id: otherSession.id } }), ).not.toBeNull(); - // Owned delete succeeds. - expect(await destroySessionById(owner.id, ownerSession.id)).toBe(true); + // The row id is not an accepted key: a caller who somehow learned it + // still cannot revoke with it, which is what keeps the id out of the + // client's hands from being load-bearing in both directions. + expect(await destroySessionById(owner.id, ownerSession.id)).toBe(false); + expect( + await prisma.session.findUnique({ where: { id: ownerSession.id } }), + ).not.toBeNull(); + + // Owned delete succeeds on the handle. + expect( + await destroySessionById(owner.id, sessionHandle(ownerSession.id)), + ).toBe(true); expect( await prisma.session.findUnique({ where: { id: ownerSession.id } }), ).toBeNull(); 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