mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 05:42:19 +00:00
Account deletion and the record wipe both demand a fresh second-factor proof once a factor is enrolled, and that proof lived on the session row. A native client has no session row, so an account with TOTP or a security key could not delete itself from the app on any request it could build: requireFreshMfaIfEnrolled fell through to requireFreshMfa, which resolves via getSession and answers 401 to a perfectly valid token. App Store guideline 5.1.1(v) asks for exactly that deletion. Add requireFreshMfaOrElevationIfEnrolled beside the cookie-only sibling rather than softening it. Its cookie arm is the sibling's, unchanged; its Bearer arm takes the single-use elevation the second-factor management routes already accept, with the fresh-factor rule pinned on, so a password-proved elevation cannot erase an account any more than a password login satisfies the gate on the web. No factor rides the request body: one mint surface, one redemption path. Scope stays fail-closed, so a narrow token is refused before the header is read. The elevation is spent immediately before the erasure, after the typed confirmation and the last-admin check, so a rejected request does not burn a proof against a 5-per-15-minute mint ceiling. The Bearer branch of requireMfaManagementAuth moves into a shared helper so both gates read and claim in the same place. The elevation guard grows a section naming the two routes, because the set of surfaces an elevation can unlock only stays small if adding to it is a visible edit.
380 lines
15 KiB
TypeScript
380 lines
15 KiB
TypeScript
/**
|
|
* Structural guards on the step-up elevation boundary.
|
|
*
|
|
* An elevation lets a Bearer token reach a surface that is otherwise
|
|
* cookie-only. That is a deliberate, narrow widening, and its entire safety
|
|
* argument rests on the set of routes it can unlock being small, known, and
|
|
* unable to grow by accident. Behavioural tests prove the mechanism works;
|
|
* these prove nobody quietly pointed it somewhere else.
|
|
*
|
|
* MATCHING IS ON THE IMPORT, NOT THE CALL. An earlier revision grepped for
|
|
* `requireMfaManagementAuth(` and was evadable in one line:
|
|
*
|
|
* import { requireMfaManagementAuth as gate } from "@/lib/api-handler";
|
|
* const auth = await gate();
|
|
*
|
|
* A call-shaped regex sees nothing there. An import-shaped one cannot be dodged
|
|
* that way, because the binding has to be imported from the module under some
|
|
* local name before it can be called at all. The helpers below parse the import
|
|
* specifier — including its `as` alias — and check calls against the LOCAL name,
|
|
* so a rename is followed rather than lost.
|
|
*
|
|
* They remain tripwires, not proofs. They cannot show an allowlist is correct,
|
|
* only that it has not changed without someone editing this file.
|
|
*/
|
|
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 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 read(rel: string): string {
|
|
return readFileSync(join(SRC, rel), "utf8");
|
|
}
|
|
|
|
/** Local names bound for `exportName` from any module matching `moduleRe`. */
|
|
function importedAs(
|
|
rel: string,
|
|
exportName: string,
|
|
moduleRe: RegExp,
|
|
): string[] {
|
|
const src = read(rel);
|
|
const names: string[] = [];
|
|
for (const m of src.matchAll(
|
|
/import\s*\{([\s\S]*?)\}\s*from\s*["']([^"']+)["']/g,
|
|
)) {
|
|
const [, clause, specifier] = m;
|
|
if (!moduleRe.test(specifier)) continue;
|
|
for (const part of clause.split(",")) {
|
|
const piece = part.trim().replace(/^type\s+/, "");
|
|
if (!piece) continue;
|
|
const aliased = piece.match(
|
|
/^([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)$/,
|
|
);
|
|
if (aliased) {
|
|
if (aliased[1] === exportName) names.push(aliased[2]);
|
|
} else if (piece === exportName) {
|
|
names.push(piece);
|
|
}
|
|
}
|
|
}
|
|
return names;
|
|
}
|
|
|
|
const API_HANDLER = /api-handler$/;
|
|
const STEP_UP_MODULE = /auth\/step-up$/;
|
|
|
|
/** Files that import `exportName` from the module AND call it. */
|
|
function callers(exportName: string, moduleRe: RegExp): string[] {
|
|
return sourceFiles()
|
|
.filter((rel) => {
|
|
const locals = importedAs(rel, exportName, moduleRe);
|
|
if (locals.length === 0) return false;
|
|
const src = read(rel);
|
|
return locals.some((name) => new RegExp(`\\b${name}\\s*\\(`).test(src));
|
|
})
|
|
.sort();
|
|
}
|
|
|
|
/**
|
|
* Every route an elevation can unlock: the second-factor management MUTATIONS,
|
|
* plus the one credential removal that is not a second factor and belongs here
|
|
* anyway.
|
|
*
|
|
* That one is `DELETE /api/auth/passkeys/{id}`. A passkey is the PRIMARY
|
|
* sign-in credential, and it sat behind a plain session while the second-factor
|
|
* key beside it demanded a fresh proof — the softer gate on the more valuable
|
|
* credential. It joins the set on the same `freshFactor: true` as the rest,
|
|
* differing only in `proofSource: "any-possession"` — which widens the
|
|
* REACHABILITY pre-check to count a primary passkey and does not soften the gate
|
|
* itself. A passkey-only account clears it by re-proving that passkey: on Bearer
|
|
* through the mint's `passkey` method, on the web through a fresh sign-in, which
|
|
* is what stamps `mfaVerifiedAt`.
|
|
*
|
|
* The widening that admits is real and bounded: an elevation minted against a
|
|
* re-proved factor can now also remove a passkey. That is the same class of act
|
|
* as the rest of the list — a credential the caller owns, removed after they
|
|
* prove they are the caller — and it is written down here rather than inferred,
|
|
* which is the whole point of this file.
|
|
*
|
|
* What is deliberately ABSENT is as much the point as what is present:
|
|
* - `GET /api/auth/me/mfa`. The status read is plain `requireAuth()`: it
|
|
* carries no credential material (no secret, no code, no public key, no
|
|
* credential id — only names, dates, and a count), the web shows the same
|
|
* screen to any authenticated session, and `/api/auth/me` already exposes
|
|
* `totpConfirmedAt` over Bearer. Gating it would force a factor re-proof to
|
|
* render a screen and buy nothing;
|
|
* - every `/api/admin/*` route. `requireAdmin` is cookie-only and does not
|
|
* 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.
|
|
*
|
|
* 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`
|
|
* below. The widening is written down there, which is what this file is for.
|
|
*/
|
|
const ELEVATION_ROUTES = [
|
|
"app/api/auth/passkeys/[id]/route.ts",
|
|
"app/api/auth/me/mfa/disable/route.ts",
|
|
"app/api/auth/me/mfa/recovery-codes/regenerate/route.ts",
|
|
"app/api/auth/me/mfa/totp/confirm/route.ts",
|
|
"app/api/auth/me/mfa/totp/setup/route.ts",
|
|
"app/api/auth/me/mfa/webauthn/[id]/route.ts",
|
|
"app/api/auth/me/mfa/webauthn/register/options/route.ts",
|
|
"app/api/auth/me/mfa/webauthn/register/verify/route.ts",
|
|
].sort();
|
|
|
|
/**
|
|
* The two erasure routes that accept a FRESH-FACTOR elevation over Bearer.
|
|
*
|
|
* App Store guideline 5.1.1(v) requires that an account created in the app can
|
|
* be deleted from the app. Both routes gated an MFA-enrolled account on a
|
|
* cookie-side fresh factor and a Bearer transport had no way to present one,
|
|
* so a native-app user with a second factor could not delete their account at
|
|
* all. They now resolve through `requireFreshMfaOrElevationIfEnrolled`, the
|
|
* sibling of `requireFreshMfaIfEnrolled` whose cookie arm is unchanged and
|
|
* whose Bearer arm runs the same elevation validation the MFA-management
|
|
* routes use, with `freshFactor` pinned on — so a password-proved elevation
|
|
* cannot erase an account, exactly as a password login cannot on the web.
|
|
*
|
|
* The set is two routes and must stay two. Password change, the encrypted
|
|
* export, key rotation and the sharing invite keep the cookie-only sibling;
|
|
* moving one of them here is a visible edit to this list, not a drive-by.
|
|
*/
|
|
const ERASURE_ROUTES = [
|
|
"app/api/settings/account/route.ts",
|
|
"app/api/settings/data/route.ts",
|
|
].sort();
|
|
|
|
describe("S1 — the elevation-accepting route set is frozen", () => {
|
|
// A sweep that finds nothing agrees with every allowlist, so the size of
|
|
// the tree being read is asserted before anything is concluded from it.
|
|
// Pinned below the real source-file count with headroom, not at one.
|
|
it("reads the tree it claims to sweep", () => {
|
|
expect(sourceFiles().length).toBeGreaterThan(1500);
|
|
});
|
|
|
|
it("only the known MFA-management mutations call the gate", () => {
|
|
const found = callers("requireMfaManagementAuth", API_HANDLER).filter(
|
|
(rel) => rel !== "lib/api-handler.ts",
|
|
);
|
|
expect(found).toEqual(ELEVATION_ROUTES);
|
|
});
|
|
|
|
it("follows an aliased import rather than losing it", () => {
|
|
// Guards the guard. Without this the allowlist above is one rename away
|
|
// from meaningless, which is precisely how the previous revision was
|
|
// evadable.
|
|
const probe = `
|
|
import { requireMfaManagementAuth as gate } from "@/lib/api-handler";
|
|
const auth = await gate();
|
|
`;
|
|
const names: string[] = [];
|
|
for (const m of probe.matchAll(
|
|
/import\s*\{([\s\S]*?)\}\s*from\s*["']([^"']+)["']/g,
|
|
)) {
|
|
for (const part of m[1].split(",")) {
|
|
const aliased = part
|
|
.trim()
|
|
.match(/^([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)$/);
|
|
if (aliased && aliased[1] === "requireMfaManagementAuth") {
|
|
names.push(aliased[2]);
|
|
}
|
|
}
|
|
}
|
|
expect(names).toEqual(["gate"]);
|
|
expect(new RegExp(`\\b${names[0]}\\s*\\(`).test(probe)).toBe(true);
|
|
});
|
|
|
|
it("no admin route calls the gate", () => {
|
|
const admin = callers("requireMfaManagementAuth", API_HANDLER).filter(
|
|
(rel) => rel.startsWith("app/api/admin/"),
|
|
);
|
|
expect(admin).toEqual([]);
|
|
});
|
|
|
|
it("the old cookie-only gates no longer linger on the MFA surface", () => {
|
|
for (const rel of ELEVATION_ROUTES) {
|
|
expect(importedAs(rel, "requireCookieAuth", API_HANDLER)).toEqual([]);
|
|
expect(importedAs(rel, "requireFreshMfa", API_HANDLER)).toEqual([]);
|
|
}
|
|
});
|
|
|
|
it("every gated route spends its elevation", () => {
|
|
// `commitElevation` is what makes the proof single-use. A route that gates
|
|
// but never commits would leave the elevation redeemable for the rest of
|
|
// its window — the one fail-open the two-phase split introduces, closed
|
|
// here.
|
|
for (const rel of ELEVATION_ROUTES) {
|
|
expect(read(rel)).toMatch(/\.commitElevation\(\)/);
|
|
}
|
|
});
|
|
});
|
|
|
|
describe("S6 — the erasure routes accept a fresh-factor elevation", () => {
|
|
it("only the two erasure routes call the erasure gate", () => {
|
|
const found = callers(
|
|
"requireFreshMfaOrElevationIfEnrolled",
|
|
API_HANDLER,
|
|
).filter((rel) => rel !== "lib/api-handler.ts");
|
|
expect(found).toEqual(ERASURE_ROUTES);
|
|
});
|
|
|
|
it("neither erasure route still carries the cookie-only sibling", () => {
|
|
for (const rel of ERASURE_ROUTES) {
|
|
expect(importedAs(rel, "requireFreshMfaIfEnrolled", API_HANDLER)).toEqual(
|
|
[],
|
|
);
|
|
expect(importedAs(rel, "requireFreshMfa", API_HANDLER)).toEqual([]);
|
|
}
|
|
});
|
|
|
|
it("every erasure route spends its elevation", () => {
|
|
for (const rel of ERASURE_ROUTES) {
|
|
expect(read(rel)).toMatch(/\.commitElevation\(\)/);
|
|
}
|
|
});
|
|
|
|
it("the erasure gate pins the fresh-factor arm and never reads a body factor", () => {
|
|
// A password-proved elevation must not erase an account, and the gate
|
|
// must not grow a second, inline factor check beside the elevation: one
|
|
// mint surface, one redemption path.
|
|
const src = read("lib/api-handler.ts");
|
|
const start = src.indexOf(
|
|
"export async function requireFreshMfaOrElevationIfEnrolled(",
|
|
);
|
|
expect(start).toBeGreaterThan(-1);
|
|
const body = src.slice(start, src.indexOf("\nexport ", start + 1));
|
|
expect(body).toMatch(/freshFactor:\s*true/);
|
|
expect(body).not.toMatch(/verifyMfaFactor|verifyTotp|recoveryCode/);
|
|
});
|
|
});
|
|
|
|
describe("S2 — mint and redeem happen in exactly one place each", () => {
|
|
it("only the gate claims an elevation", () => {
|
|
const found = callers("claimStepUpElevation", STEP_UP_MODULE).filter(
|
|
(rel) => rel !== "lib/auth/step-up.ts",
|
|
);
|
|
expect(found).toEqual(["lib/api-handler.ts"]);
|
|
});
|
|
|
|
it("only the gate validates one", () => {
|
|
const found = callers("validateStepUpElevation", STEP_UP_MODULE).filter(
|
|
(rel) => rel !== "lib/auth/step-up.ts",
|
|
);
|
|
expect(found).toEqual(["lib/api-handler.ts"]);
|
|
});
|
|
|
|
it("only the mint route mints one", () => {
|
|
const found = callers("mintStepUpElevation", STEP_UP_MODULE).filter(
|
|
(rel) => rel !== "lib/auth/step-up.ts",
|
|
);
|
|
expect(found).toEqual(["app/api/auth/step-up/route.ts"]);
|
|
});
|
|
|
|
it("no file outside the gate reads the elevation header", () => {
|
|
// Both spellings — the literal string AND the exported constant — across
|
|
// every source file, not only `route.ts`. A helper could read it too.
|
|
const offenders = sourceFiles()
|
|
.filter((rel) => rel !== "lib/api-handler.ts")
|
|
.filter((rel) => {
|
|
const src = read(rel);
|
|
if (/["']x-step-up["']/i.test(src)) return true;
|
|
return (
|
|
importedAs(rel, "STEP_UP_ELEVATION_HEADER", API_HANDLER).length > 0
|
|
);
|
|
});
|
|
expect(offenders).toEqual([]);
|
|
});
|
|
});
|
|
|
|
describe("S3 — requireAdmin stays cookie-only", () => {
|
|
it("resolves through getSession and nothing else", () => {
|
|
const src = read("lib/api-handler.ts");
|
|
const body = src.slice(
|
|
src.indexOf("export async function requireAdmin("),
|
|
src.indexOf("export async function requireCookieAuth("),
|
|
);
|
|
expect(body).toContain("await getSession()");
|
|
expect(body).not.toMatch(/authenticateBearer|resolveBearerToken/);
|
|
expect(body).not.toMatch(
|
|
/claimStepUpElevation|validateStepUpElevation|STEP_UP_ELEVATION_HEADER|requireMfaManagementAuth/,
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("S4 — the mint surface refuses a cookie", () => {
|
|
it("both step-up routes resolve via requireBearerAuth only", () => {
|
|
for (const rel of [
|
|
"app/api/auth/step-up/route.ts",
|
|
"app/api/auth/step-up/options/route.ts",
|
|
]) {
|
|
expect(importedAs(rel, "requireBearerAuth", API_HANDLER)).toEqual([
|
|
"requireBearerAuth",
|
|
]);
|
|
expect(importedAs(rel, "requireAuth", API_HANDLER)).toEqual([]);
|
|
expect(importedAs(rel, "requireCookieAuth", API_HANDLER)).toEqual([]);
|
|
}
|
|
});
|
|
|
|
it("requireBearerAuth never consults the session cookie", () => {
|
|
const src = read("lib/api-handler.ts");
|
|
const body = src.slice(
|
|
src.indexOf("export async function requireBearerAuth("),
|
|
src.indexOf("export const STEP_UP_ELEVATION_HEADER"),
|
|
);
|
|
expect(body).not.toContain("getSession(");
|
|
});
|
|
});
|
|
|
|
describe("S5 — a password proof cannot satisfy a fresh-factor gate", () => {
|
|
it("the fresh-factor set excludes password", async () => {
|
|
// The B1 invariant, pinned at the definition. The web stamps
|
|
// `mfaVerifiedAt` only for a completed second factor or a primary passkey
|
|
// login; the Bearer arm must not be looser.
|
|
const { FRESH_FACTOR_METHODS, isFreshFactorMethod } =
|
|
await import("@/lib/auth/step-up");
|
|
expect([...FRESH_FACTOR_METHODS].sort()).toEqual([
|
|
"passkey",
|
|
"totp",
|
|
"webauthn",
|
|
]);
|
|
expect(isFreshFactorMethod("password")).toBe(false);
|
|
});
|
|
|
|
it("the destructive routes all request the fresh-factor arm", () => {
|
|
// Miss this on one route and a password-proved elevation tears down the
|
|
// second factor there.
|
|
for (const rel of [
|
|
"app/api/auth/me/mfa/disable/route.ts",
|
|
"app/api/auth/me/mfa/recovery-codes/regenerate/route.ts",
|
|
"app/api/auth/me/mfa/webauthn/[id]/route.ts",
|
|
]) {
|
|
expect(read(rel)).toMatch(/freshFactor:\s*true/);
|
|
}
|
|
});
|
|
|
|
it("the claim re-checks the factor rather than trusting the validation", () => {
|
|
const src = read("lib/auth/step-up.ts");
|
|
const claim = src.slice(
|
|
src.indexOf("export async function claimStepUpElevation("),
|
|
);
|
|
expect(claim).toMatch(/requireFreshFactor\s*&&\s*!isFreshFactorMethod/);
|
|
});
|
|
});
|