From 99f0607a6989ad385af167a96173e005d509e5d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 17 Oct 2026 09:17:03 +0200 Subject: [PATCH 1/6] fix(auth): let every fresh-factor refusal open the re-proof dialog A Guardian signed in through single sign-on with a second factor enrolled could not hand a managed profile over once five minutes had passed since sign-in: the handover answered 401 auth.stepup.required, and the panel printed "Confirm your second factor, then try again" with no way to do so. The same held for every action behind requireFreshMfa: creating, editing and ending a managed profile, adding and removing a Guardian, disabling the second factor, rotating recovery codes, removing a security key or a passkey, changing the password, the data wipe, account deletion and the key rotation. The recent-proof routes had the dialog; these had text. - requireFreshMfa names the proofs that clear a stale stamp in meta.methods (the possession proofs the account holds, never a password, which stamps reproofAt only). - The re-proof flow treats that refusal like auth.reproof.required: open the dialog, post the proof to /api/auth/reproof, send the request once more. The flow moves into a React-free module so it can be driven in tests; the hook binds it to the dialog. - The managed-profile mutation hooks take a required proof guard, and every caller passes recentProof.run. The other surfaces wrap their requests in recentProof.run; disabling the second factor asks before its confirmation opens, so the code typed there is not spent twice. - An account with nothing to re-prove in place (single sign-on only, no factor) is told to sign in again when adding a factor, instead of being offered a password it does not have. - step-up-web-caller-guard freezes the gated route methods and fails when a web request to one of them runs outside the flow. An account whose only second factor is the identity provider's is still asked for an app factor on the managed-profile acts: the app cannot see the provider's factor. Enrolling right after a sign-in needs no further proof, and the confirmation stamps the session, so the act follows. Refs #959 --- docs/api/openapi.yaml | 15 +- messages/de.json | 1 + messages/en.json | 1 + messages/es.json | 1 + messages/fr.json | 1 + messages/it.json | 1 + messages/ko.json | 1 + messages/pl.json | 1 + .../step-up-web-caller-guard.test.ts | 383 ++++++++++++++ src/app/api/auth/reproof/route.ts | 6 +- src/components/admin/encryption-section.tsx | 30 +- .../managed-profile-step-up-flow.test.ts | 339 ++++++++++++ .../settings/access/grant-action-error.tsx | 18 +- .../settings/access/managed-profile-card.tsx | 17 +- .../access/managed-profile-create-form.tsx | 303 ++++++----- .../access/managed-profile-edit-form.tsx | 271 +++++----- .../access/managed-profile-guardians.tsx | 21 +- .../access/managed-profile-handover.tsx | 16 +- src/components/settings/advanced-section.tsx | 66 ++- src/components/settings/password-card.tsx | 39 +- .../passkey-removal-step-up.test.tsx | 17 +- .../existing-factor-reauth-dialog.tsx | 16 + .../security-section/passkey-list-section.tsx | 24 +- .../security-section/recent-proof-flow.ts | 144 ++++++ .../security-section/security-keys-card.tsx | 22 +- .../settings/security-section/totp-card.tsx | 77 ++- .../security-section/use-recent-proof.tsx | 126 ++--- .../require-fresh-mfa-if-enrolled.test.ts | 2 + ...fresh-mfa-or-elevation-if-enrolled.test.ts | 2 + src/lib/__tests__/require-fresh-mfa.test.ts | 40 ++ src/lib/api-handler.ts | 15 +- src/lib/openapi/routes/account-sharing.ts | 2 +- src/lib/openapi/routes/auth.ts | 2 +- .../managed-profiles-invalidation.test.ts | 8 +- src/lib/queries/use-managed-profiles.ts | 71 ++- ...anaged-profile-handover-stepup-sso.test.ts | 489 ++++++++++++++++++ 36 files changed, 2122 insertions(+), 466 deletions(-) create mode 100644 src/__tests__/step-up-web-caller-guard.test.ts create mode 100644 src/components/settings/access/__tests__/managed-profile-step-up-flow.test.ts create mode 100644 src/components/settings/security-section/recent-proof-flow.ts create mode 100644 tests/integration/managed-profile-handover-stepup-sso.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 85801ca8e..98dd87378 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -2333,7 +2333,11 @@ paths: /api/mcp/tokens, POST /api/tokens/measurements, POST /api/tokens/workouts, POST /api/tokens/documents, and the admin backup download, upload and restore, the admin data wipe and the admin password reset. Those answer 401 `auth.reproof.required` with `meta.methods` (the proofs this account can give) when the session signed in or - re-proved more than five minutes ago. + re-proved more than five minutes ago. A `totp`, `webauthn` or `passkey` proof here also clears the routes that + ask for a fresh second factor (the managed-profile acts, the second-factor and passkey changes, the password + change, account deletion, the data wipe and key rotation): on a browser session those answer 401 + `auth.stepup.required` with `meta.methods` naming the possession proofs the account holds, and the web re-proves + here and retries. The body takes the same shapes as POST /api/auth/step-up. On an account with a second factor only `totp`, @@ -21681,10 +21685,11 @@ paths: schema: $ref: "#/components/schemas/ManagedProfileEnvelope" "401": - description: "No fresh second-factor proof (`meta.errorCode: auth.stepup.required`), or none enrolled at all - (`meta.errorCode: auth.stepup.mfa_not_enrolled`) — this gate is unconditional, so an account without a - second factor cannot create a managed profile until it enrols one. The ordinary unauthenticated 401 arrives - here too, including for a Bearer caller, whose transport this gate does not read." + description: "No fresh second-factor proof (`meta.errorCode: auth.stepup.required`, with `meta.methods` naming the + proofs that clear it at POST /api/auth/reproof), or none enrolled at all (`meta.errorCode: + auth.stepup.mfa_not_enrolled`) — this gate is unconditional, so an account without a second factor cannot + create a managed profile until it enrols one. The ordinary unauthenticated 401 arrives here too, including + for a Bearer caller, whose transport this gate does not read." content: application/json: schema: diff --git a/messages/de.json b/messages/de.json index d59a91674..71c80c016 100644 --- a/messages/de.json +++ b/messages/de.json @@ -7323,6 +7323,7 @@ "emailChangePassword": "Aktuelles Passwort (nötig, um die E-Mail-Adresse zu ändern)", "reproof": { "description": "Damit werden deine Gesundheitsdaten weitergegeben oder exportiert. Bestätige, dass du es bist, um fortzufahren.", + "descriptionChange": "Das ändert etwas, das nur du ändern können solltest. Bestätige, dass du es bist, um fortzufahren.", "signInAgain": "Melde dich neu an und versuche es dann noch einmal. Dein Konto hat kein Passwort und keinen Passkey, den du hier bestätigen könntest.", "failed": "Das hat nicht geklappt. Prüfe deine Eingabe und versuche es noch einmal." }, diff --git a/messages/en.json b/messages/en.json index cd32491df..799837e13 100644 --- a/messages/en.json +++ b/messages/en.json @@ -7323,6 +7323,7 @@ "emailChangePassword": "Current password (needed to change the email address)", "reproof": { "description": "This shares or exports your health data. Confirm it's you to continue.", + "descriptionChange": "This changes something only you should be able to change. Confirm it's you to continue.", "signInAgain": "Sign in again, then try once more. Your account has no password or passkey to confirm here.", "failed": "That did not work. Check what you entered and try again." }, diff --git a/messages/es.json b/messages/es.json index 56ddea9d6..ad66196db 100644 --- a/messages/es.json +++ b/messages/es.json @@ -7323,6 +7323,7 @@ "emailChangePassword": "Contraseña actual (necesaria para cambiar la dirección de correo)", "reproof": { "description": "Esto comparte o exporta tus datos de salud. Confirma que eres tú para continuar.", + "descriptionChange": "Esto cambia algo que solo tú deberías poder cambiar. Confirma que eres tú para continuar.", "signInAgain": "Vuelve a iniciar sesión y inténtalo de nuevo. Tu cuenta no tiene contraseña ni llave de acceso que confirmar aquí.", "failed": "No ha funcionado. Revisa lo que has introducido e inténtalo de nuevo." }, diff --git a/messages/fr.json b/messages/fr.json index 788760a93..f3bc886bd 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -7323,6 +7323,7 @@ "emailChangePassword": "Mot de passe actuel (nécessaire pour changer l'adresse e-mail)", "reproof": { "description": "Cette action partage ou exporte vos données de santé. Confirmez qu'il s'agit bien de vous pour continuer.", + "descriptionChange": "Cela modifie quelque chose que vous seul devriez pouvoir modifier. Confirmez qu'il s'agit bien de vous pour continuer.", "signInAgain": "Reconnectez-vous, puis réessayez. Votre compte n'a ni mot de passe ni clé d'accès à confirmer ici.", "failed": "Cela n'a pas fonctionné. Vérifiez votre saisie et réessayez." }, diff --git a/messages/it.json b/messages/it.json index 40ddbeffa..09c83a377 100644 --- a/messages/it.json +++ b/messages/it.json @@ -7323,6 +7323,7 @@ "emailChangePassword": "Password attuale (necessaria per cambiare l'indirizzo email)", "reproof": { "description": "Questa azione condivide o esporta i tuoi dati sanitari. Conferma che sei tu per continuare.", + "descriptionChange": "Questo modifica qualcosa che solo tu dovresti poter modificare. Conferma che sei tu per continuare.", "signInAgain": "Accedi di nuovo, poi riprova. Il tuo account non ha una password o una passkey da confermare qui.", "failed": "Non ha funzionato. Controlla quanto inserito e riprova." }, diff --git a/messages/ko.json b/messages/ko.json index 4259bb70a..d408e3a2e 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -7323,6 +7323,7 @@ "emailChangePassword": "현재 비밀번호 (이메일 주소를 변경하려면 필요)", "reproof": { "description": "건강 데이터를 공유하거나 내보내는 작업입니다. 계속하려면 본인임을 확인하세요.", + "descriptionChange": "본인만 변경할 수 있어야 하는 항목을 변경합니다. 계속하려면 본인임을 확인하세요.", "signInAgain": "다시 로그인한 후 한 번 더 시도하세요. 이 계정에는 여기서 확인할 비밀번호나 패스키가 없습니다.", "failed": "확인하지 못했습니다. 입력한 내용을 확인하고 다시 시도하세요." }, diff --git a/messages/pl.json b/messages/pl.json index d071630d4..320f8480e 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -7323,6 +7323,7 @@ "emailChangePassword": "Obecne hasło (potrzebne do zmiany adresu e-mail)", "reproof": { "description": "Ta czynność udostępnia lub eksportuje Twoje dane zdrowotne. Potwierdź, że to Ty, aby kontynuować.", + "descriptionChange": "To zmienia coś, co tylko Ty powinieneś móc zmienić. Potwierdź, że to Ty, aby kontynuować.", "signInAgain": "Zaloguj się ponownie, a potem spróbuj jeszcze raz. Twoje konto nie ma hasła ani klucza dostępu do potwierdzenia tutaj.", "failed": "To się nie udało. Sprawdź wprowadzone dane i spróbuj ponownie." }, diff --git a/src/__tests__/step-up-web-caller-guard.test.ts b/src/__tests__/step-up-web-caller-guard.test.ts new file mode 100644 index 000000000..b6f1a8d76 --- /dev/null +++ b/src/__tests__/step-up-web-caller-guard.test.ts @@ -0,0 +1,383 @@ +/** + * Every web action behind a fresh-proof gate goes through the re-proof dialog. + * + * Two server gates hold an action back until the session has proved something + * recently: the recent-proof gate (`requireRecentProof`, + * `assertRecentCookieProof`, 401 `auth.reproof.required`) and the fresh-factor + * gate (`requireFreshMfa` and the helpers that escalate to it, 401 + * `auth.stepup.required`). Both name the proofs that clear them in + * `meta.methods`, and `useRecentProof` opens the dialog on either, posts the + * proof to `/api/auth/reproof` and runs the action once more. + * + * The defect this exists for: the managed-profile acts (handing a profile + * over, adding and removing a Guardian, creating, editing and ending a + * record) answered the fresh-factor refusal with a sentence telling the + * person to confirm their second factor, and offered nowhere to do it. The + * same was true of disabling the second factor, rotating recovery codes, + * removing a security key or passkey, changing the password, the data wipe, + * account deletion and the key rotation. The recent-proof surfaces had the + * dialog; the fresh-factor ones had text. + * + * What it pins: + * + * 1. The set of gated route methods, discovered from the route files and + * frozen below. A new gated route has to be entered here, with either + * its web callers checked by the sweep or the reason it has none. + * 2. Every request a web file makes to one of them runs inside the shared + * flow: `recentProof.run(…)` in a component, or `guard(…)` inside a + * query hook whose `guard` parameter every caller fills with + * `recentProof.run`. A gated request outside it is the defect again. + * + * Every sweep asserts a floor, because a matcher that finds nothing agrees + * with every list. + */ +import { describe, expect, it } 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"); +} + +/** + * Source with comments removed, so a route that only DESCRIBES a gate in its + * doc comment is not counted, and a path quoted in a comment is not a call. + */ +function code(rel: string): string { + return read(rel) + .replace(/\/\*[\s\S]*?\*\//g, "") + .split("\n") + .map((line) => line.replace(/(^|[^:"'`\\])\/\/.*$/, "$1")) + .join("\n"); +} + +const METHODS = ["GET", "POST", "PUT", "PATCH", "DELETE"] as const; + +const GATE = + /\b(?:requireFreshMfa|requireFreshMfaIfEnrolled|requireFreshMfaOrElevationIfEnrolled|requireRecentProof|assertRecentCookieProof)\s*\(|\bfreshFactor\s*:\s*true\b/; + +/** `src/app/api/a/[id]/route.ts` -> `/api/a/[id]`. */ +function routePath(rel: string): string { + return "/" + rel.replace(/^app\//, "").replace(/\/route\.ts$/, ""); +} + +/** The gated `METHOD /api/path` pairs, read off the route files. */ +function discoverGatedRoutes(): string[] { + const routes = walkSourceFiles(join(SRC, "app", "api"), { floor: 300 }) + .filter((p) => p.endsWith("/route.ts") && !p.includes("__tests__")) + .map((p) => `app/api/${p}`); + expect(routes.length).toBeGreaterThan(300); + + const found = new Set(); + for (const rel of routes) { + const src = code(rel); + if (!GATE.test(src)) continue; + const exportRe = new RegExp( + `export\\s+(?:const\\s+(${METHODS.join("|")})\\s*=|async\\s+function\\s+(${METHODS.join("|")})\\s*\\()`, + "g", + ); + const exports = [...src.matchAll(exportRe)].map((m) => ({ + method: m[1] ?? m[2], + index: m.index ?? 0, + })); + const path = routePath(rel); + // A gate before the first exported method lives in a helper or a + // `handler` constant the exports wrap, and binds every method. + const preamble = src.slice(0, exports[0]?.index ?? src.length); + if (GATE.test(preamble)) { + for (const e of exports) found.add(`${e.method} ${path}`); + } + exports.forEach((e, i) => { + const block = src.slice(e.index, exports[i + 1]?.index ?? src.length); + if (GATE.test(block)) found.add(`${e.method} ${path}`); + }); + } + return [...found].sort(); +} + +/** + * Every gated route method, and how the web reaches it. `"web"` means the + * sweep below finds its callers and checks them; anything else is the reason + * no web file calls it. + */ +const GATED_ROUTES: Record = { + "DELETE /api/admin/data": "web", + "DELETE /api/auth/me/mfa/webauthn/[id]": "web", + "DELETE /api/auth/passkeys/[id]": "web", + "DELETE /api/managed-profiles/[id]": "web", + "DELETE /api/managed-profiles/[id]/guardians/[grantId]": "web", + "DELETE /api/settings/account": "web", + "DELETE /api/settings/data": "web", + "GET /api/admin/backups/[id]/download": "web", + "GET /api/auth/reproof": + "the flow's own question: the web asks it inside recentProof.run before an upload or a typed confirmation", + "GET /api/export": + "the plaintext whole-record download (`type=all`); the web's whole-record export is the full backup, and its single-type exports have their own routes", + "GET /api/export/full-backup": "web", + "GET /api/fhir/Patient/$everything": + "a FHIR client endpoint, reached with a fhir:read token; no web file calls it", + "GET /api/mcp/oauth/authorize": + "a browser navigation from an assistant, answered with a page that links to /confirm-access, which runs the dialog", + "PATCH /api/managed-profiles/[id]": "web", + "POST /api/account/grants": "web", + "POST /api/admin/backups/[id]/restore": "web", + "POST /api/admin/backups/upload": "web", + "POST /api/admin/encryption/rotate": "web", + "POST /api/admin/users/[id]/reset-password": "web", + "POST /api/auth/me/mfa/disable": "web", + "POST /api/auth/me/mfa/recovery-codes/regenerate": "web", + "POST /api/auth/password": "web", + "POST /api/export/encrypted": "web", + "POST /api/export/health-record": "web", + "POST /api/managed-profiles": "web", + "POST /api/managed-profiles/[id]/guardians": "web", + "POST /api/managed-profiles/[id]/handover": "web", + "POST /api/mcp/oauth/authorize": + "the consent form's own submission, refused into the same /confirm-access page as the GET", + "POST /api/mcp/tokens": "web", + "POST /api/share-links": "web", + "POST /api/tokens/documents": "web", + "POST /api/tokens/measurements": "web", + "POST /api/tokens/workouts": "web", +}; + +/** The web side: everything under src that is not a route, a test or docs. */ +function webFiles(): string[] { + return walkSourceFiles(SRC, { floor: 3000 }) + .filter((p) => !p.startsWith("generated/")) + .filter((p) => !p.startsWith("app/api/")) + .filter((p) => !p.startsWith("lib/openapi/")) + .filter((p) => !p.includes("__tests__")) + .filter((p) => !p.endsWith(".test.ts") && !p.endsWith(".test.tsx")) + .sort(); +} + +/** A string or template literal naming exactly this path. */ +function pathPattern(path: string): RegExp { + const body = path + .split("/") + .map((seg) => + /^\[.+\]$/.test(seg) + ? "\\$\\{[^}]+\\}" + : seg.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), + ) + .join("/"); + return new RegExp(`["'\`]${body}(?=["'\`?])`, "g"); +} + +/** Which method a call naming the path at `index` sends. */ +function methodAt(src: string, index: number, matchLength: number): string { + const before = src.slice(Math.max(0, index - 160), index); + const helper = [ + ...before.matchAll(/\bapi(Get|Post|Put|Patch|Delete)\b(?:<[^()]*>)?\s*\(/g), + ].pop(); + // The helper must be the call this literal is the first argument of. + if ( + helper && + /^\s*$/.test(before.slice((helper.index ?? 0) + helper[0].length)) + ) { + return helper[1].toUpperCase(); + } + const after = src.slice(index + matchLength, index + matchLength + 240); + const explicit = after.match(/method\s*:\s*["'](\w+)["']/); + return explicit ? explicit[1].toUpperCase() : "GET"; +} + +/** The name of the function the code at `index` sits in, if it has one. */ +function enclosingFunction(src: string, index: number): string | null { + const head = src.slice(0, index); + const decls = [ + ...head.matchAll( + /(?:\bfunction\s+(\w+)\s*\(|\b(?:const|let)\s+(\w+)\s*=\s*(?:async\s+)?(?:\([^)]*\)|\w+)\s*=>)/g, + ), + ].pop(); + return decls ? (decls[1] ?? decls[2] ?? null) : null; +} + +/** + * The request at `index` runs inside the shared flow: directly inside a + * `recentProof.run(…)` or a hook's `guard(…)`, or in a named function that is + * itself handed to, or called from, `recentProof.run`. + */ +function guardedAt(src: string, index: number): boolean { + const before = src.slice(Math.max(0, index - 400), index); + if (/(?:\.run|\bguard)\s*\(/.test(before)) return true; + const name = enclosingFunction(src, index); + if (!name) return false; + return new RegExp( + `\\.run\\(\\s*(?:async\\s*)?(?:\\(\\)\\s*=>\\s*)?${name}\\b`, + ).test(src); +} + +interface GatedCall { + file: string; + route: string; + guarded: boolean; +} + +/** + * A query hook whose request is not guarded inside the hook is still guarded + * when every component that mounts it sends the mutation through the flow: + * `recentProof.run(() => mutation.mutateAsync(…))`. At least one such + * component must exist, or nothing is guarded. + */ +function hookCallersWrap( + files: string[], + hookFile: string, + src: string, + index: number, +): boolean { + const hook = [ + ...src.slice(0, index).matchAll(/export function (use\w+)/g), + ].pop()?.[1]; + if (!hook) return false; + let callers = 0; + for (const file of files) { + if (file === hookFile) continue; + const other = code(file); + for (const m of other.matchAll( + new RegExp(`\\b(?:const|let)\\s+(\\w+)\\s*=\\s*${hook}\\s*\\(`, "g"), + )) { + callers++; + const wraps = new RegExp( + `\\.run\\(\\s*(?:async\\s*)?\\(\\)\\s*=>\\s*${m[1]}\\.mutateAsync\\b`, + ); + if (!wraps.test(other)) return false; + } + } + return callers > 0; +} + +function gatedWebCalls(): GatedCall[] { + const calls: GatedCall[] = []; + const files = webFiles(); + const sources = new Map(files.map((f) => [f, code(f)])); + for (const route of Object.keys(GATED_ROUTES)) { + if (GATED_ROUTES[route] !== "web") continue; + const [method, path] = route.split(" "); + const re = pathPattern(path); + for (const file of files) { + const src = sources.get(file) ?? ""; + for (const m of src.matchAll(re)) { + const index = m.index ?? 0; + // The path handed to a component as a prop (`endpoint="/api/…"`): + // the request is wherever that component sends the prop. + const prop = src.slice(Math.max(0, index - 40), index).match(/(\w+)=$/); + if (prop) { + const sends = [ + ...src.matchAll( + new RegExp(`\\bapiFetchRaw\\(\\s*${prop[1]}\\s*,`, "g"), + ), + ]; + for (const send of sends) { + const at = send.index ?? 0; + if (methodAt(src, at, send[0].length) !== method) continue; + calls.push({ file, route, guarded: guardedAt(src, at) }); + } + continue; + } + if (methodAt(src, index, m[0].length) !== method) continue; + const guarded = + guardedAt(src, index) || + (file.startsWith("lib/queries/") && + hookCallersWrap(files, file, src, index)); + calls.push({ file, route, guarded }); + } + } + } + return calls; +} + +/** + * Calls the sweep cannot read as guarded, each checked by hand and pinned. + * The request runs after a `recentProof.run(() => apiGet("/api/auth/reproof"))` + * a few lines earlier, so the proof is in place before a file is sent or a + * typed confirmation is spent. + */ +const PRECHECKED = new Set([ + // `upload.mutationFn` asks first, then calls `uploadOnce`, so a large file + // is not sent once only to be refused. + "components/admin/backups-section.tsx -> POST /api/admin/backups/upload", +]); + +describe("fresh-proof gates on the web", () => { + it("freezes the set of gated route methods", () => { + const discovered = discoverGatedRoutes(); + expect(discovered.length).toBeGreaterThanOrEqual(30); + expect(discovered).toEqual(Object.keys(GATED_ROUTES).sort()); + }); + + it("finds a web caller for every route marked as called from the web", () => { + const calls = gatedWebCalls(); + expect(calls.length).toBeGreaterThanOrEqual(25); + const reached = new Set(calls.map((c) => c.route)); + const unreached = Object.keys(GATED_ROUTES).filter( + (r) => GATED_ROUTES[r] === "web" && !reached.has(r), + ); + expect(unreached).toEqual([]); + }); + + it("sends every gated request through the re-proof flow", () => { + const unguarded = gatedWebCalls() + .filter((c) => !c.guarded) + .map((c) => `${c.file} -> ${c.route}`) + .filter((key) => !PRECHECKED.has(key)); + expect(unguarded).toEqual([]); + }); + + it("renders the dialog wherever a component runs the flow", () => { + const files = [...new Set(gatedWebCalls().map((c) => c.file))].filter((f) => + f.endsWith(".tsx"), + ); + expect(files.length).toBeGreaterThanOrEqual(10); + const missing = files.filter((f) => { + const src = code(f); + return !/useRecentProof\s*\(/.test(src) || !/\.dialog\b/.test(src); + }); + expect(missing).toEqual([]); + }); + + it("makes every caller of a guarded query hook pass recentProof.run", () => { + // Hooks whose gated request runs inside `guard(…)`. + const hookFiles = [ + ...new Set( + gatedWebCalls() + .filter((c) => c.file.startsWith("lib/queries/")) + .map((c) => c.file), + ), + ]; + expect(hookFiles.length).toBeGreaterThanOrEqual(1); + const hooks: string[] = []; + for (const file of hookFiles) { + for (const m of code(file).matchAll( + /export function (use\w+)\(\s*guard\s*:\s*ProofGuard\s*\)/g, + )) { + hooks.push(m[1]); + } + } + expect(hooks.length).toBeGreaterThanOrEqual(6); + + const offenders: string[] = []; + let callSites = 0; + for (const file of webFiles()) { + if (hookFiles.includes(file)) continue; + const src = code(file); + for (const hook of hooks) { + for (const m of src.matchAll( + new RegExp(`\\b${hook}\\s*\\(([^)]*)\\)`, "g"), + )) { + callSites++; + if (!/^\s*\w+\.run\s*$/.test(m[1])) { + offenders.push(`${file}: ${hook}(${m[1].trim()})`); + } + } + } + } + expect(callSites).toBeGreaterThanOrEqual(6); + expect(offenders).toEqual([]); + }); +}); diff --git a/src/app/api/auth/reproof/route.ts b/src/app/api/auth/reproof/route.ts index f01203d96..776e5d8a3 100644 --- a/src/app/api/auth/reproof/route.ts +++ b/src/app/api/auth/reproof/route.ts @@ -6,7 +6,11 @@ * token, or run an admin backup, restore, wipe or reset. Those routes answer * 401 `auth.reproof.required` with `meta.methods` when the session carries no * recent proof; the web opens the re-proof dialog, posts the proof here, and - * retries. + * retries. The fresh-factor routes (`requireFreshMfa`: the managed-profile + * acts, second-factor and passkey changes, the password change, account + * deletion, the data wipe, key rotation) answer 401 `auth.stepup.required` + * with `meta.methods` on a browser session, and the same dialog clears them + * here with a possession proof. * * What a proof stamps on the session row: * - `totp`, `webauthn` or `passkey`: `mfaVerifiedAt` and `reproofAt`. The diff --git a/src/components/admin/encryption-section.tsx b/src/components/admin/encryption-section.tsx index 3685d74c3..0783af6a5 100644 --- a/src/components/admin/encryption-section.tsx +++ b/src/components/admin/encryption-section.tsx @@ -45,6 +45,11 @@ import { useFormatters, useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { apiFetchRaw, apiGet } from "@/lib/api/api-fetch"; import { getApiErrorMessage } from "./_shared"; +import { + recentProofErrorMessage, + throwIfReproofRequired, + useRecentProof, +} from "@/components/settings/security-section/use-recent-proof"; import { KeyBackupCard } from "./key-backup-card"; import { useKeyBackupStatus } from "./use-key-backup-status"; @@ -105,12 +110,22 @@ export function EncryptionSection() { staleTime: 15_000, }); + // Rotation needs the admin's second factor proved within five minutes. When + // it is older the server names the proofs that clear it, the shared dialog + // collects one, and the rotation is queued once more. + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); const rotate = useMutation({ mutationFn: async () => { - const res = await apiFetchRaw("/api/admin/encryption/rotate", { - method: "POST", - credentials: "include", - }); + const res = await recentProof.run(async () => + throwIfReproofRequired( + await apiFetchRaw("/api/admin/encryption/rotate", { + method: "POST", + credentials: "include", + }), + ), + ); if (!res.ok) { if (res.status === 401) { throw new Error(t("admin.section.encryption.stepUpNeeded")); @@ -126,10 +141,12 @@ export function EncryptionSection() { }); }, onError: (err: Error) => { - showErrorToast( + const message = recentProofErrorMessage( + err, err.message || t("admin.section.encryption.rotateFailed"), - { cause: err }, ); + if (message === null) return; + showErrorToast(message, { cause: err }); }, }); @@ -182,6 +199,7 @@ export function EncryptionSection() { return (
+ {recentProof.dialog} {/* ── Coverage summary ───────────────────────────────────────── */} diff --git a/src/components/settings/access/__tests__/managed-profile-step-up-flow.test.ts b/src/components/settings/access/__tests__/managed-profile-step-up-flow.test.ts new file mode 100644 index 000000000..35424713c --- /dev/null +++ b/src/components/settings/access/__tests__/managed-profile-step-up-flow.test.ts @@ -0,0 +1,339 @@ +/** + * #959 — "I get the error message that I need to confirm my second factor, + * but there is nowhere to do so." + * + * Handing a managed profile over, adding a Guardian and removing one are gated + * on a second factor proved within five minutes (`requireFreshMfa`). Its + * refusal, 401 `auth.stepup.required`, now names the proofs that clear it in + * `meta.methods`, and each of these mutations runs inside the re-proof flow: + * the dialog opens with those proofs, the proof goes to `/api/auth/reproof`, + * and the request is sent once more. + * + * Driven for real below: the hooks' own `mutationFn`s, the flow + * `useRecentProof` binds to the dialog, and a fake network that answers the + * way the routes do. `@tanstack/react-query` is reduced to the two calls the + * hook bodies make, as in `managed-profiles-invalidation.test.ts`; the unit + * environment has no DOM, so the dialog itself is represented by the view the + * flow drives (which proofs it offers, its error line) and by the test acting + * as the person: proving, or closing it. + * + * Mutation checks, run: drop `guard(…)` from `useCreateManagedProfileHandover` + * and every handover case goes red (the refusal escapes, no dialog); make + * `isProofRefusal` match `auth.reproof.required` only and every case that + * expects a dialog goes red. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { QueryClient } from "@tanstack/react-query"; + +vi.mock("@tanstack/react-query", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useMutation: (options: unknown) => options, + useQueryClient: () => new QueryClient(), + }; +}); + +vi.mock("qrcode", () => ({ + default: { toDataURL: vi.fn(async () => "data:image/png;base64,AAAA") }, +})); + +import { + useCreateManagedProfileHandover, + useInviteManagedProfileGuardian, + useRemoveManagedProfileGuardian, +} from "@/lib/queries/use-managed-profiles"; +import { ApiError, apiPost } from "@/lib/api/api-fetch"; +import { + ReproofCancelledError, + createRecentProofFlow, + type RecentProofFlow, +} from "@/components/settings/security-section/recent-proof-flow"; +import type { ReauthMethod } from "@/components/settings/security-section/existing-factor-reauth-dialog"; +import { stepUpErrorKey } from "@/components/settings/access/grant-action-error"; + +type MutationOptions = { mutationFn: (input: I) => Promise }; + +// ── The network, answering the way the routes do ──────────────────────────── + +const STALE = { + data: null, + error: "Recent second-factor verification required", + meta: { errorCode: "auth.stepup.required", methods: ["totp", "passkey"] }, +}; + +function json(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} + +/** Every request, as `METHOD path`, in order. */ +let sent: string[] = []; +/** The session's second factor is fresh once a proof was accepted. */ +let fresh = false; +/** The codes the authenticator would accept. */ +const GOOD_CODE = "246810"; +let actionAnswer: () => Response = () => json(200, { data: {}, error: null }); + +beforeEach(() => { + sent = []; + fresh = false; + vi.spyOn(globalThis, "fetch").mockImplementation(async (input, init) => { + const path = String(input); + const method = (init?.method ?? "GET").toUpperCase(); + sent.push(`${method} ${path}`); + if (path === "/api/auth/reproof") { + const proof = JSON.parse(String(init?.body ?? "{}")); + if (proof.method === "totp" && proof.code === GOOD_CODE) { + fresh = true; + return json(200, { data: { method: "totp" }, error: null }); + } + return json(401, { + data: null, + error: "Verification failed", + meta: { errorCode: "auth.reproof.failed" }, + }); + } + return fresh ? actionAnswer() : json(401, STALE); + }); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +// ── The dialog, as the flow drives it ─────────────────────────────────────── + +interface Dialog { + flow: RecentProofFlow; + /** What the dialog offered each time it opened. */ + offered: ReauthMethod[][]; + open: boolean; + error: string | null; + /** Resolves when the dialog next opens. */ + opened: () => Promise; +} + +function dialog(): Dialog { + let notify: (() => void) | null = null; + const d: Dialog = { + offered: [], + open: false, + error: null, + opened: () => + new Promise((resolve) => { + if (d.open) resolve(); + else notify = resolve; + }), + flow: undefined as unknown as RecentProofFlow, + }; + d.flow = createRecentProofFlow({ + view: { + setMethods: (methods) => { + d.open = methods !== null; + if (methods) { + d.offered.push(methods); + notify?.(); + notify = null; + } + }, + setPending: () => {}, + setError: (error) => { + d.error = error; + }, + }, + // The same call `useRecentProof` makes. + submit: (proof) => apiPost("/api/auth/reproof", proof), + messages: () => ({ signInAgain: "sign in again", failed: "failed" }), + }); + return d; +} + +const HANDOVER = "/api/managed-profiles/p1/handover"; + +function useHandover(d: Dialog) { + return useCreateManagedProfileHandover( + d.flow.run, + ) as unknown as MutationOptions< + { profileId: string; expiresInDays: 7; proposals: [] }, + { url: string; qrDataUrl: string } + >; +} + +describe("handing a profile over with a stale second factor", () => { + beforeEach(() => { + actionAnswer = () => + json(201, { + data: { + url: "https://example.test/claim#t", + token: "t", + expiresAt: "2026-10-24T00:00:00.000Z", + }, + error: null, + }); + }); + + it("opens the dialog with the proofs the server named, then hands over", async () => { + const d = dialog(); + const pending = useHandover(d).mutationFn({ + profileId: "p1", + expiresInDays: 7, + proposals: [], + }); + + await d.opened(); + expect(d.offered).toEqual([["totp", "passkey"]]); + await d.flow.prove({ method: "totp", code: GOOD_CODE }); + + const link = await pending; + expect(link.url).toBe("https://example.test/claim#t"); + expect(d.open).toBe(false); + expect(sent).toEqual([ + `POST ${HANDOVER}`, + "POST /api/auth/reproof", + `POST ${HANDOVER}`, + ]); + }); + + it("keeps the dialog open on a wrong code and retries only after a right one", async () => { + const d = dialog(); + const pending = useHandover(d).mutationFn({ + profileId: "p1", + expiresInDays: 7, + proposals: [], + }); + + await d.opened(); + await d.flow.prove({ method: "totp", code: "000000" }); + expect(d.open).toBe(true); + expect(d.error).toBe("failed"); + expect(sent).toEqual([`POST ${HANDOVER}`, "POST /api/auth/reproof"]); + + await d.flow.prove({ method: "totp", code: GOOD_CODE }); + await pending; + expect(sent.filter((s) => s === `POST ${HANDOVER}`)).toHaveLength(2); + }); + + it("sends nothing more when the dialog is closed", async () => { + const d = dialog(); + const pending = useHandover(d).mutationFn({ + profileId: "p1", + expiresInDays: 7, + proposals: [], + }); + + await d.opened(); + d.flow.cancel(); + + await expect(pending).rejects.toBeInstanceOf(ReproofCancelledError); + expect(sent).toEqual([`POST ${HANDOVER}`]); + }); + + it("asks once: a second refusal after the proof is the answer", async () => { + actionAnswer = () => json(401, STALE); + const d = dialog(); + const pending = useHandover(d).mutationFn({ + profileId: "p1", + expiresInDays: 7, + proposals: [], + }); + + await d.opened(); + await d.flow.prove({ method: "totp", code: GOOD_CODE }); + + await expect(pending).rejects.toMatchObject({ status: 401 }); + expect(d.offered).toHaveLength(1); + }); + + it("opens no dialog for an account with no second factor, and says where to get one", async () => { + vi.mocked(globalThis.fetch).mockResolvedValueOnce( + json(401, { + data: null, + error: "Recent second-factor verification required", + meta: { errorCode: "auth.stepup.mfa_not_enrolled" }, + }), + ); + const d = dialog(); + const err = await useHandover(d) + .mutationFn({ profileId: "p1", expiresInDays: 7, proposals: [] }) + .catch((e: unknown) => e); + + expect(d.offered).toEqual([]); + expect(err).toBeInstanceOf(ApiError); + expect(stepUpErrorKey(err as ApiError)).toBe( + "recordSharing.managed.errorMfaRequired", + ); + }); +}); + +describe("adding and removing a Guardian with a stale second factor", () => { + beforeEach(() => { + actionAnswer = () => json(201, { data: { id: "g2" }, error: null }); + }); + + it("adds the Guardian after the proof", async () => { + const d = dialog(); + const invite = useInviteManagedProfileGuardian( + d.flow.run, + ) as unknown as MutationOptions< + { profileId: string; identifier: string; expiresAt: null }, + unknown + >; + const pending = invite.mutationFn({ + profileId: "p1", + identifier: "someone", + expiresAt: null, + }); + + await d.opened(); + expect(d.offered).toEqual([["totp", "passkey"]]); + await d.flow.prove({ method: "totp", code: GOOD_CODE }); + await pending; + + expect(sent).toEqual([ + "POST /api/managed-profiles/p1/guardians", + "POST /api/auth/reproof", + "POST /api/managed-profiles/p1/guardians", + ]); + }); + + it("removes the Guardian after the proof", async () => { + actionAnswer = () => json(200, { data: { revoked: true }, error: null }); + const d = dialog(); + const remove = useRemoveManagedProfileGuardian( + d.flow.run, + ) as unknown as MutationOptions< + { profileId: string; grantId: string }, + unknown + >; + const pending = remove.mutationFn({ profileId: "p1", grantId: "g2" }); + + await d.opened(); + await d.flow.prove({ method: "totp", code: GOOD_CODE }); + await pending; + + expect(sent).toEqual([ + "DELETE /api/managed-profiles/p1/guardians/g2", + "POST /api/auth/reproof", + "DELETE /api/managed-profiles/p1/guardians/g2", + ]); + }); + + it("removes nothing when the dialog is closed", async () => { + const d = dialog(); + const remove = useRemoveManagedProfileGuardian( + d.flow.run, + ) as unknown as MutationOptions< + { profileId: string; grantId: string }, + unknown + >; + const pending = remove.mutationFn({ profileId: "p1", grantId: "g2" }); + + await d.opened(); + d.flow.cancel(); + await expect(pending).rejects.toBeInstanceOf(ReproofCancelledError); + expect(sent).toEqual(["DELETE /api/managed-profiles/p1/guardians/g2"]); + }); +}); diff --git a/src/components/settings/access/grant-action-error.tsx b/src/components/settings/access/grant-action-error.tsx index fdef676ce..a398e3b5f 100644 --- a/src/components/settings/access/grant-action-error.tsx +++ b/src/components/settings/access/grant-action-error.tsx @@ -65,7 +65,7 @@ const STEP_UP_ACTIONS = new Set([ * * `requireFreshMfa` refuses for two different reasons and says which: a stale * proof (`auth.stepup.required`), which the person clears by proving their - * factor, and NO factor at all (`auth.stepup.mfa_not_enrolled`), which they + * factor in the re-proof dialog (`useRecentProof`), and NO factor at all (`auth.stepup.mfa_not_enrolled`), which they * cannot clear that way. Collapsing the two tells somebody with no second * factor to confirm one — a dead end offered by the very surface that owns * the route out of it. @@ -74,9 +74,19 @@ const STEP_UP_ACTIONS = new Set([ * invitation, and the two acts below) so the two sentences cannot come apart. */ export function stepUpErrorKey(err: ApiError): string { - return err.meta?.errorCode === "auth.stepup.mfa_not_enrolled" - ? "recordSharing.managed.errorMfaRequired" - : "recordSharing.managed.errorStepUp"; + switch (err.meta?.errorCode) { + case "auth.stepup.mfa_not_enrolled": + return "recordSharing.managed.errorMfaRequired"; + // The re-proof dialog had nothing to offer (an account that signs in + // through an identity provider only); signing in again is the way through. + case "auth.reproof.sign_in_again": + return "settings.reproof.signInAgain"; + // A stale proof normally never gets here: every action in this family runs + // through the re-proof dialog, which clears it and sends the request again. + // This is the sentence for a refusal the dialog could not take up. + default: + return "recordSharing.managed.errorStepUp"; + } } /** diff --git a/src/components/settings/access/managed-profile-card.tsx b/src/components/settings/access/managed-profile-card.tsx index a9f9248b8..60520a9f7 100644 --- a/src/components/settings/access/managed-profile-card.tsx +++ b/src/components/settings/access/managed-profile-card.tsx @@ -7,6 +7,10 @@ import { GrantActionAlert, grantActionErrorKey, } from "@/components/settings/access/grant-action-error"; +import { + ReproofCancelledError, + useRecentProof, +} from "@/components/settings/security-section/use-recent-proof"; import { ManagedProfileCreateForm } from "@/components/settings/access/managed-profile-create-form"; import { ManagedProfileEditForm } from "@/components/settings/access/managed-profile-edit-form"; import { ManagedProfileGuardians } from "@/components/settings/access/managed-profile-guardians"; @@ -69,11 +73,17 @@ import { export function ManagedProfileCard() { const { t } = useTranslations(); const { user } = useAuth(); - const remove = useDeleteManagedProfile(); + // Ending a record is gated on a fresh second factor; the dialog collects it + // and the deletion goes out once more. + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); + const remove = useDeleteManagedProfile(recentProof.run); const profiles = managedProfilesOf(user?.accountAccess?.accounts); return ( + {recentProof.dialog} void; } = {}) { const { t, locale: actorLocale } = useTranslations(); - const create = useCreateManagedProfile(); + // Gated on a fresh second factor: the dialog collects it and the request + // goes out once more. A closed dialog changed nothing and says nothing. + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); + const create = useCreateManagedProfile(recentProof.run); const [displayName, setDisplayName] = useState(""); const [dateOfBirth, setDateOfBirth] = useState(""); @@ -135,165 +144,173 @@ export function ManagedProfileCreateForm({ // person is being asked to retry, and a form that emptied itself // would make them type it all again to find out whether the second // attempt fails the same way. - onError: (err) => setError(t(createManagedProfileErrorKey(err))), + onError: (err) => { + if (err instanceof ReproofCancelledError) return; + setError(t(createManagedProfileErrorKey(err))); + }, }, ); }; return ( -
-
- - { - setDisplayName(e.target.value); - setError(null); - }} - placeholder={t("recordSharing.managed.namePlaceholder")} - autoComplete="off" - maxLength={80} - aria-invalid={ - displayName.length > 0 && nameIssue !== null ? "true" : undefined - } - aria-describedby="managed-profile-name-hint" - /> - 0 && nameIssue !== null ? t(nameIssue) : null - } - /> -
+ <> + {/* Outside the form: the dialog's own form must not submit this one + through the portal. */} + {recentProof.dialog} + +
+ + { + setDisplayName(e.target.value); + setError(null); + }} + placeholder={t("recordSharing.managed.namePlaceholder")} + autoComplete="off" + maxLength={80} + aria-invalid={ + displayName.length > 0 && nameIssue !== null ? "true" : undefined + } + aria-describedby="managed-profile-name-hint" + /> + 0 && nameIssue !== null ? t(nameIssue) : null + } + /> +
-
- - -

