Gate passkey removal on the same step-up as second-factor removal

DELETE /api/auth/passkeys/{id} took a plain cookie session or a wildcard
Bearer, while DELETE /api/auth/me/mfa/webauthn/{id} beside it demanded a
fresh factor proof. The softer gate sat on the primary sign-in credential:
the last-method check stops an account locking itself out, but nothing
stopped a hijacked session stripping the credentials the owner signs in
with.

The route now goes through requireMfaManagementAuth, the same gate the
security-key removal uses, in a new "if-enrolled" mode. An account with a
second factor pays the full price on either transport — a freshly verified
cookie session, or a factor-proved X-Step-Up elevation on Bearer, so the
native client keeps the capability rather than being sent to the website.
An account with no second factor keeps the contract it had, because a gate
it can never satisfy is not a safeguard but a locked door in front of its
own credential list.

The elevation is spent immediately before the delete, so a foreign id or a
refused last credential does not burn a proof that then has to be minted
again. The route is added to the elevation allowlist with its reason, and
the either-factor enrolment test that was written out three times in the
API kit is now one function.
This commit is contained in:
Marc-André Bombeck committed 2026-08-23 16:55:59 +02:00
1 parent fffdc83ca4
commit e3be71f0e5
7 files changed
+392 -42

No files matched your search

