mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 13:52:20 +00:00
Let a passkey satisfy the passkey-removal step-up
The gate landed with a softer mode for accounts holding no second factor, on the premise that they could not clear it. That premise was wrong. A passkey is a possession factor here and the codebase already says so in three places: a primary passkey login stamps Session.mfaVerifiedAt, passkey sits in FRESH_FACTOR_METHODS so a passkey-proved elevation reaches the fresh-factor routes, and the enrolment policy counts a primary passkey as satisfying it outright. The only thing refusing such an account was the reachability pre-check inside requireFreshMfa, which counts second factors and nothing else. So the mode is gone and the route asks for a full freshFactor: true with proofSource: "any-possession" — a second knob that widens which credentials count as "could ever prove this", and changes nothing about the gate. A passkey-only account now gets a real gate with the re-provable auth.stepup.required rather than no gate at all, and the dead-end auth.stepup.mfa_not_enrolled is reserved for an account holding nothing. hasSecondFactorEnrolled is deliberately NOT widened. It answers a different question, and requireFreshMfaIfEnrolled asks it before deciding whether account deletion, the data reset, the password change, the encrypted export and key rotation gate at all — counting a passkey there would change five destructive actions in passing. The two predicates sit beside each other with docblocks saying why they differ, and collapsing them turns eight existing tests red across three files. Recovery is named rather than assumed. On Bearer: mint an elevation with method "passkey". On the web: sign in again, which is what stamps the session — there is no in-page re-proof for a cookie session because the mint endpoint is Bearer-only by design. That limit is written into the route docblock instead of left to be discovered, and the removal's refusal now renders its own sentence in all six locales rather than the server's English "second-factor" prose, which was wrong for the account this card mostly serves.
This commit is contained in:
1 parent
283699db63
commit
4e182862c5
17 files changed
+469
-147
No files matched your search
@@ -0,0 +1,110 @@
|
||||
/**
|
||||
* The removal's refusal has to say something true, in the person's language.
|
||||
*
|
||||
* `DELETE /api/auth/passkeys/{id}` is step-up gated now, and the server answers
|
||||
* 401 with `meta.errorCode: auth.stepup.required` and the prose "Recent
|
||||
* second-factor verification required". Rendering that prose would have been
|
||||
* wrong twice over: it is English regardless of the operator's locale, and for
|
||||
* the passkey-only account this card mostly serves there is no second factor —
|
||||
* the credential to re-prove is the passkey itself.
|
||||
*
|
||||
* So the card maps the refusal to its own sentence. Pinned here:
|
||||
* 1. a step-up refusal renders the card's sentence, not the server's prose,
|
||||
* 2. the mfa_not_enrolled arm maps too — both codes mean "the gate stopped
|
||||
* you", and handling only the first falls through to raw English,
|
||||
* 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.
|
||||
*
|
||||
* 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`
|
||||
* code and case 2 goes red; drop the fallback arm and case 3 goes red.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { resolve } from "node:path";
|
||||
|
||||
import { ApiError } from "@/lib/api/api-fetch";
|
||||
import { describeStepUp } from "../security-keys-card";
|
||||
import en from "@/../messages/en.json";
|
||||
import de from "@/../messages/de.json";
|
||||
|
||||
const STEP_UP_SENTENCE = en.settings.passkeyStepUpRequired;
|
||||
|
||||
function refusal(errorCode: string): ApiError {
|
||||
return new ApiError("Recent second-factor verification required", 401, {
|
||||
errorCode,
|
||||
});
|
||||
}
|
||||
|
||||
describe("passkey removal — the step-up refusal a person sees", () => {
|
||||
it("replaces the server's prose on a step-up refusal", () => {
|
||||
const shown = describeStepUp(
|
||||
refusal("auth.stepup.required"),
|
||||
"fallback",
|
||||
STEP_UP_SENTENCE,
|
||||
);
|
||||
|
||||
expect(shown).toBe(STEP_UP_SENTENCE);
|
||||
expect(shown).not.toContain("second-factor verification required");
|
||||
});
|
||||
|
||||
it("maps the not-enrolled arm too", () => {
|
||||
expect(
|
||||
describeStepUp(
|
||||
refusal("auth.stepup.mfa_not_enrolled"),
|
||||
"fallback",
|
||||
STEP_UP_SENTENCE,
|
||||
),
|
||||
).toBe(STEP_UP_SENTENCE);
|
||||
});
|
||||
|
||||
it("leaves an ordinary refusal saying its own thing", () => {
|
||||
const lastMethod = new ApiError(
|
||||
"Cannot delete — at least one authentication method must remain",
|
||||
400,
|
||||
);
|
||||
|
||||
expect(describeStepUp(lastMethod, "fallback", STEP_UP_SENTENCE)).toContain(
|
||||
"at least one authentication method",
|
||||
);
|
||||
});
|
||||
|
||||
it("carries the sentence in every locale the app offers", () => {
|
||||
// The EN↔DE pair is the smoke test; `i18n-locale-integrity.test.ts` holds
|
||||
// the other four to the same guarantee.
|
||||
expect(de.settings.passkeyStepUpRequired).toBeTruthy();
|
||||
expect(de.settings.passkeyStepUpRequired).not.toBe(
|
||||
en.settings.passkeyStepUpRequired,
|
||||
);
|
||||
});
|
||||
|
||||
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.
|
||||
expect(STEP_UP_SENTENCE.toLowerCase()).toContain("sign in again");
|
||||
});
|
||||
|
||||
it("routes the removal through the mapper rather than the raw message", () => {
|
||||
const source = readFileSync(
|
||||
resolve(__dirname, "../passkey-list-section.tsx"),
|
||||
"utf8",
|
||||
);
|
||||
// Bounded by the next top-level declaration rather than by the first
|
||||
// `});`, which lands inside `mutationFn` and would slice the handler away.
|
||||
const start = source.indexOf("const remove = useMutation");
|
||||
const end = source.indexOf("const DEVICE_TYPE_LABELS", start);
|
||||
expect(start, "the removal mutation was renamed").toBeGreaterThan(-1);
|
||||
expect(end, "the slice's end anchor is gone").toBeGreaterThan(start);
|
||||
const body = source.slice(start, end);
|
||||
|
||||
expect(body).toContain("describeStepUp");
|
||||
expect(body).toContain("settings.passkeyStepUpRequired");
|
||||
});
|
||||
});
|
||||
@@ -40,6 +40,7 @@ import {
|
||||
apiPost,
|
||||
} from "@/lib/api/api-fetch";
|
||||
import { describePasskeyError } from "@/lib/passkey-errors";
|
||||
import { describeStepUp } from "./security-keys-card";
|
||||
|
||||
interface PasskeyInfo {
|
||||
id: string;
|
||||
@@ -169,7 +170,14 @@ export function PasskeyListSection({
|
||||
setMsg(null);
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.passkeys() });
|
||||
},
|
||||
onError: (err: Error) => setMsg(err.message),
|
||||
// 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) =>
|
||||
setMsg(
|
||||
describeStepUp(err, err.message, t("settings.passkeyStepUpRequired")),
|
||||
),
|
||||
});
|
||||
|
||||
const DEVICE_TYPE_LABELS: Record<string, string> = {
|
||||
|
||||
@@ -41,7 +41,21 @@ export interface WebauthnKeyInfo {
|
||||
lastUsedAt: string | null;
|
||||
}
|
||||
|
||||
function describeStepUp(
|
||||
/**
|
||||
* Turn a refusal into a sentence, with the step-up arm told apart from the rest.
|
||||
*
|
||||
* Exported because the passkey card next door needs the identical mapping since
|
||||
* passkey removal became step-up gated too. The server's own prose there is
|
||||
* "Recent second-factor verification required", which is English-only and, for a
|
||||
* passkey-only account, not even accurate about which credential to re-prove —
|
||||
* so the caller supplies its own sentence and this only decides when to use it.
|
||||
*
|
||||
* Matched on `meta.errorCode`, and on the whole `auth.stepup` prefix rather than
|
||||
* one code: `auth.stepup.required` and `auth.stepup.mfa_not_enrolled` both mean
|
||||
* "the gate stopped you", and a card that handled only the first would fall
|
||||
* through to raw server prose on the second.
|
||||
*/
|
||||
export function describeStepUp(
|
||||
err: unknown,
|
||||
fallback: string,
|
||||
stepUpMsg: string,
|
||||
|
||||
Reference in new issue
Block a user