mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
fix(auth): count a passkey as the second factor on the managed-profile acts
An account whose only strong credential is a primary passkey was refused auth.stepup.mfa_not_enrolled on every action behind requireFreshMfa, so it could not create, edit, hand over or delete a managed profile or manage its guardians, although a passkey sign-in or re-proof already stamps the session the gate reads. requireFreshMfa now defaults to the any-possession proof source: a passkey makes the gate reachable, and a passkey re-proof through the dialog clears it. The refusal offers passkey in meta.methods, never the password. hasSecondFactorEnrolled keeps its narrow meaning on purpose. It decides whether the conditional gates apply at all (account deletion, the data reset, the password change, the export, offering manage access, the recent-proof branches and the native Bearer rules), so none of those change, and password sign-in still challenges only TOTP and security keys. The MFA-management routes keep the second-factor source. The decision and its limits are written beside FreshFactorProofSource. Copy, the OpenAPI description and the sharing and key-rotation docs say "second factor or passkey" where this gate applies. Refs #959
This commit is contained in:
1 parent
d9b6061d28
commit
792fc89a93
17 files changed
+381
-63
No files matched your search
@@ -600,7 +600,7 @@ describe("ending a managed profile", () => {
|
||||
"p1",
|
||||
);
|
||||
const html = renderGatedSection();
|
||||
expect(html).toContain("Confirm your second factor");
|
||||
expect(html).toContain("Confirm your second factor or passkey");
|
||||
expect(html).not.toContain("The profile could not be deleted");
|
||||
});
|
||||
|
||||
@@ -616,7 +616,7 @@ describe("ending a managed profile", () => {
|
||||
"p1",
|
||||
);
|
||||
const html = renderGatedSection();
|
||||
expect(html).toContain("does not have one yet");
|
||||
expect(html).toContain("has neither yet");
|
||||
expect(html).toContain("Security");
|
||||
});
|
||||
|
||||
|
||||
@@ -133,9 +133,10 @@ describe("requireFreshMfa", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects a user with neither TOTP nor a security key", async () => {
|
||||
it("rejects a user with neither TOTP nor a security key nor a passkey", async () => {
|
||||
mockSession({ ...MFA_USER, totpConfirmedAt: null });
|
||||
vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(0 as never);
|
||||
vi.mocked(prisma.passkey.count).mockResolvedValue(0 as never);
|
||||
await expect(requireFreshMfa(5 * 60)).rejects.toMatchObject({
|
||||
errorCode: "auth.stepup.mfa_not_enrolled",
|
||||
});
|
||||
@@ -163,21 +164,50 @@ describe("requireFreshMfa", () => {
|
||||
* parameter, and it changes nothing else: the freshness read below is the same
|
||||
* read either way.
|
||||
*
|
||||
* A primary passkey is deliberately outside the default set and inside
|
||||
* `"any-possession"`. It DOES stamp `mfaVerifiedAt` on login, so it can clear
|
||||
* the gate — but counting it by default would pull every passkey-holding
|
||||
* Since v1.43.3 a primary passkey counts by default: `requireFreshMfa`
|
||||
* defaults to `"any-possession"`, so a passkey-only account clears the
|
||||
* managed-profile acts with a passkey re-proof. It DOES stamp `mfaVerifiedAt`
|
||||
* on login and on re-proof, so it can clear the gate. What stays narrow is
|
||||
* `hasSecondFactorEnrolled`: widening THAT would pull every passkey-holding
|
||||
* account into `requireFreshMfaIfEnrolled`'s gate on account deletion, the
|
||||
* data reset, the password change, the encrypted export and key rotation,
|
||||
* none of which have ever applied to them. The sibling suite
|
||||
* `require-fresh-mfa-if-enrolled.test.ts` is what catches that if the two
|
||||
* predicates are ever collapsed into one.
|
||||
* data reset, the password change, the encrypted export and key rotation.
|
||||
* The sibling suite `require-fresh-mfa-if-enrolled.test.ts` is what catches
|
||||
* that if the two predicates are ever collapsed into one.
|
||||
*/
|
||||
it("admits a passkey-only account by default, with a fresh stamp", async () => {
|
||||
mockSession({ ...MFA_USER, totpConfirmedAt: null });
|
||||
vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(0 as never);
|
||||
vi.mocked(prisma.passkey.count).mockResolvedValue(1 as never);
|
||||
vi.mocked(prisma.session.findUnique).mockResolvedValue({
|
||||
mfaVerifiedAt: new Date(Date.now() - 60_000),
|
||||
} as never);
|
||||
|
||||
const ctx = await requireFreshMfa(MFA_STEP_UP_MAX_AGE_SECONDS);
|
||||
expect(ctx.user.id).toBe("user-1");
|
||||
});
|
||||
|
||||
it("asks a passkey-only account for its passkey by default when the stamp is stale", async () => {
|
||||
mockSession({ ...MFA_USER, totpConfirmedAt: null });
|
||||
vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(0 as never);
|
||||
vi.mocked(prisma.passkey.count).mockResolvedValue(1 as never);
|
||||
vi.mocked(prisma.session.findUnique).mockResolvedValue({
|
||||
mfaVerifiedAt: null,
|
||||
} as never);
|
||||
|
||||
await expect(requireFreshMfa(5 * 60)).rejects.toMatchObject({
|
||||
errorCode: "auth.stepup.required",
|
||||
extraMeta: { methods: ["passkey"] },
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses a passkey-only account when only a second factor counts", async () => {
|
||||
mockSession({ ...MFA_USER, totpConfirmedAt: null });
|
||||
vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(0 as never);
|
||||
vi.mocked(prisma.passkey.count).mockResolvedValue(1 as never);
|
||||
|
||||
await expect(requireFreshMfa(5 * 60)).rejects.toMatchObject({
|
||||
await expect(
|
||||
requireFreshMfa(5 * 60, "second-factor"),
|
||||
).rejects.toMatchObject({
|
||||
errorCode: "auth.stepup.mfa_not_enrolled",
|
||||
});
|
||||
});
|
||||
|
||||
+41
-7
@@ -1253,11 +1253,13 @@ export type FreshMfaContext = AuthContext & { mfaVerifiedAt: Date };
|
||||
* the route is protecting.
|
||||
*
|
||||
* `second-factor` — a confirmed TOTP secret or a registered second-factor
|
||||
* security key. The default, and the right question for the routes that MANAGE
|
||||
* a second factor: there is nothing there to protect if nothing is enrolled.
|
||||
* security key. The default of `requireMfaManagementAuth`, and the right
|
||||
* question for the routes that MANAGE a second factor: there is nothing there
|
||||
* to protect if nothing is enrolled.
|
||||
*
|
||||
* `any-possession` — the above, OR a registered PRIMARY passkey. The right
|
||||
* question for a route protecting the passkey itself, because a passkey is a
|
||||
* `any-possession` — the above, OR a registered PRIMARY passkey. The default
|
||||
* of `requireFreshMfa` since v1.43.3, and the right question for a route
|
||||
* protecting the passkey itself, because a passkey is a
|
||||
* possession factor in its own right and this codebase already treats it as
|
||||
* one in three places: `/api/auth/passkey/login-verify` stamps `mfaVerifiedAt`
|
||||
* on a passkey login (v1.23, M-review M1), `passkey` is in
|
||||
@@ -1270,6 +1272,37 @@ export type FreshMfaContext = AuthContext & { mfaVerifiedAt: Date };
|
||||
*/
|
||||
export type FreshFactorProofSource = "second-factor" | "any-possession";
|
||||
|
||||
/*
|
||||
* A PASSKEY COUNTS AS A SECOND FACTOR FOR THE UNCONDITIONAL STEP-UP GATE.
|
||||
*
|
||||
* Maintainer decision, v1.43.3. `requireFreshMfa` defaults to
|
||||
* `any-possession`, so an account whose only strong credential is a passkey
|
||||
* is no longer refused `mfa_not_enrolled` on the routes that call it directly
|
||||
* (creating, editing, deleting and handing over a managed profile, inviting
|
||||
* and removing a guardian, the admin key rotation). It clears the gate the
|
||||
* way every other account does: a passkey sign-in or a passkey re-proof at
|
||||
* `POST /api/auth/reproof` stamps `Session.mfaVerifiedAt`, and the gate reads
|
||||
* that stamp inside the same five minutes.
|
||||
*
|
||||
* The limits, each deliberate:
|
||||
*
|
||||
* - `hasSecondFactorEnrolled` keeps its narrow meaning. It decides whether
|
||||
* the CONDITIONAL gates (`requireFreshMfaIfEnrolled`,
|
||||
* `requireFreshMfaOrElevationIfEnrolled`, `requireRecentProof`'s branches,
|
||||
* the enrollment proofs) apply at all. Widening it would newly gate
|
||||
* account deletion, the data reset, the password change and the export
|
||||
* for every passkey holder, take the password re-proof away from them, and
|
||||
* change what the native app's Bearer requests need. None of that is part
|
||||
* of the decision.
|
||||
* - Password login is untouched. It asks for a second factor from
|
||||
* `totpConfirmedAt` and the security-key count only; a passkey-only
|
||||
* account signing in with its password is not challenged, as before.
|
||||
* - The MFA-management routes (`requireMfaManagementAuth`) still default to
|
||||
* `second-factor`: disabling TOTP or rotating recovery codes on an account
|
||||
* with no second factor has nothing to protect.
|
||||
* - Cookie only, as ever. The Bearer path is not reached from here.
|
||||
*/
|
||||
|
||||
/**
|
||||
* v1.23 — step-up gate. Passes only for a COOKIE session whose
|
||||
* `Session.mfaVerifiedAt` is within `maxAgeSeconds` AND whose user holds a
|
||||
@@ -1285,12 +1318,13 @@ export type FreshFactorProofSource = "second-factor" | "any-possession";
|
||||
* waves gate account deletion, key rotation, and passphrase export on it.
|
||||
*
|
||||
* @param proofSource which credentials count as "can prove a factor". Defaults
|
||||
* to the second-factor set every existing caller means; see
|
||||
* {@link FreshFactorProofSource}.
|
||||
* to `any-possession`, so a primary passkey counts (see the decision note
|
||||
* below {@link FreshFactorProofSource}); `requireMfaManagementAuth` passes
|
||||
* `second-factor` explicitly.
|
||||
*/
|
||||
export async function requireFreshMfa(
|
||||
maxAgeSeconds: number,
|
||||
proofSource: FreshFactorProofSource = "second-factor",
|
||||
proofSource: FreshFactorProofSource = "any-possession",
|
||||
): Promise<FreshMfaContext> {
|
||||
const sessionData = await getSession();
|
||||
if (!sessionData)
|
||||
|
||||
@@ -21,6 +21,13 @@ import { prisma } from "@/lib/db";
|
||||
* a behaviour change to five destructive actions, made in passing, for the
|
||||
* benefit of a sixth. `canProveFreshFactor` in `api-handler.ts` is where the wider question
|
||||
* lives.
|
||||
*
|
||||
* Since v1.43.3 a passkey DOES count as a second factor where the gate is
|
||||
* unconditional: `requireFreshMfa` defaults to `any-possession`, so a
|
||||
* passkey-only account clears the managed-profile acts with a passkey
|
||||
* re-proof instead of being told to enrol something. That decision was made
|
||||
* there and not here, on purpose; the note beside `FreshFactorProofSource`
|
||||
* lists what it does and does not change.
|
||||
*/
|
||||
export async function hasSecondFactorEnrolled(user: {
|
||||
id: string;
|
||||
|
||||
@@ -75,7 +75,7 @@ const accountGrantInvite = inviteGrantSchema.meta({
|
||||
const createManagedProfileRequest = createManagedProfileSchema.meta({
|
||||
id: "CreateManagedProfileRequest",
|
||||
description:
|
||||
"Create a health record for somebody who has no login of their own — a child, a dependent adult, an animal. The caller becomes its first Guardian in the same transaction, through an ordinary MANAGE grant with the new record as grantor, so the profile appears in `accountAccess.accounts` on the next `GET /api/auth/me` and is switched into like any other shared record. `displayName` is the only required text; `dateOfBirth` is an optional `YYYY-MM-DD` and is never synthesised from a year. `gender` is optional and takes the same three values `PATCH /api/auth/me` takes for an ordinary account (`MALE`, `FEMALE`, `OTHER`) or null; absent and null both mean not recorded, and the value is what the cycle module derives its default from, so a record created for a child who does not need it starts with it off. `locale` and `timezone` belong to the RECORD and not to the Guardian reading it: they decide how the profile's own reminders are worded and when its day starts. Strict — an unexpected field is a 422 rather than an ignored key. Cookie transport only and step-up gated (`requireFreshMfa`, unconditional): a Bearer caller cannot mint a credential-less person and a permanent management relationship, and an account with no second factor enrolled is refused rather than waved through. Rate-limited to ten an hour per caller.",
|
||||
"Create a health record for somebody who has no login of their own — a child, a dependent adult, an animal. The caller becomes its first Guardian in the same transaction, through an ordinary MANAGE grant with the new record as grantor, so the profile appears in `accountAccess.accounts` on the next `GET /api/auth/me` and is switched into like any other shared record. `displayName` is the only required text; `dateOfBirth` is an optional `YYYY-MM-DD` and is never synthesised from a year. `gender` is optional and takes the same three values `PATCH /api/auth/me` takes for an ordinary account (`MALE`, `FEMALE`, `OTHER`) or null; absent and null both mean not recorded, and the value is what the cycle module derives its default from, so a record created for a child who does not need it starts with it off. `locale` and `timezone` belong to the RECORD and not to the Guardian reading it: they decide how the profile's own reminders are worded and when its day starts. Strict — an unexpected field is a 422 rather than an ignored key. Cookie transport only and step-up gated (`requireFreshMfa`, unconditional): a Bearer caller cannot mint a credential-less person and a permanent management relationship, and an account with no second factor enrolled is refused rather than waved through. A primary passkey counts as that factor here: a passkey sign-in or a passkey re-proof at POST /api/auth/reproof stamps the session. Rate-limited to ten an hour per caller.",
|
||||
});
|
||||
|
||||
const updateManagedProfileRequest = updateManagedProfileSchema.meta({
|
||||
@@ -1089,7 +1089,7 @@ export const accountSharingPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
"401": {
|
||||
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.",
|
||||
"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 with neither a second factor nor a passkey cannot create a managed profile until it enrols one. A passkey counts: an account whose only strong credential is a passkey is offered `passkey` in `meta.methods`. The ordinary unauthenticated 401 arrives here too, including for a Bearer caller, whose transport this gate does not read.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
"429": {
|
||||
|
||||
Reference in new issue
Block a user