+19 -4
View File
@@ -2581,9 +2581,17 @@ paths:
the account has no password — an account must keep at least one way in.
No step-up: this endpoint takes a plain cookie session or a wildcard Bearer token, where removing a
second-factor security key (DELETE /api/auth/me/mfa/webauthn/{id}) requires a fresh factor proof. The asymmetry
is in the code as it stands, not a description of intent.
Step-up gated for accounts that can clear the gate, on the same mechanism as removing a second-factor security
key (DELETE /api/auth/me/mfa/webauthn/{id}). An account with a second factor enrolled — a confirmed TOTP secret
or a registered security key — must present a fresh factor proof: a cookie session that completed one within the
last five minutes, or, on Bearer, an `X-Step-Up` elevation minted at POST /api/auth/step-up against a re-proved
factor (a password-proved elevation is NOT enough). An account with no second factor enrolled is unaffected and
keeps the plain session-or-token contract, because a gate it could never satisfy would lock it out of its own
credential list.
The elevation is spent only when the deletion is about to happen. A 404 for an unknown id, or the
last-credential refusal, leaves it unconsumed and reusable.
parameters:
- in: path
name: id
@@ -2604,7 +2612,14 @@ paths:
application/json:
schema:
$ref: "#/components/schemas/ErrorEnvelope"
"401": *a1
"401":
description: Not authenticated, or the account has a second factor and the request carried no proof of it fresh enough
(`meta.errorCode` = `auth.stepup.required`). Re-prove the factor and retry — on the web by re-verifying, on
a native client by minting an elevation at POST /api/auth/step-up. Nothing was removed.
content:
application/json:
schema:
$ref: "#/components/schemas/ErrorEnvelope"
"404":
description: No such passkey for this account.
content:
+17 -2
View File
@@ -87,8 +87,22 @@ function callers(exportName: string, moduleRe: RegExp): string[] {
}
/**
* Every route an elevation can unlock. This is the whole set — the second-factor
* management MUTATIONS and nothing besides.
* 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 `freshFactor: "if-enrolled"`, so an account
* with a second factor pays the full price and an account without one keeps the
* contract it had rather than losing the ability to remove its own credential.
*
* 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
@@ -108,6 +122,7 @@ function callers(exportName: string, moduleRe: RegExp): string[] {
* exists to prevent.
*/
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",
@@ -16,6 +16,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
vi.mock("@/lib/api-handler", () => ({
apiHandler: (fn: unknown) => fn,
requireAuth: vi.fn(),
requireMfaManagementAuth: vi.fn(),
}));
vi.mock("@/lib/db", () => ({
@@ -35,8 +36,8 @@ vi.mock("@/lib/auth/audit", () => ({
auditLog: vi.fn().mockResolvedValue(undefined),
}));
import { PATCH } from "../route";
import { requireAuth } from "@/lib/api-handler";
import { DELETE, PATCH } from "../route";
import { requireAuth, requireMfaManagementAuth } from "@/lib/api-handler";
import { prisma } from "@/lib/db";
const USER = { id: "user-1", username: "u", passwordHash: "argon2id$…" };
@@ -70,9 +71,18 @@ type Handler = (
ctx: { params: Promise<{ id: string }> },
) => Promise<Response>;
const commitElevation = vi.fn().mockResolvedValue(undefined);
beforeEach(() => {
vi.resetAllMocks();
vi.mocked(requireAuth).mockResolvedValue({ user: USER } as never);
commitElevation.mockClear().mockResolvedValue(undefined);
vi.mocked(requireMfaManagementAuth).mockResolvedValue({
transport: "cookie",
user: USER,
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
commitElevation,
} as never);
});
describe("PATCH /api/auth/passkeys/{id} — the rename refusal", () => {
@@ -155,3 +165,104 @@ describe("PATCH /api/auth/passkeys/{id} — the rename refusal", () => {
expect(prisma.passkey.update).not.toHaveBeenCalled();
});
});
/**
* The removal's gate, and what it must NOT be.
*
* A passkey is the primary sign-in credential and it came off a plain session,
* while the second-factor key next door demanded a fresh proof. The route now
* asks the same gate the second-factor removal asks — but with
* `freshFactor: "if-enrolled"`, not `true`. That distinction is the whole
* design: `true` refuses an account with no second factor outright, and most
* accounts holding a passkey have nothing else enrolled, so `true` would take
* their own credential list away from them.
*
* The gate itself is behavioural and lives against a real database in
* `tests/integration/step-up-elevation.test.ts` (E14). What is pinned here is
* what this file can prove without one: the route asks for the right mode, and
* it spends the proof only when it is actually about to delete.
*
* Mutation check: change the option to `true` and the first case goes red; move
* `commitElevation()` above the ownership check and the third goes red; drop it
* altogether and the second goes red.
*/
describe("DELETE /api/auth/passkeys/{id} — the removal's gate", () => {
const mkDelete = () =>
new Request("http://localhost/api/auth/passkeys/pk-1", {
method: "DELETE",
});
it("asks for the step-up gate in its if-enrolled mode", async () => {
vi.mocked(prisma.passkey.findUnique).mockResolvedValue({
id: "pk-1",
userId: USER.id,
name: "Phone",
} as never);
vi.mocked(prisma.passkey.count).mockResolvedValue(2 as never);
vi.mocked(prisma.passkey.delete).mockResolvedValue({} as never);
await (DELETE as unknown as Handler)(mkDelete(), { params });
expect(requireMfaManagementAuth).toHaveBeenCalledWith({
freshFactor: "if-enrolled",
});
// Not `true`: that arm refuses an account with no second factor, which is
// a lockout on this route rather than a safeguard.
expect(requireMfaManagementAuth).not.toHaveBeenCalledWith({
freshFactor: true,
});
});
it("spends the proof once, on the way to the delete", async () => {
vi.mocked(prisma.passkey.findUnique).mockResolvedValue({
id: "pk-1",
userId: USER.id,
name: "Phone",
} as never);
vi.mocked(prisma.passkey.count).mockResolvedValue(2 as never);
vi.mocked(prisma.passkey.delete).mockResolvedValue({} as never);
const res = await (DELETE as unknown as Handler)(mkDelete(), { params });
expect(res.status).toBe(200);
expect(commitElevation).toHaveBeenCalledTimes(1);
expect(prisma.passkey.delete).toHaveBeenCalledWith({
where: { id: "pk-1" },
});
});
it("does not spend the proof on another account's passkey id", async () => {
vi.mocked(prisma.passkey.findUnique).mockResolvedValue({
id: "pk-1",
userId: "someone-else",
name: "Not yours",
} as never);
const res = await (DELETE as unknown as Handler)(mkDelete(), { params });
expect(res.status).toBe(404);
expect(commitElevation).not.toHaveBeenCalled();
expect(prisma.passkey.delete).not.toHaveBeenCalled();
});
it("does not spend the proof when the last credential is refused", async () => {
vi.mocked(prisma.passkey.findUnique).mockResolvedValue({
id: "pk-1",
userId: USER.id,
name: "Only one",
} as never);
vi.mocked(prisma.passkey.count).mockResolvedValue(1 as never);
vi.mocked(requireMfaManagementAuth).mockResolvedValue({
transport: "cookie",
user: { ...USER, passwordHash: null },
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
commitElevation,
} as never);
const res = await (DELETE as unknown as Handler)(mkDelete(), { params });
expect(res.status).toBe(400);
expect(commitElevation).not.toHaveBeenCalled();
expect(prisma.passkey.delete).not.toHaveBeenCalled();
});
});
+40 -2
View File
@@ -8,7 +8,11 @@ import {
safeJson,
} from "@/lib/api-response";
import { NextRequest } from "next/server";
import { apiHandler, requireAuth } from "@/lib/api-handler";
import {
apiHandler,
requireAuth,
requireMfaManagementAuth,
} from "@/lib/api-handler";
import { annotate } from "@/lib/logging/context";
import { passkeyRenameSchema } from "@/lib/validations/auth";
@@ -61,12 +65,40 @@ export const PATCH = apiHandler(
},
);
/**
* Remove a passkey — step-up gated, on the same mechanism the second-factor
* security-key removal uses.
*
* The two deletions were not equals. Removing a registered SECOND factor
* demanded a fresh factor proof; removing a passkey, which is the PRIMARY
* sign-in credential, took a plain cookie session or a wildcard Bearer. The
* last-method check below stops an account locking itself out, but it does
* nothing about a hijacked session quietly stripping the credentials the owner
* signs in with. So this route goes through `requireMfaManagementAuth` too.
*
* Two consequences worth stating rather than discovering:
*
* BEARER IS NOT REFUSED. `requireFreshMfa` is cookie-only, so a gate built on
* it alone would have told the native client to go and use the website. The
* gate this route borrows has a Bearer arm — a step-up elevation minted
* against a re-proved factor, exactly what the app already does to remove a
* security key — so the app keeps the capability and pays the same price the
* web pays.
*
* AN ACCOUNT WITH NO SECOND FACTOR IS NOT LOCKED OUT. `freshFactor: true`
* refuses those outright, which is right for a route that manages a second
* factor and wrong here: most accounts with a passkey have nothing else
* enrolled, and they would lose the ability to remove their own credential.
* `"if-enrolled"` gates the accounts that can clear the gate and leaves the
* rest exactly where they were.
*/
export const DELETE = apiHandler(
async (
request: NextRequest,
{ params }: { params: Promise<{ id: string }> },
) => {
const { user } = await requireAuth();
const auth = await requireMfaManagementAuth({ freshFactor: "if-enrolled" });
const { user } = auth;
const { id } = await params;
@@ -91,6 +123,12 @@ export const DELETE = apiHandler(
);
}
// Ownership resolved and the last-method check passed — spend the elevation
// now, so a 404 for someone else's id or a refused last credential does not
// burn a proof the caller then has to mint again. Same ordering as the
// security-key removal.
await auth.commitElevation();
await prisma.passkey.delete({ where: { id } });
await auditLog("auth.passkey.delete", {
+64 -31
View File
@@ -1080,13 +1080,8 @@ export async function requireFreshMfa(
// unreachable for it by design (the management UI gates enrolment first).
// Either factor counts: a confirmed TOTP secret OR a registered WebAuthn
// security key — both stamp `Session.mfaVerifiedAt` on a completed login.
if (!sessionData.user.totpConfirmedAt) {
const webauthnKeyCount = await prisma.webauthnMfaCredential.count({
where: { userId: sessionData.user.id },
});
if (webauthnKeyCount === 0) {
throw new StepUpRequiredError("auth.stepup.mfa_not_enrolled");
}
if (!(await hasSecondFactorEnrolled(sessionData.user))) {
throw new StepUpRequiredError("auth.stepup.mfa_not_enrolled");
}
// Read the freshness stamp off the live session row — `getSession`'s
@@ -1125,22 +1120,33 @@ export async function requireFreshMfaIfEnrolled(
maxAgeSeconds: number,
): Promise<AuthContext> {
const auth = await requireAuth();
// Either factor enrols the account: a confirmed TOTP secret OR a registered
// WebAuthn security key. A webauthn-only user must clear step-up too, so the
// destructive-action boundary tracks `requireFreshMfa`'s either-factor rule.
let enrolled = Boolean(auth.user.totpConfirmedAt);
if (!enrolled) {
const webauthnKeyCount = await prisma.webauthnMfaCredential.count({
where: { userId: auth.user.id },
});
enrolled = webauthnKeyCount > 0;
}
if (enrolled) {
if (await hasSecondFactorEnrolled(auth.user)) {
await requireFreshMfa(maxAgeSeconds);
}
return auth;
}
/**
* Does this account have a second factor at all?
*
* Either factor enrols it: a confirmed TOTP secret OR a registered WebAuthn
* security key. A webauthn-only account must clear step-up too, so every
* boundary that asks this question tracks `requireFreshMfa`'s either-factor
* rule rather than reading `totpConfirmedAt` alone. It was written out three
* times before it was a function, which is two more places for the second arm
* to be forgotten.
*/
async function hasSecondFactorEnrolled(user: {
id: string;
totpConfirmedAt: Date | null;
}): Promise<boolean> {
if (user.totpConfirmedAt) return true;
const webauthnKeyCount = await prisma.webauthnMfaCredential.count({
where: { userId: user.id },
});
return webauthnKeyCount > 0;
}
/**
* A Bearer-resolved caller.
*
@@ -1269,16 +1275,31 @@ export type MfaManagementContext = {
* @param options.freshFactor mirrors the cookie path's `requireFreshMfa`. Set by
* the destructive routes (disable, recovery-code rotation, security-key
* removal).
*
* `"if-enrolled"` is the third value and it is the one that needs explaining.
* `true` refuses an account with no second factor outright, which is correct
* for the routes that MANAGE a second factor — there is nothing to remove if
* nothing is enrolled. Passkey removal is not like that: the passkey is the
* primary sign-in credential, most accounts holding one have no second factor
* beside it, and a gate they cannot satisfy would not be a safeguard but a
* locked door in front of their own credential list. So `"if-enrolled"`
* applies the full gate to the accounts that can clear it and leaves the rest
* on the plain `requireAuth()` contract they had, which is the same line
* `requireFreshMfaIfEnrolled` draws for the destructive account actions.
*/
export async function requireMfaManagementAuth(
options: { freshFactor?: boolean } = {},
options: { freshFactor?: boolean | "if-enrolled" } = {},
): Promise<MfaManagementContext> {
const freshFactor = options.freshFactor === true;
const freshFactorMode = options.freshFactor ?? false;
// Cookie first, and via the original helpers — the web path runs the same
// code it always did.
const sessionData = await getSession();
if (sessionData) {
const freshFactor =
freshFactorMode === "if-enrolled"
? await hasSecondFactorEnrolled(sessionData.user)
: freshFactorMode;
const resolved = freshFactor
? await requireFreshMfa(MFA_STEP_UP_MAX_AGE_SECONDS)
: await requireCookieAuth();
@@ -1294,6 +1315,24 @@ export async function requireMfaManagementAuth(
// token is refused here and never gets as far as presenting an elevation.
const auth = await requireBearerAuth();
// Resolved BEFORE the header is read, because on `"if-enrolled"` an account
// with no second factor needs no elevation and must not be asked for one —
// demanding a header it can never mint would be the lockout this mode exists
// to avoid, wearing a 401.
const freshFactor =
freshFactorMode === "if-enrolled"
? await hasSecondFactorEnrolled(auth.user)
: freshFactorMode;
if (freshFactorMode === "if-enrolled" && !freshFactor) {
return {
transport: "bearer",
user: auth.user,
apiTokenId: auth.apiTokenId,
accessTokenHash: auth.accessTokenHash,
commitElevation: async () => {},
};
}
let raw: string | null = null;
try {
const headerList = await headers();
@@ -1341,17 +1380,11 @@ export async function requireMfaManagementAuth(
// Parity with the cookie path: `requireFreshMfa` refuses an account with no
// second factor enrolled, because a step-up-gated action is meaningless there.
// The Bearer path holds the same line rather than becoming the softer route.
if (freshFactor) {
let enrolled = Boolean(auth.user.totpConfirmedAt);
if (!enrolled) {
const keys = await prisma.webauthnMfaCredential.count({
where: { userId: auth.user.id },
});
enrolled = keys > 0;
}
if (!enrolled) {
throw new StepUpRequiredError("auth.stepup.mfa_not_enrolled");
}
// Unreachable under `"if-enrolled"` — that mode returned above when the
// account had no factor, so arriving here with `freshFactor` true means the
// enrolment was already established.
if (freshFactor && !(await hasSecondFactorEnrolled(auth.user))) {
throw new StepUpRequiredError("auth.stepup.mfa_not_enrolled");
}
return {
+9 -1
View File
@@ -1529,7 +1529,8 @@ export const authPaths: NonNullable<ZodOpenApiObject["paths"]> = {
summary: "Remove a registered passkey",
description:
"Deletes one primary sign-in credential, scoped to the authenticated user. Refuses to remove the LAST one when the account has no password — an account must keep at least one way in.\n\n" +
"No step-up: this endpoint takes a plain cookie session or a wildcard Bearer token, where removing a second-factor security key (DELETE /api/auth/me/mfa/webauthn/{id}) requires a fresh factor proof. The asymmetry is in the code as it stands, not a description of intent.",
"Step-up gated for accounts that can clear the gate, on the same mechanism as removing a second-factor security key (DELETE /api/auth/me/mfa/webauthn/{id}). An account with a second factor enrolled — a confirmed TOTP secret or a registered security key — must present a fresh factor proof: a cookie session that completed one within the last five minutes, or, on Bearer, an `X-Step-Up` elevation minted at POST /api/auth/step-up against a re-proved factor (a password-proved elevation is NOT enough). An account with no second factor enrolled is unaffected and keeps the plain session-or-token contract, because a gate it could never satisfy would lock it out of its own credential list.\n\n" +
"The elevation is spent only when the deletion is about to happen. A 404 for an unknown id, or the last-credential refusal, leaves it unconsumed and reusable.",
requestParams: { path: z.object({ id: z.string() }) },
responses: {
"200": {
@@ -1553,6 +1554,13 @@ export const authPaths: NonNullable<ZodOpenApiObject["paths"]> = {
content: { "application/json": { schema: errorEnvelope } },
},
...stdResponses,
// After `stdResponses`, or the generic 401 there overwrites this one
// and the step-up contract disappears from the published operation.
"401": {
description:
"Not authenticated, or the account has a second factor and the request carried no proof of it fresh enough (`meta.errorCode` = `auth.stepup.required`). Re-prove the factor and retry — on the web by re-verifying, on a native client by minting an elevation at POST /api/auth/step-up. Nothing was removed.",
content: { "application/json": { schema: errorEnvelope } },
},
},
},
},
+130
View File
@@ -1156,3 +1156,133 @@ describe("E13 — the second-factor status read needs no elevation", () => {
expect(text).not.toContain("credentialId");
});
});
// ── E14 — passkey removal joins the gate, without locking anyone out ──
/**
* `DELETE /api/auth/passkeys/{id}` used to take a plain cookie session or a
* wildcard Bearer, while the second-factor key beside it demanded a fresh
* proof. That was the softer gate on the PRIMARY sign-in credential, so it now
* goes through `requireMfaManagementAuth({ freshFactor: "if-enrolled" })`.
*
* The mode is the part worth pinning behaviourally, because it is the part that
* can go wrong in two opposite directions and a status-only test would miss
* both:
*
* - gate everyone, and an account with no second factor — which is most
* accounts holding a passkey — can never remove its own credential again.
* That is a lockout dressed as a safeguard;
* - gate nobody, and the change did nothing.
*
* The Bearer arm is here for the same reason. `requireFreshMfa` is cookie-only,
* so a gate built on it alone would have answered the native client with "use
* the website". These cases prove the app keeps the capability by minting the
* same elevation it already mints to remove a security key.
*
* Mutation check: change the route's option to `true` and the two unenrolled
* cases go red; drop the option entirely and the two refusals go red; move
* `commitElevation()` above the 404 branch and the last case goes red.
*/
describe("E14 — removing a passkey is step-up gated for enrolled accounts", () => {
async function seedPasskey(label: string): Promise<string> {
const row = await getPrismaClient().passkey.create({
data: {
userId: USER_ID,
name: label,
credentialId: `cred-${label}`,
credentialPublicKey: Buffer.from([1, 2, 3]),
credentialDeviceType: "multiDevice",
transports: ["internal"],
},
select: { id: true },
});
return row.id;
}
async function callDelete(id: string): Promise<Response> {
const { DELETE } = await import("@/app/api/auth/passkeys/[id]/route");
return await DELETE(req(`/api/auth/passkeys/${id}`, "DELETE"), {
params: Promise.resolve({ id }),
});
}
async function useCookieSession(): Promise<string> {
const session = await getPrismaClient().session.create({
data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) },
});
cookieJar.set("healthlog_session", session.id);
return session.id;
}
it("lets a Bearer client with no second factor remove one, as before", async () => {
const passkeyId = await seedPasskey("e14a");
const { raw } = await mintToken("e14a");
useToken(raw);
useElevation(null);
expect((await callDelete(passkeyId)).status).toBe(200);
expect(await getPrismaClient().passkey.count()).toBe(0);
});
it("lets a cookie session with no second factor remove one, as before", async () => {
const passkeyId = await seedPasskey("e14b");
await useCookieSession();
expect((await callDelete(passkeyId)).status).toBe(200);
expect(await getPrismaClient().passkey.count()).toBe(0);
});
it("refuses a Bearer client on an enrolled account with no elevation", async () => {
const passkeyId = await seedPasskey("e14c");
await enrolTotp();
const { raw } = await mintToken("e14c");
useToken(raw);
useElevation(null);
expect((await callDelete(passkeyId)).status).toBe(401);
// The refusal is only worth anything if the row survived it.
expect(await getPrismaClient().passkey.count()).toBe(1);
});
it("refuses a cookie session on an enrolled account with no fresh factor", async () => {
const passkeyId = await seedPasskey("e14d");
await enrolTotp();
await useCookieSession();
expect((await callDelete(passkeyId)).status).toBe(401);
expect(await getPrismaClient().passkey.count()).toBe(1);
});
it("admits the native client on a factor-proved elevation, and spends it", async () => {
const passkeyId = await seedPasskey("e14e");
const secret = await enrolTotp();
const { raw, id: tokenId } = await mintToken("e14e");
useToken(raw);
const elevation = elevationOf(await mintTotpElevation(secret));
useElevation(elevation);
expect((await callDelete(passkeyId)).status).toBe(200);
expect(await getPrismaClient().passkey.count()).toBe(0);
const row = await getPrismaClient().stepUpElevation.findFirst({
where: { apiTokenId: tokenId },
});
expect(row?.consumedAt).not.toBeNull();
});
it("does not burn the elevation on a foreign passkey id", async () => {
await seedPasskey("e14f");
const secret = await enrolTotp();
const { raw, id: tokenId } = await mintToken("e14f");
useToken(raw);
const elevation = elevationOf(await mintTotpElevation(secret));
useElevation(elevation);
expect((await callDelete("pk-does-not-exist")).status).toBe(404);
const row = await getPrismaClient().stepUpElevation.findFirst({
where: { apiTokenId: tokenId },
});
expect(row?.consumedAt).toBeNull();
});
});