mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 21:10:59 +00:00
fix(api): declare the auth gate's 403 on every operation behind it
requireAuth answers 403 auth.scope.insufficient to a narrow Bearer token on a route that names no scope it holds, and requireAdmin answers 403 auth.admin.required to a session that is not an administrator's. Both run before any route body, and the contract only said so in the prose of the shared 401, so a client generated from it had no typed branch for either. The route kit accepted the scope refusal on any operation that declared a 401 for the same reason. The 403 is added to the assembled path table rather than to stdResponses. That set is spread into most operations, about three hundred of them carry a 403 of their own, and most spread the set after it, so a shared 403 there would have overwritten every module-off and sharing refusal in the document. withScopeRefusal adds the shared response where an operation has none and prefixes an operation's own 403 with the gate's code, keeping the sharing sentence last. Operations that take no credential, and the three surfaces that authenticate by a credential of their own, are left as they were. The kit no longer tolerates the refusal on an operation's behalf, and a guard holds every operation behind the gate to declaring it.
This commit is contained in:
1 parent
4f2af7c544
commit
c272324c1e
7 files changed
+1371
-689
No files matched your search
+1094
-657
File diff suppressed because it is too large.
Load diff
@@ -89,10 +89,12 @@ Every call is also a contract check. The status must be one the registry
|
||||
(`src/lib/openapi/routes/`) declares for that path and method, a JSON body
|
||||
must parse with the registered schema, and a key the schema does not declare
|
||||
fails the call: a client generated from the contract cannot see that field, so
|
||||
nobody promised to keep it. The one generic answer accepted without its own
|
||||
entry is the scope gate's 403 `auth.scope.insufficient`, which the shared 401
|
||||
documents for every authenticated operation. A test that calls an unregistered
|
||||
route on purpose passes `contract: { skip: "<reason>" }`.
|
||||
nobody promised to keep it. There is no generic answer the kit accepts on an
|
||||
operation's behalf: the scope gate's 403 `auth.scope.insufficient` is declared
|
||||
on every operation that passes the gate (`withScopeRefusal` in
|
||||
`src/lib/openapi/routes/shared.ts`), so it is checked like any other status. A
|
||||
test that calls an unregistered route on purpose passes
|
||||
`contract: { skip: "<reason>" }`.
|
||||
`tests/integration/routes/kit-contract.test.ts` proves the check can fail.
|
||||
|
||||
Assert what a person or a client would notice: the stored row, the response,
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
/**
|
||||
* The generic auth gate's 403, declared on every operation that passes it.
|
||||
*
|
||||
* `requireAuth` answers 403 `auth.scope.insufficient` to a narrow Bearer token
|
||||
* on a route that names no scope it holds, and `requireAdmin` answers 403
|
||||
* `auth.admin.required` to a session that is not an administrator's. Both run
|
||||
* before a route's own body, so every operation behind them can send the
|
||||
* status; `withScopeRefusal` declares it on the assembled table. These checks
|
||||
* hold the published document to that, and hold the helper to not trampling
|
||||
* the 403s operations already publish for reasons of their own.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { openApiPaths } from "../routes";
|
||||
import {
|
||||
adminRefusalResponse,
|
||||
errorEnvelope,
|
||||
scopeRefusalResponse,
|
||||
stdResponses,
|
||||
withScopeRefusal,
|
||||
} from "../routes/shared";
|
||||
|
||||
const METHODS = ["get", "put", "post", "patch", "delete"] as const;
|
||||
|
||||
interface Op {
|
||||
security?: unknown[];
|
||||
responses?: Record<string, { description?: string } | undefined>;
|
||||
}
|
||||
|
||||
function operations(): Array<{ path: string; method: string; op: Op }> {
|
||||
const out: Array<{ path: string; method: string; op: Op }> = [];
|
||||
for (const [path, item] of Object.entries(openApiPaths)) {
|
||||
for (const method of METHODS) {
|
||||
const op = (item as Record<string, Op | undefined>)[method];
|
||||
if (op?.responses) out.push({ path, method, op });
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
const URL_CREDENTIAL_OR_OWN_GATE = new Set([
|
||||
"/api/c/{token}/unlock",
|
||||
"/api/notifications/action/{token}",
|
||||
"/api/ingest/medication",
|
||||
]);
|
||||
|
||||
describe("the gate's 403 in the published contract", () => {
|
||||
it("is declared, naming its code, on every operation behind the gate", () => {
|
||||
const behind = operations().filter(
|
||||
({ path, op }) =>
|
||||
op.responses?.["401"] &&
|
||||
!(Array.isArray(op.security) && op.security.length === 0) &&
|
||||
!URL_CREDENTIAL_OR_OWN_GATE.has(path),
|
||||
);
|
||||
expect(behind.length).toBeGreaterThan(500);
|
||||
|
||||
const silent = behind.filter(({ path, op }) => {
|
||||
const code = path.startsWith("/api/admin/")
|
||||
? "auth.admin.required"
|
||||
: "auth.scope.insufficient";
|
||||
return !op.responses?.["403"]?.description?.includes(code);
|
||||
});
|
||||
expect(silent.map(({ path, method }) => `${method} ${path}`)).toEqual([]);
|
||||
});
|
||||
|
||||
it("is not declared on an operation that takes no credential", () => {
|
||||
const open = operations().filter(
|
||||
({ op }) => Array.isArray(op.security) && op.security.length === 0,
|
||||
);
|
||||
expect(open.length).toBeGreaterThan(0);
|
||||
const claimed = open.filter(({ op }) =>
|
||||
op.responses?.["403"]?.description?.includes("auth.scope.insufficient"),
|
||||
);
|
||||
expect(claimed.map(({ path, method }) => `${method} ${path}`)).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps an operation's own 403 and its wording", () => {
|
||||
const calendar = (
|
||||
openApiPaths["/api/cycle/calendar"] as unknown as {
|
||||
get: Op;
|
||||
}
|
||||
).get;
|
||||
const description = calendar.responses?.["403"]?.description ?? "";
|
||||
expect(description).toContain("cycle.disabled");
|
||||
expect(description).toContain("auth.scope.insufficient");
|
||||
});
|
||||
});
|
||||
|
||||
describe("withScopeRefusal", () => {
|
||||
const ownForbidden = {
|
||||
description: "Module off. `meta.errorCode` = `module.disabled`.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
};
|
||||
const table = withScopeRefusal({
|
||||
"/api/a": {
|
||||
get: { responses: { "200": {}, ...stdResponses } },
|
||||
post: { responses: { "200": {}, ...stdResponses, "403": ownForbidden } },
|
||||
},
|
||||
"/api/b": {
|
||||
get: { responses: { "200": {}, "403": ownForbidden, ...stdResponses } },
|
||||
},
|
||||
"/api/admin/x": { get: { responses: { "200": {}, ...stdResponses } } },
|
||||
"/api/open": {
|
||||
get: { security: [], responses: { "200": {}, ...stdResponses } },
|
||||
},
|
||||
"/api/no-auth-declared": { get: { responses: { "200": {} } } },
|
||||
}) as unknown as Record<string, Record<string, Op>>;
|
||||
|
||||
it("adds the shared response where an operation has none", () => {
|
||||
expect(table["/api/a"].get.responses?.["403"]).toBe(scopeRefusalResponse);
|
||||
expect(table["/api/admin/x"].get.responses?.["403"]).toBe(
|
||||
adminRefusalResponse,
|
||||
);
|
||||
});
|
||||
|
||||
it("prefixes an own 403, once per shared object, and leaves its text", () => {
|
||||
const a = table["/api/a"].post.responses?.["403"];
|
||||
const b = table["/api/b"].get.responses?.["403"];
|
||||
expect(a).toBe(b);
|
||||
expect(a?.description?.startsWith("`meta.errorCode` = `auth.scope")).toBe(
|
||||
true,
|
||||
);
|
||||
expect(a?.description?.endsWith(ownForbidden.description)).toBe(true);
|
||||
// The input object is not mutated.
|
||||
expect(ownForbidden.description).not.toContain("auth.scope");
|
||||
});
|
||||
|
||||
it("leaves an operation that takes no credential or declares no 401 alone", () => {
|
||||
expect(table["/api/open"].get.responses?.["403"]).toBeUndefined();
|
||||
expect(
|
||||
table["/api/no-auth-declared"].get.responses?.["403"],
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -96,9 +96,9 @@ import { settingsPaths } from "./settings";
|
||||
import { symptomPaths } from "./symptoms";
|
||||
import { syncPaths } from "./sync";
|
||||
import { workoutPaths } from "./workouts";
|
||||
import { coachPrefsSchema } from "./shared";
|
||||
import { coachPrefsSchema, withScopeRefusal } from "./shared";
|
||||
|
||||
export const openApiPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
const registeredPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
...cyclePaths,
|
||||
...metaPaths,
|
||||
...healthRecordPaths,
|
||||
@@ -214,6 +214,11 @@ export const openApiPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
...retiredPaths,
|
||||
};
|
||||
|
||||
// The generic auth gate's 403 is declared on the assembled table, not spread
|
||||
// from `stdResponses`; `scopeRefusalResponse` in `./shared` says why.
|
||||
export const openApiPaths: NonNullable<ZodOpenApiObject["paths"]> =
|
||||
withScopeRefusal(registeredPaths);
|
||||
|
||||
export const openApiComponents: NonNullable<ZodOpenApiObject["components"]> = {
|
||||
// Schemas listed here are forced into `components.schemas` even when no
|
||||
// route references them directly. `Medication` lives in this slot
|
||||
|
||||
@@ -351,6 +351,128 @@ export const stdResponses = {
|
||||
},
|
||||
};
|
||||
|
||||
// ── The generic 403 ──────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* The 403 the generic auth gates answer before a route's own body runs.
|
||||
*
|
||||
* `requireAuth` refuses a narrow Bearer token on a route that names no scope
|
||||
* the token holds (`auth.scope.insufficient`), and `requireAdmin` refuses a
|
||||
* session that is not an administrator's (`auth.admin.required`). The shared
|
||||
* 401 described both in its prose and no operation declared the status, so a
|
||||
* decoder generated from the contract had no typed branch for a refusal any
|
||||
* authenticated route can send.
|
||||
*
|
||||
* Neither is a member of `stdResponses`, on purpose. That set is spread into
|
||||
* several hundred operations, about three hundred of them declare a 403 of
|
||||
* their own, and most spread the set AFTER their own entries — a shared 403
|
||||
* in it would silently overwrite every module-off and sharing refusal in the
|
||||
* document. `withScopeRefusal` adds them to the assembled table instead, where
|
||||
* the order of a spread cannot decide it.
|
||||
*/
|
||||
export const scopeRefusalResponse = {
|
||||
description:
|
||||
"The credential is valid but its scope does not reach this operation. `meta.errorCode` = `auth.scope.insufficient`: a narrow Bearer token on a route that names no scope it holds, or a connector-bound token on a write. Retrying with the same token will not help; use a session, or a token minted with the scope the route names.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
};
|
||||
|
||||
/**
|
||||
* The admin surfaces' variant. `requireAdmin` reads only the session cookie,
|
||||
* so a Bearer caller never gets this far (it is a 401 there); the 403 is a
|
||||
* signed-in account that is not an administrator.
|
||||
*/
|
||||
export const adminRefusalResponse = {
|
||||
description:
|
||||
"The session is valid but its account is not an administrator. `meta.errorCode` = `auth.admin.required`.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
};
|
||||
|
||||
const GATE_SENTENCE = {
|
||||
scope:
|
||||
"`meta.errorCode` = `auth.scope.insufficient` when a Bearer token's scope does not reach this route; the generic auth gate answers it before the route's own checks run. Otherwise: ",
|
||||
admin:
|
||||
"`meta.errorCode` = `auth.admin.required` when the session's account is not an administrator; the admin gate answers it before the route's own checks run. Otherwise: ",
|
||||
} as const;
|
||||
|
||||
const GATE_CODE = {
|
||||
scope: "auth.scope.insufficient",
|
||||
admin: "auth.admin.required",
|
||||
} as const;
|
||||
|
||||
/**
|
||||
* Surfaces that authenticate by a credential of their own and never pass the
|
||||
* generic gate, so they cannot answer its 403: a URL credential (the
|
||||
* clinician share link, the notification quick answer) or the external ingest
|
||||
* token, which runs its own scope check and documents its own 403.
|
||||
*/
|
||||
const OUTSIDE_THE_GENERIC_GATE = new Set([
|
||||
"/api/c/{token}/unlock",
|
||||
"/api/notifications/action/{token}",
|
||||
"/api/ingest/medication",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Declare the gate's 403 on every operation that passes the generic auth
|
||||
* gate: one that declares a 401 and does not opt out of authentication with
|
||||
* `security: []`. Under `/api/admin/` the gate is `requireAdmin`, everywhere
|
||||
* else `requireAuth`. An operation without a 403 of its own gets the shared
|
||||
* response; one with its own keeps it, and its description is prefixed with
|
||||
* the gate's code, because a client branching on that 403 has to tell a
|
||||
* module-off refusal from a gate refusal. Prefixed, not appended: the sharing
|
||||
* refusal has to stay the description's closing sentence, byte for byte
|
||||
* (`openapi-sharing-denial.test.ts`).
|
||||
*
|
||||
* Response objects shared between operations stay shared (one augmented copy
|
||||
* per original), so the emitted document keeps its anchors.
|
||||
*/
|
||||
export function withScopeRefusal<T extends Record<string, unknown>>(
|
||||
paths: T,
|
||||
): T {
|
||||
const augmented = {
|
||||
scope: new WeakMap<object, object>(),
|
||||
admin: new WeakMap<object, object>(),
|
||||
};
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [path, item] of Object.entries(paths)) {
|
||||
if (!isPlainRecord(item) || OUTSIDE_THE_GENERIC_GATE.has(path)) {
|
||||
out[path] = item;
|
||||
continue;
|
||||
}
|
||||
const gate = path.startsWith("/api/admin/") ? "admin" : "scope";
|
||||
const nextItem: Record<string, unknown> = { ...item };
|
||||
for (const method of ["get", "put", "post", "patch", "delete"] as const) {
|
||||
const op = item[method];
|
||||
if (!isPlainRecord(op) || !isPlainRecord(op.responses)) continue;
|
||||
if (Array.isArray(op.security) && op.security.length === 0) continue;
|
||||
if (!op.responses["401"]) continue;
|
||||
const own = op.responses["403"];
|
||||
let forbidden: unknown =
|
||||
gate === "admin" ? adminRefusalResponse : scopeRefusalResponse;
|
||||
if (isPlainRecord(own)) {
|
||||
forbidden = augmented[gate].get(own);
|
||||
if (!forbidden) {
|
||||
const description =
|
||||
typeof own.description === "string" ? own.description : "";
|
||||
forbidden = description.includes(GATE_CODE[gate])
|
||||
? own
|
||||
: { ...own, description: GATE_SENTENCE[gate] + description };
|
||||
augmented[gate].set(own, forbidden as object);
|
||||
}
|
||||
}
|
||||
nextItem[method] = {
|
||||
...op,
|
||||
responses: { ...op.responses, "403": forbidden },
|
||||
};
|
||||
}
|
||||
out[path] = nextItem;
|
||||
}
|
||||
return out as T;
|
||||
}
|
||||
|
||||
function isPlainRecord(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* The 429 the shared single-record write bucket answers with.
|
||||
*
|
||||
|
||||
@@ -76,15 +76,17 @@ describe("callRoute's contract check", () => {
|
||||
).rejects.toThrow(/answered 409, which the contract does not declare/);
|
||||
});
|
||||
|
||||
it("refuses a 403 that is not the generic scope refusal", async () => {
|
||||
it("refuses the scope gate's 403 on an operation that declares none", async () => {
|
||||
// `/api/version` takes no credential, so no 403 is declared on it; the
|
||||
// generic refusal is accepted only where the contract says so.
|
||||
await expect(
|
||||
callRoute(
|
||||
answering(403, {
|
||||
data: null,
|
||||
error: "nope",
|
||||
meta: { errorCode: "http.forbidden" },
|
||||
error: "Insufficient scope",
|
||||
meta: { errorCode: "auth.scope.insufficient" },
|
||||
}),
|
||||
{ method: "GET", route: "/api/tokens", auth: anonymous },
|
||||
{ method: "GET", route: "/api/version", auth: anonymous },
|
||||
),
|
||||
).rejects.toThrow(/answered 403/);
|
||||
});
|
||||
|
||||
+2
-22
@@ -207,20 +207,10 @@ interface ResponseObject {
|
||||
$ref?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* The generic scope gate answers 403 `auth.scope.insufficient` on any route a
|
||||
* narrow Bearer token does not reach. The registry documents that refusal in
|
||||
* the prose of the shared 401 (`stdResponses`) rather than as a 403 entry on
|
||||
* every operation, so an operation that declares the shared 401 is taken to
|
||||
* declare this 403 as well, and nothing else is.
|
||||
*/
|
||||
const SCOPE_REFUSAL_CODE = "auth.scope.insufficient";
|
||||
|
||||
function lookupResponse(
|
||||
method: HttpMethod,
|
||||
route: string,
|
||||
status: number,
|
||||
body: unknown,
|
||||
): ResponseObject {
|
||||
const pathItem = (openApiPaths as Record<string, unknown>)[route];
|
||||
if (!isRecord(pathItem)) {
|
||||
@@ -235,17 +225,7 @@ function lookupResponse(
|
||||
);
|
||||
}
|
||||
const responses = isRecord(operation.responses) ? operation.responses : {};
|
||||
let declared = responses[String(status)] ?? responses.default;
|
||||
if (
|
||||
!isRecord(declared) &&
|
||||
status === 403 &&
|
||||
isRecord(responses["401"]) &&
|
||||
isRecord(body) &&
|
||||
isRecord(body.meta) &&
|
||||
body.meta.errorCode === SCOPE_REFUSAL_CODE
|
||||
) {
|
||||
declared = responses["401"];
|
||||
}
|
||||
const declared = responses[String(status)] ?? responses.default;
|
||||
if (!isRecord(declared)) {
|
||||
throw new ContractViolation(
|
||||
`${method} ${route} answered ${status}, which the contract does not declare (declared: ${Object.keys(responses).join(", ")}).`,
|
||||
@@ -274,7 +254,7 @@ function checkContract(
|
||||
body: unknown,
|
||||
isJson: boolean,
|
||||
): void {
|
||||
const declared = lookupResponse(method, route, status, body);
|
||||
const declared = lookupResponse(method, route, status);
|
||||
const content = declared.content ?? {};
|
||||
const mediaTypes = Object.keys(content);
|
||||
if (!isJson) {
|
||||
|
||||
Reference in new issue
Block a user