fix(auth): ask for a fresh proof before the record or a credential to it leaves

Exporting the whole record, making a clinician share link, minting a token,
and the admin backup download, upload and restore, data wipe and password
reset ran on a live session alone. The plaintext full backup did, while the
encrypted one beside it asked for a second factor. Whoever held a stolen
session could take the record, or leave behind a credential that outlives
the session.

These now go through one gate, requireRecentProof (assertRecentCookieProof
for the routes that resolve their caller themselves). A browser session
passes after a sign-in or re-proof within five minutes: a second factor on
an account that has one, otherwise the password or a passkey. The refusal is
401 auth.reproof.required with meta.methods, and the web opens the re-proof
dialog, posts the proof to the new POST /api/auth/reproof and retries. A
password proof stamps the new Session.reproofAt (migration 0359), never
mfaVerifiedAt, so it cannot stand in for a second factor anywhere.

On Bearer, the full backup asks for an X-Step-Up elevation; the encrypted
export asks for a second-factor elevation on an account that has one. Share
links and connector tokens keep taking the token alone, because the shipped
app calls them without an elevation and reads a 401 there as a dead
session; the guard test names them.

Key rotation keeps its stricter second-factor gate.
This commit is contained in:
Marc-André Bombeck committed 2026-09-26 19:54:38 +02:00
1 parent a7f66b8519
commit 0fc83cb0a1
56 files changed
+2028 -235

No files matched your search

