mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(auth): answer 503 for every session lookup failure that is not about the value
The catch kept a denylist of "store unreachable" signals and folded everything else into a missing session. The pool's own connect timeout carries no code, and 53300, 53200 and 55P03 are about the store, so all four answered 401 and signed the client out. The shape check already refuses crafted values before the query; the catch now keeps only a short allowlist of input refusals (SQLSTATE class 22, Prisma validation) as null and raises 503 for the rest.
This commit is contained in:
1 parent
2b8defcbd2
commit
47f20b773d
3 files changed
+85
-24
No files matched your search
@@ -312,6 +312,42 @@ describe("a session lookup that fails is not a missing session", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("raises for the pool's own connect timeout, which carries no code", async () => {
|
||||
// pg-pool rejects with exactly this message and nothing else; the driver
|
||||
// adapter hands it through unchanged.
|
||||
vi.mocked(prisma.session.findUnique).mockRejectedValueOnce(
|
||||
new Error("timeout exceeded when trying to connect") as never,
|
||||
);
|
||||
await expect(validateSessionFromCookieValue(SECRET)).rejects.toBeInstanceOf(
|
||||
SessionStoreUnavailableError,
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["53300", "sorry, too many clients already"],
|
||||
["55P03", "canceling statement due to lock timeout"],
|
||||
["53200", "out of memory"],
|
||||
])("raises for SQLSTATE %s, which is about the store", async (state, msg) => {
|
||||
vi.mocked(prisma.session.findUnique).mockRejectedValueOnce(
|
||||
Object.assign(new Error(msg), {
|
||||
code: "P2010",
|
||||
meta: { driverAdapterError: { cause: { originalCode: state } } },
|
||||
}) as never,
|
||||
);
|
||||
await expect(validateSessionFromCookieValue(SECRET)).rejects.toBeInstanceOf(
|
||||
SessionStoreUnavailableError,
|
||||
);
|
||||
});
|
||||
|
||||
it("answers null for a Prisma validation refusal", async () => {
|
||||
vi.mocked(prisma.session.findUnique).mockRejectedValueOnce(
|
||||
Object.assign(new Error("Invalid value provided"), {
|
||||
name: "PrismaClientValidationError",
|
||||
}) as never,
|
||||
);
|
||||
await expect(validateSessionFromCookieValue(SECRET)).resolves.toBe(null);
|
||||
});
|
||||
|
||||
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
|
||||
|
||||
@@ -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 { isDatabaseUnavailable, isP2025 } from "@/lib/prisma-errors";
|
||||
import { isInputRefusal, 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";
|
||||
@@ -116,11 +116,13 @@ async function findSessionByCookie(cookieValue: string) {
|
||||
// signed its person out. It is a 503 now, with a code a client can retry
|
||||
// on (`auth.session.unavailable`).
|
||||
//
|
||||
// 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)) {
|
||||
// The shape check above already refused every crafted value before the
|
||||
// query, so a failure here is about the store, not the caller: the pool
|
||||
// had no connection free, the server ran out of connections or memory, a
|
||||
// lock wait ran out. All of those are a 503. Only a refusal that names
|
||||
// the value itself (a data exception, a Prisma validation error) is
|
||||
// answered like a value that matches nothing.
|
||||
if (!isInputRefusal(error)) {
|
||||
throw new SessionStoreUnavailableError(error);
|
||||
}
|
||||
getEvent()?.addWarning(
|
||||
|
||||
+41
-18
@@ -41,23 +41,46 @@ export function isP2025(err: unknown): boolean {
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
* Prisma's own codes for a query it refused because of the value it was given:
|
||||
* a value too long for its column (P2000), an invalid value or type for a
|
||||
* field (P2006, P2007, P2023), and a query that failed validation (P2009,
|
||||
* P2012, P2019, P2020).
|
||||
*/
|
||||
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,
|
||||
);
|
||||
const PRISMA_INPUT_CODES = new Set([
|
||||
"P2000",
|
||||
"P2006",
|
||||
"P2007",
|
||||
"P2009",
|
||||
"P2012",
|
||||
"P2019",
|
||||
"P2020",
|
||||
"P2023",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Whether a query failed because of the value it was asked about, rather than
|
||||
* because of the store it asked. Deliberately a short allowlist: Postgres'
|
||||
* data-exception class (SQLSTATE 22, an encoding or a cast it will not accept)
|
||||
* and Prisma's validation refusals. Everything else, including failures with
|
||||
* no recognisable code at all, is not an answer about the input.
|
||||
*
|
||||
* The allowlist runs this way round because the other direction cannot be
|
||||
* closed: the pool's own "timeout exceeded when trying to connect" carries no
|
||||
* code, nor do half the ways a connection ends, and a resource SQLSTATE
|
||||
* (53300 too many connections, 53200 out of memory, 55P03 a lock timeout) says
|
||||
* nothing about the value either. A denylist of "the database is down" signals
|
||||
* folded each of those into "this value matches nothing".
|
||||
*/
|
||||
export function isInputRefusal(err: unknown): boolean {
|
||||
if (
|
||||
typeof err === "object" &&
|
||||
err !== null &&
|
||||
(err as { name?: unknown }).name === "PrismaClientValidationError"
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
const { code } = classifyRestoreFailure(err);
|
||||
if (code === null) return false;
|
||||
if (PRISMA_INPUT_CODES.has(code)) return true;
|
||||
return /^22[0-9A-Z]{3}$/.test(code);
|
||||
}
|
||||
Reference in new issue
Block a user