diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index af613a5d8..c851293d9 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -81,7 +81,9 @@ paths: route, including any Bearer token against an admin surface) and `auth.admin.required` (a cookie session that is not an admin's). A route that checks a credential of its own — sign-in, the MFA challenge exchange, a password confirmation — may still answer 401 with prose and no code, so a missing `meta.errorCode` means the - route refused the credential it was handed, not that the response is malformed." + route refused the credential it was handed, not that the response is malformed. A session that could not be + checked because the session store was unreachable is NOT a 401: it answers 503 with + `auth.session.unavailable`, and the client keeps its credential and retries." content: application/json: schema: @@ -36112,35 +36114,35 @@ components: anamnesis.lifeConfirm.invalid, anamnesis.lifeSections.invalid; auth — auth.admin.required, auth.already_authenticated, auth.mfa.code_invalid, auth.missing, auth.refresh.invalid, auth.refresh.reuse, auth.refresh.revoked, auth.reproof.failed, auth.reproof.required, - auth.reproof.too_weak, auth.scope.insufficient, auth.session.insecure_transport, auth.stepup.required, - auth.token.expired, auth.token.invalid; backup — backup.foreign_reference, backup.key.missing, - backup.payload.undecryptable, backup.restore.active, backup.section.missing, backup.upload.too_large; - biomarkers — biomarkers.unit.locked; body-site — body-site.invalid; coach — coach.budget.exceeded, - coach.conversation.invalidTitle, coach.conversation.notFound, coach.fenced.attachmentConflict, - coach.fenced.attachmentLimit, coach.fenced.attachmentNotFound, coach.fenced.attachmentNotIndexed, - coach.fenced.attachmentRequired, coach.fenced.attachmentUnavailable, coach.fenced.invalid, - coach.request.invalid; coach-prefs — coach-prefs.body.invalid_json, coach-prefs.body.invalid_shape; - consent — consent.ai.confirmRequired, consent.ai.required; cycle — cycle.bulk.invalid, - cycle.bulk.too_large, cycle.calendar.invalid, cycle.calendar.range, cycle.cycles.invalid, - cycle.day-log.conflict, cycle.day-log.invalid, cycle.disabled, cycle.period.invalid, - cycle.symptom.custom.invalid, cycle.symptom.custom.limit; cycle-prefs — cycle-prefs.body.invalid_json, - cycle-prefs.invalid; disable-coach — disable-coach.body.invalid_json; dawarich — - dawarich.credentials_rejected, dawarich.home_place_unknown, dawarich.invalid, dawarich.not_connected, - dawarich.rate_limited, dawarich.unreachable; day — day.invalid; demo — demo.readOnly; documents — - documents.chat.conversationNotFound, documents.chat.invalid, documents.inbound.alreadyConfirmed, - documents.inbound.alreadyPartlyConfirmed, documents.inbound.budgetExceeded, - documents.inbound.decryptFailed, documents.inbound.encounterNotFound, documents.inbound.episodeNotFound, - documents.inbound.extractFailed, documents.inbound.factNotFound, documents.inbound.factNotPending, - documents.inbound.factTypeMismatch, documents.inbound.fileTooLarge, documents.inbound.fileType, - documents.inbound.invalidMetadata, documents.inbound.invalidQuery, documents.inbound.invalidUpdate, - documents.inbound.localOcrDisabled, documents.inbound.notFound, documents.inbound.notIndexed, - documents.inbound.pdfNeedsAnthropic, documents.inbound.providerUnsupported, - documents.inbound.quotaExceeded, documents.inbound.rateLimited, documents.inbound.restoreDuplicate, - documents.inbound.restoreGone, documents.inbound.sourceAliasLimit, documents.inbound.uploadBusy, - documents.inbound.uploadTimeout, documents.inbound.vaccinationNotFound, documents.sources.authRefused, - documents.sources.badResponse, documents.sources.browserOnly, documents.sources.invalidAddress, - documents.sources.linkTargetNotFound, documents.sources.notConnected, documents.sources.notFound, - documents.sources.organizationRequired, documents.sources.originNotAllowed, + auth.reproof.too_weak, auth.scope.insufficient, auth.session.insecure_transport, + auth.session.unavailable, auth.stepup.required, auth.token.expired, auth.token.invalid; backup — + backup.foreign_reference, backup.key.missing, backup.payload.undecryptable, backup.restore.active, + backup.section.missing, backup.upload.too_large; biomarkers — biomarkers.unit.locked; body-site — + body-site.invalid; coach — coach.budget.exceeded, coach.conversation.invalidTitle, + coach.conversation.notFound, coach.fenced.attachmentConflict, coach.fenced.attachmentLimit, + coach.fenced.attachmentNotFound, coach.fenced.attachmentNotIndexed, coach.fenced.attachmentRequired, + coach.fenced.attachmentUnavailable, coach.fenced.invalid, coach.request.invalid; coach-prefs — + coach-prefs.body.invalid_json, coach-prefs.body.invalid_shape; consent — consent.ai.confirmRequired, + consent.ai.required; cycle — cycle.bulk.invalid, cycle.bulk.too_large, cycle.calendar.invalid, + cycle.calendar.range, cycle.cycles.invalid, cycle.day-log.conflict, cycle.day-log.invalid, + cycle.disabled, cycle.period.invalid, cycle.symptom.custom.invalid, cycle.symptom.custom.limit; + cycle-prefs — cycle-prefs.body.invalid_json, cycle-prefs.invalid; disable-coach — + disable-coach.body.invalid_json; dawarich — dawarich.credentials_rejected, dawarich.home_place_unknown, + dawarich.invalid, dawarich.not_connected, dawarich.rate_limited, dawarich.unreachable; day — + day.invalid; demo — demo.readOnly; documents — documents.chat.conversationNotFound, + documents.chat.invalid, documents.inbound.alreadyConfirmed, documents.inbound.alreadyPartlyConfirmed, + documents.inbound.budgetExceeded, documents.inbound.decryptFailed, documents.inbound.encounterNotFound, + documents.inbound.episodeNotFound, documents.inbound.extractFailed, documents.inbound.factNotFound, + documents.inbound.factNotPending, documents.inbound.factTypeMismatch, documents.inbound.fileTooLarge, + documents.inbound.fileType, documents.inbound.invalidMetadata, documents.inbound.invalidQuery, + documents.inbound.invalidUpdate, documents.inbound.localOcrDisabled, documents.inbound.notFound, + documents.inbound.notIndexed, documents.inbound.pdfNeedsAnthropic, + documents.inbound.providerUnsupported, documents.inbound.quotaExceeded, documents.inbound.rateLimited, + documents.inbound.restoreDuplicate, documents.inbound.restoreGone, documents.inbound.sourceAliasLimit, + documents.inbound.uploadBusy, documents.inbound.uploadTimeout, documents.inbound.vaccinationNotFound, + documents.sources.authRefused, documents.sources.badResponse, documents.sources.browserOnly, + documents.sources.invalidAddress, documents.sources.linkTargetNotFound, documents.sources.notConnected, + documents.sources.notFound, documents.sources.organizationRequired, documents.sources.originNotAllowed, documents.sources.permissionMissing, documents.sources.rateLimited, documents.sources.redirected, documents.sources.tokenRequired, documents.sources.unavailable, documents.sources.unreachable, documents.sources.versionTooOld; encounter — encounter.invalid, encounter.link.invalid, diff --git a/src/lib/__tests__/api-handler-session-unavailable.test.ts b/src/lib/__tests__/api-handler-session-unavailable.test.ts new file mode 100644 index 000000000..a7845b14b --- /dev/null +++ b/src/lib/__tests__/api-handler-session-unavailable.test.ts @@ -0,0 +1,50 @@ +/** + * A session store that cannot be read answers 503 through `requireAuth`. + * + * The other half of the session-lookup fix: `session.ts` raises the typed + * error, and this proves it reaches the wire as a retryable 503 with its code + * rather than as the 401 that tells a client to throw its credential away. + */ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ prisma: {} })); +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/hmac", () => ({ hashToken: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/monitoring/glitchtip", () => ({ + sendGlitchtipEvent: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { NextRequest } from "next/server"; +import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiSuccess } from "@/lib/api-response"; +import { SessionStoreUnavailableError } from "@/lib/api-errors"; +import { getSession } from "@/lib/auth/session"; + +describe("requireAuth on a failed session lookup", () => { + it("answers 503 with auth.session.unavailable, not 401", async () => { + vi.mocked(getSession).mockRejectedValueOnce( + new SessionStoreUnavailableError(new Error("pool timeout")), + ); + const route = apiHandler(async (_request: NextRequest) => { + await requireAuth(); + return apiSuccess({ ok: true }); + }); + const res = await route(new NextRequest("http://localhost/api/probe")); + expect(res.status).toBe(503); + const body = await res.json(); + expect(body.data).toBeNull(); + expect(body.meta.errorCode).toBe("auth.session.unavailable"); + }); +}); diff --git a/src/lib/api-errors.ts b/src/lib/api-errors.ts index d891d3af1..b4145c935 100644 --- a/src/lib/api-errors.ts +++ b/src/lib/api-errors.ts @@ -62,8 +62,35 @@ export const AUTH_ERROR_CODES = { * the user out, which is the opposite of what the other 401 codes ask for. */ mfaCode: "auth.mfa.code_invalid", + /** + * The session store could not be read, so the request is neither signed in + * nor signed out. 503, not 401: a client that treats this as "signed out" + * drops a perfectly good session because the database blinked. Retry. + */ + sessionUnavailable: "auth.session.unavailable", } as const; +/** + * The session lookup failed, as opposed to finding nothing. + * + * Raised by the cookie resolver in `src/lib/auth/session.ts` when the query + * itself throws — a dropped connection, a pool timeout, a failover. Answering + * that with 401 told every client to discard its credential and sign in again + * for a fault on the server's side, and a reload during a database restart + * signed the person out. The cause rides along for the wide event. + */ +export class SessionStoreUnavailableError extends HttpError { + constructor(cause: unknown) { + super( + 503, + "Sign-in could not be checked right now. Try again shortly.", + AUTH_ERROR_CODES.sessionUnavailable, + ); + this.name = "SessionStoreUnavailableError"; + this.cause = cause; + } +} + /** * v1.36.0 — the two refusals the acting-account resolver can raise. * diff --git a/src/lib/auth/__tests__/session.test.ts b/src/lib/auth/__tests__/session.test.ts index 4ffeaf403..91ec94f5f 100644 --- a/src/lib/auth/__tests__/session.test.ts +++ b/src/lib/auth/__tests__/session.test.ts @@ -86,7 +86,10 @@ import { createSession, destroyAllSessions, destroySession, + validateSessionFromCookieValue, + validateSessionWithCreatedAt, } from "../session"; +import { SessionStoreUnavailableError } from "@/lib/api-errors"; const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; @@ -254,6 +257,59 @@ describe("getSession", () => { }); }); +describe("a session lookup that fails is not a missing session", () => { + // A dropped connection used to fold into `null`, which every caller reads + // as "signed out": the route answered 401 and the client discarded a good + // credential because the database blinked. + function failLookup() { + vi.mocked(prisma.session.findUnique).mockRejectedValueOnce( + Object.assign(new Error("Can't reach database server"), { + code: "P1001", + }) as never, + ); + } + + it("getSession raises a 503 with a retryable code", async () => { + cookieState.sessionId = "hls_abc"; + failLookup(); + const err = await getSession().catch((e: unknown) => e); + expect(err).toBeInstanceOf(SessionStoreUnavailableError); + expect(err).toMatchObject({ + statusCode: 503, + errorCode: "auth.session.unavailable", + }); + // Nothing about the browser's credential changes on a fault. + expect(cookieState.delete).not.toHaveBeenCalled(); + }); + + it("validateSessionFromCookieValue raises instead of answering null", async () => { + failLookup(); + await expect( + validateSessionFromCookieValue("hls_abc"), + ).rejects.toBeInstanceOf(SessionStoreUnavailableError); + }); + + it("validateSessionWithCreatedAt raises instead of answering null", async () => { + failLookup(); + await expect( + validateSessionWithCreatedAt("hls_abc"), + ).rejects.toBeInstanceOf(SessionStoreUnavailableError); + }); + + it("still answers null for a cookie that matches nothing", async () => { + await expect(validateSessionFromCookieValue("hls_nope")).resolves.toBe( + null, + ); + }); + + it("logout still clears the cookie when the lookup fails", async () => { + cookieState.sessionId = "hls_abc"; + failLookup(); + await expect(destroySession()).resolves.toBeUndefined(); + expect(cookieState.delete).toHaveBeenCalledWith("healthlog_session"); + }); +}); + 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 diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index afe723a05..aa08c5d00 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -7,6 +7,7 @@ import { ensureDbCompatibility } from "@/lib/db-compat"; import { getEvent } from "@/lib/logging/context"; import { shouldEmitSecureCookie } from "@/lib/auth/secure-cookie"; import { isP2025 } from "@/lib/prisma-errors"; +import { SessionStoreUnavailableError } from "@/lib/api-errors"; import { locales, type Locale } from "@/lib/i18n/config"; import { LOCALE_COOKIE, setLocaleCookie } from "@/lib/i18n/locale-cookie"; @@ -73,19 +74,28 @@ function mintSessionSecret(): string { * 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) }, + try { + if (cookieValue.startsWith(SESSION_TOKEN_PREFIX)) { + return await 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; + } catch (error) { + // "No such session" and "could not look" are different answers and only + // the first one is null. A failed query used to be folded into null by + // the callers, so a database restart answered 401 and every client + // signed its person out. It is a 503 now, with a code a client can retry + // on (`auth.session.unavailable`). + throw new SessionStoreUnavailableError(error); } - - 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; } /** @@ -106,7 +116,7 @@ export async function validateSessionFromCookieValue( cookieValue: string | undefined | null, ): Promise<{ session: { id: string; expiresAt: Date }; user: User } | null> { if (!cookieValue) return null; - const session = await findSessionByCookie(cookieValue).catch(() => null); + const session = await findSessionByCookie(cookieValue); if (!session || session.expiresAt < new Date()) return null; return { session: { id: session.id, expiresAt: session.expiresAt }, @@ -138,7 +148,7 @@ export async function validateSessionWithCreatedAt( user: User; } | null> { if (!cookieValue) return null; - const session = await findSessionByCookie(cookieValue).catch(() => null); + const session = await findSessionByCookie(cookieValue); if (!session || session.expiresAt < new Date()) return null; return { session: { @@ -462,7 +472,15 @@ export async function destroySession(): Promise { // 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. - const session = await findSessionByCookie(cookieValue).catch(() => null); + // A lookup that fails cannot find the row to delete, but must not keep + // the cookie in the browser either; the warning is what tells the + // operator a server-side session outlived its logout. + const session = await findSessionByCookie(cookieValue).catch((err) => { + getEvent()?.addWarning( + `destroySession lookup failed: ${err instanceof Error ? err.message : String(err)}`, + ); + return null; + }); if (session) { await prisma.session .delete({ where: { id: session.id } }) diff --git a/src/lib/openapi/error-codes.ts b/src/lib/openapi/error-codes.ts index ee8ad71f9..0360f87aa 100644 --- a/src/lib/openapi/error-codes.ts +++ b/src/lib/openapi/error-codes.ts @@ -81,6 +81,7 @@ export const ERROR_CODE_CATALOGUE: Readonly> = "auth.reproof.too_weak", "auth.scope.insufficient", "auth.session.insecure_transport", + "auth.session.unavailable", "auth.stepup.required", "auth.token.expired", "auth.token.invalid", diff --git a/src/lib/openapi/routes/shared.ts b/src/lib/openapi/routes/shared.ts index c8722ac9c..1f391bd0f 100644 --- a/src/lib/openapi/routes/shared.ts +++ b/src/lib/openapi/routes/shared.ts @@ -279,7 +279,7 @@ export const malformedJsonResponse = { export const stdResponses = { "401": { description: - "Authentication required or invalid credentials. The generic auth gates — the ones every route passes through before its own body runs — name the reason in `meta.errorCode`, and that is the field to branch on there; the English sentence beside it may be reworded or localised at any time. `auth.missing`: no session cookie and no Bearer header; send the user to sign in. `auth.token.expired`: the Bearer token's lifetime has passed; refresh and retry the request. `auth.token.invalid`: unknown, revoked, or the owning account is gone; discard the credential and sign in again — retrying will not help. `auth.mfa.code_invalid`: a second factor presented on a step-up flow did not verify; re-prompt for a code and keep the session. The related 403 refusals carry `auth.scope.insufficient` (the credential is valid but its scope does not reach this route, including any Bearer token against an admin surface) and `auth.admin.required` (a cookie session that is not an admin's). A route that checks a credential of its own — sign-in, the MFA challenge exchange, a password confirmation — may still answer 401 with prose and no code, so a missing `meta.errorCode` means the route refused the credential it was handed, not that the response is malformed.", + "Authentication required or invalid credentials. The generic auth gates — the ones every route passes through before its own body runs — name the reason in `meta.errorCode`, and that is the field to branch on there; the English sentence beside it may be reworded or localised at any time. `auth.missing`: no session cookie and no Bearer header; send the user to sign in. `auth.token.expired`: the Bearer token's lifetime has passed; refresh and retry the request. `auth.token.invalid`: unknown, revoked, or the owning account is gone; discard the credential and sign in again — retrying will not help. `auth.mfa.code_invalid`: a second factor presented on a step-up flow did not verify; re-prompt for a code and keep the session. The related 403 refusals carry `auth.scope.insufficient` (the credential is valid but its scope does not reach this route, including any Bearer token against an admin surface) and `auth.admin.required` (a cookie session that is not an admin's). A route that checks a credential of its own — sign-in, the MFA challenge exchange, a password confirmation — may still answer 401 with prose and no code, so a missing `meta.errorCode` means the route refused the credential it was handed, not that the response is malformed. A session that could not be checked because the session store was unreachable is NOT a 401: it answers 503 with `auth.session.unavailable`, and the client keeps its credential and retries.", content: { "application/json": { schema: errorEnvelope } }, }, "422": {