@@ -46,6 +46,7 @@ const EXPECTED = [
"api/auth/passkey/login-verify/route.ts",
"api/auth/passkey/register-options/route.ts",
"api/auth/password/route.ts",
"api/auth/reproof/route.ts",
"api/auth/step-up/route.ts",
];
@@ -0,0 +1,156 @@
/**
* Structural guard on the recent-proof gate.
*
* Exporting the whole record, making a clinician share link, minting a token,
* and the admin backup, restore, wipe and reset actions ask for a fresh proof,
* not only a live session (`requireRecentProof` / `assertRecentCookieProof` in
* `src/lib/api-handler.ts`). This file pins which routes those are, so a route
* cannot drop the gate, and a new one cannot quietly join the list of routes
* whose Bearer arm still takes the token alone.
*
* Matching is on the IMPORT and then on calls of the local name, the way
* `step-up-elevation-guard.test.ts` does it, so an aliased import is followed.
* Every sweep asserts it matched something: a matcher that finds nothing agrees
* with every list.
*/
import { describe, it, expect } from "vitest";
import { readFileSync } from "node:fs";
import { join } from "node:path";
import { walkSourceFiles } from "./helpers/source-files";
const SRC = join(process.cwd(), "src");
function read(rel: string): string {
return readFileSync(join(SRC, rel), "utf8");
}
function sourceFiles(): string[] {
return walkSourceFiles(SRC, { floor: 3000 })
.filter((p) => !p.startsWith("generated/"))
.filter((p) => !p.includes("__tests__"))
.filter((p) => !p.endsWith(".test.ts") && !p.endsWith(".test.tsx"))
.sort();
}
function importedAs(rel: string, exportName: string): string[] {
const names: string[] = [];
for (const m of read(rel).matchAll(
/import\s*\{([\s\S]*?)\}\s*from\s*["']([^"']+)["']/g,
)) {
if (!/api-handler$/.test(m[2])) continue;
for (const part of m[1].split(",")) {
const piece = part.trim().replace(/^type\s+/, "");
const aliased = piece.match(
/^([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)$/,
);
if (aliased && aliased[1] === exportName) names.push(aliased[2]);
else if (piece === exportName) names.push(piece);
}
}
return names;
}
/** Every call of `exportName` in `rel`, as the source text of its argument. */
function callArgs(rel: string, exportName: string): string[] {
const src = read(rel);
const args: string[] = [];
for (const local of importedAs(rel, exportName)) {
for (const m of src.matchAll(
new RegExp(`\\b${local}\\s*\\(([^)]*)\\)`, "g"),
)) {
args.push(m[1].replace(/\s+/g, " ").trim());
}
}
return args;
}
function callers(exportName: string): string[] {
return sourceFiles().filter(
(rel) => rel !== "lib/api-handler.ts" && callArgs(rel, exportName).length,
);
}
/** Routes that resolve their own caller and take the recent-proof gate. */
const RECENT_PROOF_ROUTES: Record<string, string> = {
"app/api/export/full-backup/route.ts": "elevation",
"app/api/export/encrypted/route.ts": "elevation-if-enrolled",
// The shipped iOS app calls these two on its token without an elevation and
// reads a 401 outside its known paths as a dead session. Tightening them is
// an app release first; see the iOS note for v1.39.3.
"app/api/share-links/route.ts": "token",
"app/api/mcp/tokens/route.ts": "token",
};
/** Cookie-only routes that add the gate after their own resolver. */
const COOKIE_PROOF_ROUTES = [
"app/api/admin/backups/[id]/download/route.ts",
"app/api/admin/backups/[id]/restore/route.ts",
"app/api/admin/backups/upload/route.ts",
"app/api/admin/data/route.ts",
"app/api/admin/users/[id]/reset-password/route.ts",
"app/api/auth/reproof/route.ts",
"app/api/tokens/documents/route.ts",
"app/api/tokens/measurements/route.ts",
].sort();
describe("the recent-proof gate", () => {
it("reads the tree it claims to sweep", () => {
expect(sourceFiles().length).toBeGreaterThan(1500);
});
it("is taken by exactly the known routes, each with its Bearer rule", () => {
const found: Record<string, string> = {};
for (const rel of callers("requireRecentProof")) {
const args = callArgs(rel, "requireRecentProof");
expect(args).toHaveLength(1);
const rule = args[0].match(/bearer:\s*"([^"]+)"/)?.[1];
expect(rule, rel).toBeDefined();
found[rel] = rule!;
}
expect(Object.keys(found).length).toBeGreaterThan(0);
expect(found).toEqual(RECENT_PROOF_ROUTES);
});
it("the cookie arm is added by exactly the known cookie-only routes", () => {
const found = callers("assertRecentCookieProof");
expect(found.length).toBeGreaterThan(0);
expect(found).toEqual(COOKIE_PROOF_ROUTES);
});
it("each cookie-only route resolves a cookie before it asks", () => {
for (const rel of COOKIE_PROOF_ROUTES) {
const resolvers = [
...importedAs(rel, "requireAdmin"),
...importedAs(rel, "requireCookieAuth"),
];
expect(resolvers, rel).toHaveLength(1);
expect(importedAs(rel, "requireAuth"), rel).toEqual([]);
}
});
it("every route whose Bearer arm takes an elevation spends it", () => {
const withElevation = Object.entries(RECENT_PROOF_ROUTES).filter(
([, rule]) => rule !== "token",
);
expect(withElevation.length).toBeGreaterThan(0);
for (const [rel] of withElevation) {
expect(read(rel), rel).toMatch(/\.commitElevation\(\)/);
}
});
it("key rotation keeps its stricter second-factor gate", () => {
const rel = "app/api/admin/encryption/rotate/route.ts";
expect(callArgs(rel, "requireFreshMfa").length).toBeGreaterThan(0);
});
it("the gate reads the session's own stamps, and only a second factor on an enrolled account", () => {
const src = read("lib/api-handler.ts");
const start = src.indexOf("export async function assertRecentCookieProof(");
expect(start).toBeGreaterThan(-1);
const body = src.slice(start, src.indexOf("\nexport ", start + 1));
// The enrolled arm names mfaVerifiedAt alone: a password re-proof
// (`reproofAt`) or a bare sign-in (`createdAt`) must not stand in for it.
expect(body).toMatch(/enrolled\s*\?\s*fresh\(row\?\.mfaVerifiedAt\)\s*:/);
});
});
@@ -118,11 +118,14 @@ function callers(exportName: string, moduleRe: RegExp): string[] {
* consult this mechanism at all;
* - the trusted-device routes. "Remember this browser" is a browser concept
* with no native equivalent to manage;
* - `requireFreshMfaIfEnrolled`'s callers (password change, encrypted
* export, encryption-key rotation, the sharing invite). Those are
* destructive account actions, not second-factor management, and letting an
* elevation satisfy them would be exactly the silent widening this file
* exists to prevent.
* - `requireFreshMfaIfEnrolled`'s callers (password change, encryption-key
* rotation, the sharing invite). Those are destructive account actions,
* not second-factor management, and letting an elevation satisfy them
* would be exactly the silent widening this file exists to prevent.
*
* The two whole-record exports reach the elevation through a third gate,
* `requireRecentProof`, whose route list and per-route Bearer rule are frozen
* in `recent-proof-surface-guard.test.ts`.
*
* Account deletion and the record wipe used to sit in that last bullet. They
* left it deliberately, and through a different gate: see `ERASURE_ROUTES`