From 58c5416bc9d5527ac07a5e80afc341845bb5b0ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 17 May 2026 00:21:44 +0200 Subject: [PATCH] security: destroyAllSessions wipes API tokens + refresh tokens; tighten well-known proxy bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the v1.4.34.3 security audit, both with the same shape — every authenticated surface a user owns should rotate when a credential-rotation path fires, and unfamiliar discovery sub-paths must not inherit "no auth" status by default. destroyAllSessions: Previously only ran `prisma.session.deleteMany`, leaving every long-lived `ApiToken` row (Bearer credentials issued via /settings/api-tokens) and every native-client `RefreshToken` intact. A stolen iOS Bearer kept working after the user rotated their password or after an admin reset; only the browser cookie flipped. Wraps the cleanup in a $transaction that flips `apiToken.revoked = true` and `refreshToken.revokedAt = now()` in the same atomic block so the audit trail survives. Both lookups at `src/lib/auth/api-token.ts` and `src/lib/auth/refresh-token.ts` already filter on these columns; no second code path needed. Same destroyAllSessions call already runs from: - POST /api/auth/password (user-initiated change) - POST /api/admin/users/[id]/reset-password (admin reset) - DELETE /api/settings/account (account deletion) All three paths now wipe every transport in one shot. Well-known proxy bypass: `src/proxy.ts` PUBLIC_PATHS carried "/.well-known/" with a prefix match — a future route at `/.well-known/foo` would inherit "no auth" status silently. Tighten to an exact-match Set (WELL_KNOWN_PUBLIC_PATHS) seeded with the one path we actually serve (`apple-app-site-association`). New routes must be added explicitly. Also drops the stale `/api/auth/codex/callback` PUBLIC_PATHS entry — that route doesn't exist in the codebase (the Codex OAuth flow uses `/api/auth/codex/{device-poll,device-start, disconnect}` instead). Tests: - New destroyAllSessions assertion in src/lib/auth/__tests__/ session.test.ts pins the three updateMany/deleteMany call shapes inside a single $transaction. - Existing proxy well-known + privacy + onboarding redirect suites stay green. --- src/lib/auth/__tests__/session.test.ts | 38 +++++++++++++++++++++++++- src/lib/auth/session.ts | 27 +++++++++++++++++- src/proxy.ts | 26 ++++++++++++------ 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/src/lib/auth/__tests__/session.test.ts b/src/lib/auth/__tests__/session.test.ts index efbfe8d4a..332ea303f 100644 --- a/src/lib/auth/__tests__/session.test.ts +++ b/src/lib/auth/__tests__/session.test.ts @@ -13,6 +13,13 @@ vi.mock("@/lib/db", () => ({ deleteMany: vi.fn(), update: vi.fn(), }, + apiToken: { + updateMany: vi.fn(), + }, + refreshToken: { + updateMany: vi.fn(), + }, + $transaction: vi.fn(async (ops: Promise[]) => Promise.all(ops)), }, })); @@ -36,7 +43,7 @@ vi.mock("next/headers", () => ({ })); import { prisma } from "@/lib/db"; -import { getSession } from "../session"; +import { getSession, destroyAllSessions } from "../session"; beforeEach(() => { vi.resetAllMocks(); @@ -63,3 +70,32 @@ describe("getSession", () => { expect(cookieState.delete).toHaveBeenCalledWith("hl_onboarding"); }); }); + +describe("destroyAllSessions", () => { + it("revokes web sessions, API tokens, and refresh tokens for the user", async () => { + vi.mocked(prisma.session.deleteMany).mockResolvedValue({ + count: 2, + } as never); + vi.mocked(prisma.apiToken.updateMany).mockResolvedValue({ + count: 1, + } as never); + vi.mocked(prisma.refreshToken.updateMany).mockResolvedValue({ + count: 3, + } as never); + + await destroyAllSessions("user-rotated"); + + expect(prisma.session.deleteMany).toHaveBeenCalledWith({ + where: { userId: "user-rotated" }, + }); + expect(prisma.apiToken.updateMany).toHaveBeenCalledWith({ + where: { userId: "user-rotated", revoked: false }, + data: { revoked: true }, + }); + expect(prisma.refreshToken.updateMany).toHaveBeenCalledWith({ + where: { userId: "user-rotated", revokedAt: null }, + data: { revokedAt: expect.any(Date) }, + }); + expect(prisma.$transaction).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index 0322773e0..b00870e64 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -156,8 +156,33 @@ export async function destroySession(): Promise { cookieStore.delete(ONBOARDING_COOKIE); } +/** + * Revoke every authenticated surface a user owns: web sessions, API + * tokens (long-lived Bearer credentials issued via `/settings/api-tokens`), + * and native-client refresh tokens. Called from every credential-rotation + * path (`/api/auth/password`, `/api/admin/users/[id]/reset-password`, + * `DELETE /api/settings/account`) so a remediated user wipes a stolen + * credential across every transport, not just the browser cookie. + * + * `ApiToken` flips `revoked=true` (the token-hash lookup at + * `src/lib/auth/api-token.ts` filters on this column). `RefreshToken` + * sets `revokedAt` (the rotation lookup at + * `src/lib/auth/refresh-token.ts` short-circuits on a non-null value). + * Both run as `updateMany` rather than `deleteMany` so the audit trail + * survives — the rows linger as evidence of the rotation event. + */ export async function destroyAllSessions(userId: string): Promise { - await prisma.session.deleteMany({ where: { userId } }); + await prisma.$transaction([ + prisma.session.deleteMany({ where: { userId } }), + prisma.apiToken.updateMany({ + where: { userId, revoked: false }, + data: { revoked: true }, + }), + prisma.refreshToken.updateMany({ + where: { userId, revokedAt: null }, + data: { revokedAt: new Date() }, + }), + ]); } /** diff --git a/src/proxy.ts b/src/proxy.ts index c1bdd719a..d85be54d6 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -23,7 +23,6 @@ const PUBLIC_PATHS = [ "/api/withings/webhook", "/api/telegram/webhook", "/api/integrations/moodlog/webhook", - "/api/auth/codex/callback", "/api/ingest/", // v1.4.26 — `/privacy` is a public legal page. iOS App Store Connect // requires a publicly reachable Privacy-Policy URL during submission; @@ -33,19 +32,25 @@ const PUBLIC_PATHS = [ // alongside the project credits. The CC licence requires the // attribution to be reachable without a sign-in. "/about", - // v1.4.33 — `/.well-known/*` covers IETF-registered discovery - // endpoints (RFC 8615). Apple reads - // `/.well-known/apple-app-site-association` without credentials to - // wire Web Credentials (passkey sharing) and Universal Links to the - // iOS bundle, so the path must answer 200 with the bare JSON body - // before any auth gate runs. The trailing slash future-proofs the - // namespace for `/security.txt`, `/openid-configuration`, etc. - "/.well-known/", // `/onboarding` itself + its subroutes are matched exactly via // `isPublicPath()` so we don't admit `/onboarding-export` etc. "/robots.txt", ]; +/** + * Exact-match allowlist for IETF-registered discovery endpoints + * (RFC 8615). Apple reads `/.well-known/apple-app-site-association` + * without credentials to wire passkey sharing and Universal Links; + * the bare JSON body has to answer 200 before any auth gate runs. + * + * Exact match (not prefix) — a future `/.well-known/openid-configuration` + * or `/.well-known/security.txt` must be added here explicitly so a new + * sub-path doesn't auto-inherit "no auth" status. + */ +const WELL_KNOWN_PUBLIC_PATHS = new Set([ + "/.well-known/apple-app-site-association", +]); + /** * v1.4.22 W5 reconcile (Sec-MED-2) — `/onboarding` matches exact * page + subroutes (`/onboarding`, `/onboarding/*`) only, NOT the @@ -59,6 +64,9 @@ function isPublicPath(pathname: string): boolean { if (pathname === "/onboarding" || pathname.startsWith("/onboarding/")) { return true; } + if (WELL_KNOWN_PUBLIC_PATHS.has(pathname)) { + return true; + } return PUBLIC_PATHS.some((p) => pathname.startsWith(p)); }