- {t("recordSharing.managed.dobHint")} -

-
+
+ + +

+ {t("recordSharing.managed.dobHint")} +

+
-
- - setLocale(e.target.value as Locale)} - > - {locales.map((option) => ( - - ))} - -

- {t("recordSharing.managed.localeHint")} -

-
+
+ + setLocale(e.target.value as Locale)} + > + {locales.map((option) => ( + + ))} + +

+ {t("recordSharing.managed.localeHint")} +

+
-
- - setGender(genderFromSelect(e.target.value))} - > - {MANAGED_PROFILE_GENDER_OPTIONS.map((option) => ( - - ))} - -

- {t("recordSharing.managed.genderHint")} -

-
+
+ + setGender(genderFromSelect(e.target.value))} + > + {MANAGED_PROFILE_GENDER_OPTIONS.map((option) => ( + + ))} + +

+ {t("recordSharing.managed.genderHint")} +

+
-
- { - setTimezone(next); - setError(null); - }} - hint={t("recordSharing.managed.timezoneHint")} - /> - {/* The picker's own list comes from the engine, so the select branch +
+ { + setTimezone(next); + setError(null); + }} + hint={t("recordSharing.managed.timezoneHint")} + /> + {/* The picker's own list comes from the engine, so the select branch cannot produce an unknown zone. Its free-text fallback (older engines without `Intl.supportedValuesOf`) can, and this sentence is then the ONLY explanation for a submit button that will not press. It rides beside the picker rather than through its `hint` prop, because that prop renders muted meta and a refusal rendered as meta is a refusal nobody reads. */} - -
+ +
- + - {error && ( -

- {error} -

- )} - {created && ( -

- {t("recordSharing.managed.created", { name: created })} -

- )} -
+ {error && ( +

+ {error} +

+ )} + {created && ( +

+ {t("recordSharing.managed.created", { name: created })} +

+ )} + + ); } diff --git a/src/components/settings/access/managed-profile-edit-form.tsx b/src/components/settings/access/managed-profile-edit-form.tsx index 2245f61c3..76db29edb 100644 --- a/src/components/settings/access/managed-profile-edit-form.tsx +++ b/src/components/settings/access/managed-profile-edit-form.tsx @@ -10,6 +10,10 @@ import { genderFromSelect, } from "@/components/settings/access/managed-profile-create-form"; import { stepUpErrorKey } from "@/components/settings/access/grant-action-error"; +import { + ReproofCancelledError, + useRecentProof, +} from "@/components/settings/security-section/use-recent-proof"; import { SettingsCardActions } from "@/components/settings/_card-actions"; import { TimezonePicker } from "@/components/settings/timezone-picker"; import { Button } from "@/components/ui/button"; @@ -124,7 +128,12 @@ function ManagedProfileEditFields({ onDone: () => void; }) { const { t, locale: actorLocale } = useTranslations(); - const update = useUpdateManagedProfile(); + // Gated on a fresh second factor: the dialog collects it and the request + // goes out once more. A closed dialog changed nothing and says nothing. + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); + const update = useUpdateManagedProfile(recentProof.run); const [displayName, setDisplayName] = useState(profile.displayName ?? ""); const [dateOfBirth, setDateOfBirth] = useState(profile.dateOfBirth ?? ""); @@ -172,143 +181,151 @@ function ManagedProfileEditFields({ // Nothing is reset: a refused save is one somebody is being asked to // retry, and a form that restored the server's values would throw the // edit away to explain that it did not land. - onError: (err) => setError(t(editManagedProfileErrorKey(err))), + onError: (err) => { + if (err instanceof ReproofCancelledError) return; + setError(t(editManagedProfileErrorKey(err))); + }, }, ); }; return ( -
-
- - { - setDisplayName(e.target.value); - setError(null); - }} - autoComplete="off" - maxLength={80} - aria-invalid={nameIssue !== null ? "true" : undefined} - /> - -
+ <> + {/* Outside the form: the dialog's own form must not submit this one + through the portal. */} + {recentProof.dialog} + +
+ + { + setDisplayName(e.target.value); + setError(null); + }} + autoComplete="off" + maxLength={80} + aria-invalid={nameIssue !== null ? "true" : undefined} + /> + +
-
- - -
+
+ + +
-
- - setLocale(e.target.value as Locale)} - > - {locales.map((option) => ( - - ))} - -
+
+ + setLocale(e.target.value as Locale)} + > + {locales.map((option) => ( + + ))} + +
-
- - setGender(genderFromSelect(e.target.value))} - > - {MANAGED_PROFILE_GENDER_OPTIONS.map((option) => ( - - ))} - -

