mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 13:22:14 +00:00
fix(auth): answer 503 when the session store cannot be read
The cookie resolvers folded a failed query into null, so a database restart answered 401 and every client discarded a good credential. The lookup now raises SessionStoreUnavailableError, which reaches the wire as 503 with auth.session.unavailable. Logout still clears the cookie on a failed lookup and records a warning.
This commit is contained in:
1 parent
9af22f2ab2
commit
c7f69fc212
7 files changed
+198
-44
No files matched your search
+32
-30
@@ -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,
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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.
|
||||
*
|
||||
|
||||
@@ -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
|
||||
|
||||
+31
-13
@@ -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<void> {
|
||||
// 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 } })
|
||||
|
||||
@@ -81,6 +81,7 @@ export const ERROR_CODE_CATALOGUE: Readonly<Record<string, readonly string[]>> =
|
||||
"auth.reproof.too_weak",
|
||||
"auth.scope.insufficient",
|
||||
"auth.session.insecure_transport",
|
||||
"auth.session.unavailable",
|
||||
"auth.stepup.required",
|
||||
"auth.token.expired",
|
||||
"auth.token.invalid",
|
||||
|
||||
@@ -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": {
|
||||
|
||||
Reference in new issue
Block a user