mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(auth): answer a malformed session cookie as no session
The session lookup turned every query error into a 503, so a cookie holding a NUL byte reached the legacy id path, Postgres refused the value, and an anonymous caller could produce server errors at will. A cookie that cannot be a secret or a legacy id is now answered as no session before the database is asked, and only an unreachable store (connection errors, timeouts, an exhausted pool) is a 503. Any other query error is answered like a value that matches nothing.
This commit is contained in:
1 parent
c300574a8e
commit
2f7f04ecab
4 files changed
+124
-22
No files matched your search
@@ -66,14 +66,17 @@ vi.mock("next/headers", () => ({
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "../session";
|
||||
|
||||
/** A cookie value in the shape `mintSessionSecret` produces. */
|
||||
const SECRET = `hls_${"a".repeat(64)}`;
|
||||
|
||||
const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000;
|
||||
|
||||
function seedSession(userLocale: string | null) {
|
||||
cookieState.sessionId = "hls_raw-secret";
|
||||
cookieState.sessionId = SECRET;
|
||||
store.row = {
|
||||
id: "sess-1",
|
||||
userId: "user-1",
|
||||
tokenHash: "hash:hls_raw-secret",
|
||||
tokenHash: `hash:${SECRET}`,
|
||||
// Far enough out that the sliding-expiry refresh stays quiet.
|
||||
expiresAt: new Date(Date.now() + THIRTY_DAYS_MS),
|
||||
lastActiveAt: new Date(),
|
||||
|
||||
@@ -91,6 +91,11 @@ import {
|
||||
} from "../session";
|
||||
import { SessionStoreUnavailableError } from "@/lib/api-errors";
|
||||
|
||||
/** A cookie value in the shape `mintSessionSecret` produces. */
|
||||
const SECRET = `hls_${"a".repeat(64)}`;
|
||||
const FORGED_SECRET = `hls_${"b".repeat(64)}`;
|
||||
const UNKNOWN_SECRET = `hls_${"c".repeat(64)}`;
|
||||
|
||||
const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000;
|
||||
|
||||
function seed(row: Partial<SessionRow> & { id: string }): SessionRow {
|
||||
@@ -155,21 +160,21 @@ describe("createSession", () => {
|
||||
|
||||
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 row = seed({ id: "sess_cuid_1", tokenHash: `hash:${SECRET}` });
|
||||
cookieState.sessionId = SECRET;
|
||||
|
||||
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" } }),
|
||||
expect.objectContaining({ where: { tokenHash: `hash:${SECRET}` } }),
|
||||
);
|
||||
});
|
||||
|
||||
it("refuses a modern-shaped cookie whose hash matches nothing", async () => {
|
||||
seed({ id: "sess_cuid_1", tokenHash: "hash:hls_real" });
|
||||
cookieState.sessionId = "hls_forged";
|
||||
cookieState.sessionId = FORGED_SECRET;
|
||||
|
||||
await expect(getSession()).resolves.toBeNull();
|
||||
});
|
||||
@@ -202,10 +207,10 @@ 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",
|
||||
tokenHash: `hash:${SECRET}`,
|
||||
expiresAt: new Date(Date.now() + 2 * 24 * 60 * 60 * 1000),
|
||||
});
|
||||
cookieState.sessionId = "hls_abc";
|
||||
cookieState.sessionId = SECRET;
|
||||
|
||||
await getSession();
|
||||
|
||||
@@ -215,7 +220,7 @@ describe("getSession — sliding expiry", () => {
|
||||
const cookieValue = cookieState.set.mock.calls.find(
|
||||
(c) => c[0] === "healthlog_session",
|
||||
)![1];
|
||||
expect(cookieValue).toBe("hls_abc");
|
||||
expect(cookieValue).toBe(SECRET);
|
||||
});
|
||||
|
||||
it("withholds the extension from a legacy session so the id path drains", async () => {
|
||||
@@ -270,7 +275,7 @@ describe("a session lookup that fails is not a missing session", () => {
|
||||
}
|
||||
|
||||
it("getSession raises a 503 with a retryable code", async () => {
|
||||
cookieState.sessionId = "hls_abc";
|
||||
cookieState.sessionId = SECRET;
|
||||
failLookup();
|
||||
const err = await getSession().catch((e: unknown) => e);
|
||||
expect(err).toBeInstanceOf(SessionStoreUnavailableError);
|
||||
@@ -284,26 +289,64 @@ describe("a session lookup that fails is not a missing session", () => {
|
||||
|
||||
it("validateSessionFromCookieValue raises instead of answering null", async () => {
|
||||
failLookup();
|
||||
await expect(
|
||||
validateSessionFromCookieValue("hls_abc"),
|
||||
).rejects.toBeInstanceOf(SessionStoreUnavailableError);
|
||||
await expect(validateSessionFromCookieValue(SECRET)).rejects.toBeInstanceOf(
|
||||
SessionStoreUnavailableError,
|
||||
);
|
||||
});
|
||||
|
||||
it("validateSessionWithCreatedAt raises instead of answering null", async () => {
|
||||
failLookup();
|
||||
await expect(
|
||||
validateSessionWithCreatedAt("hls_abc"),
|
||||
).rejects.toBeInstanceOf(SessionStoreUnavailableError);
|
||||
await expect(validateSessionWithCreatedAt(SECRET)).rejects.toBeInstanceOf(
|
||||
SessionStoreUnavailableError,
|
||||
);
|
||||
});
|
||||
|
||||
it("raises for a pool that had no connection free (P2024)", async () => {
|
||||
vi.mocked(prisma.session.findUnique).mockRejectedValueOnce(
|
||||
Object.assign(new Error("connection pool exhausted"), {
|
||||
code: "P2024",
|
||||
}) as never,
|
||||
);
|
||||
await expect(validateSessionFromCookieValue(SECRET)).rejects.toBeInstanceOf(
|
||||
SessionStoreUnavailableError,
|
||||
);
|
||||
});
|
||||
|
||||
it("answers null, not 503, when the query refused the value itself", async () => {
|
||||
// Postgres 22021: a byte sequence it will not accept in text. Nothing is
|
||||
// wrong with the store; a 503 here would let any anonymous caller produce
|
||||
// server errors on demand.
|
||||
vi.mocked(prisma.session.findUnique).mockRejectedValueOnce(
|
||||
Object.assign(new Error("invalid byte sequence for encoding"), {
|
||||
code: "P2010",
|
||||
meta: { driverAdapterError: { cause: { originalCode: "22021" } } },
|
||||
}) as never,
|
||||
);
|
||||
await expect(validateSessionFromCookieValue("sess_legacy")).resolves.toBe(
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["a NUL byte", "\u0000"],
|
||||
["a percent-encoded NUL", "%00"],
|
||||
["a short secret", "hls_abc"],
|
||||
["a secret with non-hex", `hls_${"z".repeat(64)}`],
|
||||
["an overlong legacy id", "c".repeat(65)],
|
||||
])("never asks the database about %s", async (_name, value) => {
|
||||
vi.mocked(prisma.session.findUnique).mockClear();
|
||||
await expect(validateSessionFromCookieValue(value)).resolves.toBe(null);
|
||||
expect(prisma.session.findUnique).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still answers null for a cookie that matches nothing", async () => {
|
||||
await expect(validateSessionFromCookieValue("hls_nope")).resolves.toBe(
|
||||
await expect(validateSessionFromCookieValue(UNKNOWN_SECRET)).resolves.toBe(
|
||||
null,
|
||||
);
|
||||
});
|
||||
|
||||
it("logout still clears the cookie when the lookup fails", async () => {
|
||||
cookieState.sessionId = "hls_abc";
|
||||
cookieState.sessionId = SECRET;
|
||||
failLookup();
|
||||
await expect(destroySession()).resolves.toBeUndefined();
|
||||
expect(cookieState.delete).toHaveBeenCalledWith("healthlog_session");
|
||||
@@ -314,8 +357,8 @@ 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";
|
||||
seed({ id: "sess_cuid_1", tokenHash: `hash:${SECRET}` });
|
||||
cookieState.sessionId = SECRET;
|
||||
|
||||
await destroySession();
|
||||
|
||||
|
||||
+34
-2
@@ -6,7 +6,7 @@ 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";
|
||||
import { isP2025 } from "@/lib/prisma-errors";
|
||||
import { isDatabaseUnavailable, 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";
|
||||
@@ -41,6 +41,19 @@ function mintSessionSecret(): string {
|
||||
return `${SESSION_TOKEN_PREFIX}${randomBytes(SESSION_SECRET_BYTES).toString("hex")}`;
|
||||
}
|
||||
|
||||
/** Exactly what `mintSessionSecret` produces. */
|
||||
const SESSION_SECRET_SHAPE = new RegExp(
|
||||
`^${SESSION_TOKEN_PREFIX}[0-9a-f]{${SESSION_SECRET_BYTES * 2}}$`,
|
||||
);
|
||||
|
||||
/**
|
||||
* What a legacy cookie (a `Session.id`, a cuid) can look like. Deliberately a
|
||||
* character class rather than the cuid grammar: it exists to keep a value that
|
||||
* cannot be an id away from the query, where a NUL byte or invalid UTF-8 makes
|
||||
* Postgres raise instead of finding nothing.
|
||||
*/
|
||||
const LEGACY_SESSION_ID_SHAPE = /^[A-Za-z0-9_-]{1,64}$/;
|
||||
|
||||
/**
|
||||
* Resolve the session row a cookie value refers to, or null.
|
||||
*
|
||||
@@ -74,6 +87,14 @@ function mintSessionSecret(): string {
|
||||
* login from the deploy onward is on a secret immediately.
|
||||
*/
|
||||
async function findSessionByCookie(cookieValue: string) {
|
||||
// A malformed value matches no session: it is answered as one, before the
|
||||
// database is asked, so an anonymous caller cannot turn a crafted cookie into
|
||||
// a query error.
|
||||
if (cookieValue.startsWith(SESSION_TOKEN_PREFIX)) {
|
||||
if (!SESSION_SECRET_SHAPE.test(cookieValue)) return null;
|
||||
} else if (!LEGACY_SESSION_ID_SHAPE.test(cookieValue)) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
if (cookieValue.startsWith(SESSION_TOKEN_PREFIX)) {
|
||||
return await prisma.session.findUnique({
|
||||
@@ -94,7 +115,18 @@ async function findSessionByCookie(cookieValue: string) {
|
||||
// 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);
|
||||
//
|
||||
// Only for a store that could not be reached. Any other query error says
|
||||
// something about the value asked for, not about the store, and is
|
||||
// answered like a value that matches nothing: a 503 there would let any
|
||||
// anonymous caller produce server errors at will.
|
||||
if (isDatabaseUnavailable(error)) {
|
||||
throw new SessionStoreUnavailableError(error);
|
||||
}
|
||||
getEvent()?.addWarning(
|
||||
`session lookup refused the value: ${error instanceof Error ? error.name : "unknown"}`,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -13,6 +13,8 @@
|
||||
* unique-constraint code.
|
||||
*/
|
||||
|
||||
import { classifyRestoreFailure } from "@/lib/export/restore-failure-cause";
|
||||
|
||||
/** Narrow a thrown Prisma error to a specific known-request code. */
|
||||
function hasPrismaCode(err: unknown, code: string): boolean {
|
||||
return (
|
||||
@@ -37,3 +39,25 @@ export function isP2002(err: unknown): boolean {
|
||||
export function isP2025(err: unknown): boolean {
|
||||
return hasPrismaCode(err, "P2025");
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a query failed because the database could not be reached, as opposed
|
||||
* to refusing what it was asked. The first is worth a retry and is a 503; the
|
||||
* second is an answer about the input.
|
||||
*
|
||||
* Reads the same signals the restore classifier does (`connection` and
|
||||
* `timeout`: a connection SQLSTATE, a server shutting down, a statement or
|
||||
* idle timeout, Prisma's P1001 / P1002 / P1017 / P2028, a terminated
|
||||
* connection), plus the two that only a pooled request meets: P2024, no
|
||||
* connection free in the pool within its timeout, and P1008, an operation that
|
||||
* timed out.
|
||||
*/
|
||||
export function isDatabaseUnavailable(err: unknown): boolean {
|
||||
if (hasPrismaCode(err, "P2024") || hasPrismaCode(err, "P1008")) return true;
|
||||
const { cause } = classifyRestoreFailure(err);
|
||||
if (cause === "connection" || cause === "timeout") return true;
|
||||
const message = err instanceof Error ? err.message : "";
|
||||
return /ECONNREFUSED|ECONNRESET|ETIMEDOUT|Can't reach database server|timed out/i.test(
|
||||
message,
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user