- {t("recordSharing.managed.genderHint")} -

-
+
+ + setGender(genderFromSelect(e.target.value))} + > + {MANAGED_PROFILE_GENDER_OPTIONS.map((option) => ( + + ))} + +

+ {t("recordSharing.managed.genderHint")} +

+
-
- { - setTimezone(next); - setError(null); - }} - hint={t("recordSharing.managed.timezoneHint")} - /> - -
+
+ { + setTimezone(next); + setError(null); + }} + hint={t("recordSharing.managed.timezoneHint")} + /> + +
- {/* Above the action row, per UI-STANDARDS §12: nothing follows it. */} - {error && ( -

- {error} -

- )} + {/* Above the action row, per UI-STANDARDS §12: nothing follows it. */} + {error && ( +

+ {error} +

+ )} - - - - -
+ + + + + + ); } diff --git a/src/components/settings/access/managed-profile-guardians.tsx b/src/components/settings/access/managed-profile-guardians.tsx index 916cc25f4..b6f831e2f 100644 --- a/src/components/settings/access/managed-profile-guardians.tsx +++ b/src/components/settings/access/managed-profile-guardians.tsx @@ -8,6 +8,10 @@ import { stepUpErrorKey, } from "@/components/settings/access/grant-action-error"; import { endOfDayIso } from "@/components/settings/access/grant-invite-card"; +import { + ReproofCancelledError, + useRecentProof, +} from "@/components/settings/security-section/use-recent-proof"; import { Button } from "@/components/ui/button"; import { ConfirmButton } from "@/components/ui/confirm-button"; import { DateField } from "@/components/ui/date-field"; @@ -80,8 +84,14 @@ export function ManagedProfileGuardians({ const { t } = useTranslations(); const fmt = useFormatters(); const { user } = useAuth(); - const invite = useInviteManagedProfileGuardian(); - const remove = useRemoveManagedProfileGuardian(); + // Both acts are gated on a fresh second factor; the dialog collects it and + // the request goes out once more. A closed dialog changed nothing and says + // nothing. + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); + const invite = useInviteManagedProfileGuardian(recentProof.run); + const remove = useRemoveManagedProfileGuardian(recentProof.run); const [identifier, setIdentifier] = useState(""); const [expiresOn, setExpiresOn] = useState(""); @@ -116,6 +126,7 @@ export function ManagedProfileGuardians({ return (
+ {recentProof.dialog}

{t("recordSharing.managed.guardiansTitle", { name: profileName })}

@@ -129,7 +140,9 @@ export function ManagedProfileGuardians({ // one is the caller's own — which the route refuses anyway. const removable = !isSelf; const failed = - remove.isError && remove.variables?.grantId === guardian.grantId; + remove.isError && + !(remove.error instanceof ReproofCancelledError) && + remove.variables?.grantId === guardian.grantId; return (
  • - {invite.isError && ( + {invite.isError && !(invite.error instanceof ReproofCancelledError) && (

    + {recentProof.dialog}

    {t("recordSharing.managed.handover.description", { name: profileName, @@ -359,7 +371,7 @@ function HandoverProposalForm({ {replacing &&

    {t("recordSharing.managed.handover.replaceNote")}

    }
    • - {create.isError && ( + {create.isError && !(create.error instanceof ReproofCancelledError) && (

    (null); @@ -68,11 +76,18 @@ function DataResetCard() { setMsg(null); setMsgType(null); try { - const res = await apiFetchRaw("/api/settings/data", { - method: "DELETE", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ confirm: "DELETE" }), - }); + // An account with a second factor wipes its record only with that + // factor proved within five minutes; the shared dialog collects it when + // the server asks and the wipe is sent again. + const res = await recentProof.run(async () => + throwIfReproofRequired( + await apiFetchRaw("/api/settings/data", { + method: "DELETE", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ confirm: "DELETE" }), + }), + ), + ); const json = await res.json(); if (!res.ok) { setMsg(json.error || t("settings.dangerZoneDeleteFailed")); @@ -84,8 +99,13 @@ function DataResetCard() { setMsg(t("settings.dangerZoneSuccess")); setMsgType("success"); setConfirmOpen(false); - } catch { - setMsg(t("settings.dangerZoneDeleteFailed")); + } catch (err) { + const message = recentProofErrorMessage( + err, + t("settings.dangerZoneDeleteFailed"), + ); + if (message === null) return; + setMsg(message); setMsgType("error"); } finally { setDeleting(false); @@ -94,6 +114,7 @@ function DataResetCard() { return ( + {recentProof.dialog} {/* v1.4.43 QoL (L5) — dropped the `AlertTriangle` icon and neutralised the title colour so the danger-zone shaping is GitHub-style (red CTA only) rather than red-on-red-on-red. @@ -193,6 +214,9 @@ function DataResetCard() { */ function AccountDeleteCard() { const { t } = useTranslations(); + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); const [deleting, setDeleting] = useState(false); const [confirmOpen, setConfirmOpen] = useState(false); const [msg, setMsg] = useState(null); @@ -204,11 +228,17 @@ function AccountDeleteCard() { setMsg(null); setMsgType(null); try { - const res = await apiFetchRaw("/api/settings/account", { - method: "DELETE", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ confirm: "DELETE_ACCOUNT" }), - }); + // Same gate as the wipe: a second factor proved within five minutes on + // an account that has one, collected by the shared dialog. + const res = await recentProof.run(async () => + throwIfReproofRequired( + await apiFetchRaw("/api/settings/account", { + method: "DELETE", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ confirm: "DELETE_ACCOUNT" }), + }), + ), + ); if (!res.ok) { try { const json = await res.json(); @@ -229,15 +259,21 @@ function AccountDeleteCard() { setTimeout(() => { loadDocument("/auth/login"); }, 1_500); - } catch { - setMsg(t("settings.deleteAccountFailed")); - setMsgType("error"); + } catch (err) { setDeleting(false); + const message = recentProofErrorMessage( + err, + t("settings.deleteAccountFailed"), + ); + if (message === null) return; + setMsg(message); + setMsgType("error"); } } return ( + {recentProof.dialog} (null); const [msgType, setMsgType] = useState<"success" | "error" | null>(null); const [dialogOpen, setDialogOpen] = useState(false); + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); async function handleChangePassword(e: React.FormEvent) { e.preventDefault(); @@ -55,15 +63,23 @@ export function PasswordCard() { } try { - const res = await apiFetchRaw("/api/auth/password", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - currentPassword, - newPassword, - confirmPassword, - }), + // An account with a second factor changes its password only with that + // factor proved within five minutes; the shared dialog collects it when + // the server asks and the change is sent again. + const body = JSON.stringify({ + currentPassword, + newPassword, + confirmPassword, }); + const res = await recentProof.run(async () => + throwIfReproofRequired( + await apiFetchRaw("/api/auth/password", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body, + }), + ), + ); const json = await res.json(); if (!res.ok) { @@ -79,8 +95,10 @@ export function PasswordCard() { setCurrentPassword(""); setNewPassword(""); setConfirmPassword(""); - } catch { - setMsg({ key: "common.networkError" }); + } catch (err) { + const message = recentProofErrorMessage(err, ""); + if (message === null) return; + setMsg(message ? { text: message } : { key: "common.networkError" }); setMsgType("error"); } finally { setSaving(false); @@ -89,6 +107,7 @@ export function PasswordCard() { return ( <> + {recentProof.dialog} {/* The action is this card's only one, so it sits in the card's action row like every other primary. It stays full-width under `sm` — the German "Passwort ändern" is long enough that a right-hugging button diff --git a/src/components/settings/security-section/__tests__/passkey-removal-step-up.test.tsx b/src/components/settings/security-section/__tests__/passkey-removal-step-up.test.tsx index af14ec769..039bd96d7 100644 --- a/src/components/settings/security-section/__tests__/passkey-removal-step-up.test.tsx +++ b/src/components/settings/security-section/__tests__/passkey-removal-step-up.test.tsx @@ -15,11 +15,12 @@ * 3. an ordinary failure still shows its own message rather than being * swallowed by the step-up sentence, * 4. the sentence resolves in more than one locale, - * 5. the sentence names the recovery the web actually has. The mint endpoint - * is Bearer-only by design, so there is no in-page re-proof dialog for a - * cookie session; signing in again is the recovery, and a passkey login - * stamps `mfaVerifiedAt`. If that ever changes, this assertion is the - * reminder that the copy has to change with it. + * 5. the sentence names a recovery that always exists. A refusal that names + * its proofs (`meta.methods`) opens the re-proof dialog instead and never + * reaches this sentence (`useRecentProof`, pinned by + * `step-up-web-caller-guard.test.ts`); what is left is a refusal the + * dialog cannot take up, and signing in again is the recovery there — a + * passkey login stamps `mfaVerifiedAt`. * * Mutation check: return `err.message` from the removal's `onError` and cases 1, * 2 and 4 go red; narrow `describeStepUp` to the exact `auth.stepup.required` @@ -85,9 +86,9 @@ describe("passkey removal — the step-up refusal a person sees", () => { }); it("names the recovery the web actually has", () => { - // Not "verify to continue": there is no in-page re-proof for a cookie - // session, because the elevation mint is Bearer-only by design. Signing in - // again is the recovery, and a passkey login stamps the session. + // The dialog handles every refusal that names its proofs; this sentence + // is for the rest, where signing in again is the recovery and a passkey + // login stamps the session. expect(STEP_UP_SENTENCE.toLowerCase()).toContain("sign in again"); }); diff --git a/src/components/settings/security-section/existing-factor-reauth-dialog.tsx b/src/components/settings/security-section/existing-factor-reauth-dialog.tsx index 553fba98f..63b7234f1 100644 --- a/src/components/settings/security-section/existing-factor-reauth-dialog.tsx +++ b/src/components/settings/security-section/existing-factor-reauth-dialog.tsx @@ -56,6 +56,22 @@ const REAUTH_METHODS: readonly ReauthMethod[] = [ * none. On an account with a second factor the server leaves the password out, * so the dialog does not offer a choice that can only be refused. */ +/** + * The refusal says the account holds nothing it could re-prove here: an + * account that signs in through an identity provider only, with no password, + * passkey or second factor. A dialog would only offer it a password it does + * not have, so the callers say "sign in again" instead — a fresh sign-in is + * the proof such an account can give (`Session.createdAt` within five + * minutes). + */ +export function nothingToReprove(err: unknown): boolean { + return ( + err instanceof ApiError && + Array.isArray(err.meta?.methods) && + err.meta.methods.length === 0 + ); +} + export function offeredReauthMethods(err: unknown): ReauthMethod[] | null { if (!(err instanceof ApiError)) return null; const raw = err.meta?.methods; diff --git a/src/components/settings/security-section/passkey-list-section.tsx b/src/components/settings/security-section/passkey-list-section.tsx index 0ffff5ca6..5fd27cef5 100644 --- a/src/components/settings/security-section/passkey-list-section.tsx +++ b/src/components/settings/security-section/passkey-list-section.tsx @@ -43,6 +43,7 @@ import { } from "@/lib/api/api-fetch"; import { describePasskeyError } from "@/lib/passkey-errors"; import { describeStepUp } from "./security-keys-card"; +import { ReproofCancelledError, useRecentProof } from "./use-recent-proof"; interface PasskeyInfo { id: string; @@ -164,22 +165,30 @@ export function PasskeyListSection({ onError: (err: Error) => setMsg(err.message), }); + // Removal is step-up gated: a second factor, or on a passkey-only account a + // passkey, proved within five minutes. The server names what clears it and + // the shared dialog collects that, then the removal runs again. + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); const remove = useMutation({ mutationFn: async (id: string) => { - await apiDelete(`/api/auth/passkeys/${id}`); + await recentProof.run(() => apiDelete(`/api/auth/passkeys/${id}`)); }, onSuccess: () => { setMsg(null); queryClient.invalidateQueries({ queryKey: queryKeys.passkeys() }); }, - // Removal is step-up gated. The server's own prose says "second-factor", - // which is wrong for the passkey-only account this card mostly serves — the - // credential to re-prove is the passkey — so the refusal gets its own - // sentence naming the recovery that exists on the web: sign in again. - onError: (err: Error) => + // A refusal the dialog could not take up keeps its own sentence. The + // server's prose says "second-factor", which is wrong for the passkey-only + // account this card mostly serves, so it names the recovery that always + // exists on the web: sign in again. + onError: (err: Error) => { + if (err instanceof ReproofCancelledError) return; setMsg( describeStepUp(err, err.message, t("settings.passkeyStepUpRequired")), - ), + ); + }, }); const DEVICE_TYPE_LABELS: Record = { @@ -189,6 +198,7 @@ export function PasskeyListSection({ return ( + {recentProof.dialog} + KNOWN_METHODS.includes(m as ReauthMethod), + ); +} + +/** + * A refusal the re-proof dialog can clear. + * + * Two gates answer this way. The recent-proof gate (`auth.reproof.required`) + * in front of exports, share links, tokens, invitations and the admin data + * actions; and the fresh-factor gate (`auth.stepup.required`, from + * `requireFreshMfa`) in front of the managed-profile acts, the second-factor + * changes, passkey removal, the password change, account deletion, the data + * wipe and the key rotation. The second one names the proofs it would take in + * `meta.methods` on a browser session, and a `POST /api/auth/reproof` with one + * of them stamps the session the way signing in with the factor does, so the + * same dialog clears both. A fresh-factor refusal without `methods` (an + * account with no factor at all says `auth.stepup.mfa_not_enrolled` instead) + * is not something a dialog can answer, and stays with the caller's own + * message. + */ +export function isProofRefusal(err: unknown): boolean { + if (isReproofRequired(err)) return true; + return ( + err instanceof ApiError && + err.status === 401 && + err.meta?.errorCode === "auth.stepup.required" && + methodsFrom(err).length > 0 + ); +} + +/** The person closed the re-proof dialog. Nothing ran; show nothing. */ +export class ReproofCancelledError extends Error { + constructor() { + super("Re-proof cancelled"); + this.name = "ReproofCancelledError"; + } +} + +export interface RecentProofFlowView { + /** Open the dialog offering these proofs, or close it with `null`. */ + setMethods: (methods: ReauthMethod[] | null) => void; + setPending: (pending: boolean) => void; + setError: (error: string | null) => void; +} + +export interface RecentProofFlow { + run: (action: () => Promise) => Promise; + /** The dialog collected a proof: post it and release the waiting action. */ + prove: (proof: ExistingFactorProof) => Promise; + /** The dialog was closed without a proof. */ + cancel: () => void; +} + +export function createRecentProofFlow(deps: { + view: RecentProofFlowView; + /** Posts the proof to `/api/auth/reproof`. */ + submit: (proof: ExistingFactorProof) => Promise; + /** Read at the moment they are needed, so a locale switch is honoured. */ + messages: () => { signInAgain: string; failed: string }; +}): RecentProofFlow { + let waiter: { resolve: () => void; reject: () => void } | null = null; + + const run = async (action: () => Promise): Promise => { + try { + return await action(); + } catch (err) { + if (!isProofRefusal(err)) throw err; + const offered = methodsFrom(err as ApiError); + // Nothing to re-prove in place: an account that signs in through an + // identity provider only. A fresh sign-in is the proof it can give. + if (offered.length === 0) { + throw new ApiError(deps.messages().signInAgain, 401, { + errorCode: "auth.reproof.sign_in_again", + }); + } + await new Promise((resolve, reject) => { + waiter = { + resolve, + reject: () => reject(new ReproofCancelledError()), + }; + deps.view.setError(null); + deps.view.setMethods(offered); + }); + // Once. A second refusal is the answer, not a reason to ask again. + return action(); + } + }; + + const prove = async (proof: ExistingFactorProof): Promise => { + deps.view.setPending(true); + deps.view.setError(null); + try { + await deps.submit(proof); + deps.view.setMethods(null); + const w = waiter; + waiter = null; + w?.resolve(); + } catch (err) { + deps.view.setError( + err instanceof ApiError && err.status === 429 + ? err.message + : deps.messages().failed, + ); + } finally { + deps.view.setPending(false); + } + }; + + const cancel = () => { + deps.view.setMethods(null); + deps.view.setError(null); + const w = waiter; + waiter = null; + w?.reject(); + }; + + return { run, prove, cancel }; +} diff --git a/src/components/settings/security-section/security-keys-card.tsx b/src/components/settings/security-section/security-keys-card.tsx index 03d885c66..9a520fe82 100644 --- a/src/components/settings/security-section/security-keys-card.tsx +++ b/src/components/settings/security-section/security-keys-card.tsx @@ -37,10 +37,12 @@ import { formatDate } from "@/lib/format"; import { ExistingFactorReauthDialog, isReproofRequired, + nothingToReprove, offeredReauthMethods, type ExistingFactorProof, type ReauthMethod, } from "./existing-factor-reauth-dialog"; +import { ReproofCancelledError, useRecentProof } from "./use-recent-proof"; export interface WebauthnKeyInfo { id: string; @@ -133,6 +135,11 @@ export function SecurityKeysCard({ queryClient.invalidateQueries({ queryKey: queryKeys.mfaStatus() }); }, onError: (err, proof) => { + if (isReproofRequired(err) && nothingToReprove(err)) { + setReauthOpen(false); + setError(t("settings.reproof.signInAgain")); + return; + } if (isReproofRequired(err)) { setOfferedMethods(offeredReauthMethods(err)); setError(null); @@ -172,26 +179,35 @@ export function SecurityKeysCard({ ), }); + // Removing a key needs the second factor proved within five minutes; the + // shared dialog collects it when the server asks, then the removal runs + // again. + const recentProof = useRecentProof({ + description: t("settings.reproof.descriptionChange"), + }); const remove = useMutation({ mutationFn: async (id: string) => { - await apiDelete(`/api/auth/me/mfa/webauthn/${id}`); + await recentProof.run(() => apiDelete(`/api/auth/me/mfa/webauthn/${id}`)); }, onSuccess: () => { setError(null); queryClient.invalidateQueries({ queryKey: queryKeys.mfaStatus() }); }, - onError: (err) => + onError: (err) => { + if (err instanceof ReproofCancelledError) return; setError( describeStepUp( err, t("settings.security.keys.removeFailed"), t("settings.security.stepUpRequired"), ), - ), + ); + }, }); return ( + {recentProof.dialog} apiGet("/api/auth/reproof")); + } catch (err) { + const message = recentProofErrorMessage( + err, + t("settings.security.totp.disableFailed"), + ); + if (message) setError(message); + return; + } + setDisableOpen(true); + } + function resetWizard() { setSetup(null); setQrDataUrl(null); @@ -154,6 +187,11 @@ export function TotpCard({ } }, onError: (err, proof) => { + if (isReproofRequired(err) && nothingToReprove(err)) { + setReauthOpen(false); + setError(t("settings.reproof.signInAgain")); + return; + } if (isReproofRequired(err)) { const offered = offeredReauthMethods(err); if (offered) setReauthMethods(offered); @@ -194,31 +232,37 @@ export function TotpCard({ const disable = useMutation({ mutationFn: async () => { - await apiPost("/api/auth/me/mfa/disable", { - code: disableCode, - method: disableMethod, - }); + await recentProof.run(() => + apiPost("/api/auth/me/mfa/disable", { + code: disableCode, + method: disableMethod, + }), + ); }, onSuccess: () => { setDisableCode(""); setFreshCodes(null); queryClient.invalidateQueries({ queryKey: queryKeys.mfaStatus() }); }, - onError: (err) => + onError: (err) => { + if (err instanceof ReproofCancelledError) return; setError( describeError( err, t("settings.security.totp.disableFailed"), t("settings.security.stepUpRequired"), ), - ), + ); + }, onSettled: () => setDisableOpen(false), }); const regenerate = useMutation({ mutationFn: async () => { - return apiPost<{ recoveryCodes: string[] }>( - "/api/auth/me/mfa/recovery-codes/regenerate", + return recentProof.run(() => + apiPost<{ recoveryCodes: string[] }>( + "/api/auth/me/mfa/recovery-codes/regenerate", + ), ); }, onSuccess: (data) => { @@ -226,14 +270,16 @@ export function TotpCard({ setFreshCodes(data.recoveryCodes); queryClient.invalidateQueries({ queryKey: queryKeys.mfaStatus() }); }, - onError: (err) => + onError: (err) => { + if (err instanceof ReproofCancelledError) return; setError( describeError( err, t("settings.security.recovery.regenFailed"), t("settings.security.stepUpRequired"), ), - ), + ); + }, onSettled: () => setRegenerateOpen(false), }); @@ -250,6 +296,7 @@ export function TotpCard({ return ( + {recentProof.dialog} - + { + if (open) void openDisable(); + else setDisableOpen(false); + }} + >