Skip to content

Commit 45204a5

Browse files
os-warrenclaude
andauthored
fix(plugin-auth): 2FA re-enrollment must not inherit verified from the previous enrollment (#10994)
* fix(plugin-auth): 2FA re-enrollment must not inherit `verified` from the previous enrollment better-auth's `/two-factor/enable` writes `verified: existingTwoFactor?.verified === true` alongside a freshly generated secret, and `sys_two_factor` declares `user_id` unique — so re-enrolling rewrote the account's one row with a secret nobody had confirmed while carrying the prior enrollment's flag over. The flag stopped describing the stored secret, and the sign-in challenge honoured the replacement immediately. The vendor already gates that challenge on the flag (`TOTP_NOT_ENABLED` in `totp/index.mjs`, and `twoFactorMethods` in the post-sign-in hook) — it is what makes a first enrollment inert until confirmed. Re-enrollment slipped past it only because the value handed to the gate was inherited, so this restores the flag instead of adding a second owner of the same decision. Tightening only: same body, same response shape, same status; first-time enrollment unaffected; rotation still reachable, now via the same confirmation step a first enrollment takes. Part of #10700 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx * docs(auth): state that `/two-factor/enable` re-enrollment needs the same confirmation step The 2FA section already listed TOTP confirmation as part of a complete opt-in UX, but said nothing about what a SECOND `enable` does to an account that is already enrolled. Now it does: the replacement secret is unconfirmed, the sign-in challenge refuses it (`400 TOTP_NOT_ENABLED`) and omits `totp` from `twoFactorMethods` until the session-lane verify succeeds, the replaced secret stops working when `enable` returns, and the backup codes in that same response are the recovery path. Part of #10700 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cdaa72f commit 45204a5

5 files changed

Lines changed: 654 additions & 0 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
`POST /api/v1/auth/two-factor/enable` no longer leaves `sys_two_factor.verified`
6+
describing the enrollment *before* the secret it stores.
7+
8+
better-auth's enable handler computes the row it writes as
9+
`verified: existingTwoFactor != null && existingTwoFactor.verified === true`
10+
(measured on the installed 1.7.1, `dist/plugins/two-factor/index.mjs`), and
11+
`sys_two_factor` declares `user_id` unique — so a second `enable` on an account
12+
that already has a confirmed factor rewrites that one row with a brand-new
13+
secret while inheriting the old enrollment's flag. The flag then said
14+
"user-confirmed" about a secret nobody had ever confirmed, and the sign-in
15+
challenge honoured it.
16+
17+
The vendor already gates the challenge on that flag, in both places it matters:
18+
`totp/index.mjs` refuses an unconfirmed factor with `TOTP_NOT_ENABLED` before
19+
any lockout bookkeeping, and the post-sign-in hook offers `totp` among
20+
`twoFactorMethods` only when the flag is not `false`. That gate is exactly what
21+
a *first* enrollment relies on. Re-enrollment was the one path that slipped past
22+
it — not because the gate was missing, but because the value handed to it was
23+
inherited. So the fix restores the flag rather than adding a second gate:
24+
after a successful `method: 'totp'` enable, `verified` is set to `false`, and
25+
the freshly issued secret becomes live only once the caller proves possession of
26+
it through `/two-factor/verify-totp`.
27+
28+
This is a tightening. The request body, the response shape and the status are
29+
unchanged, a first-time enrollment is unaffected (better-auth already wrote
30+
`false` there), and a rotation is still reachable and still completes — it now
31+
takes the same confirmation step a first enrollment takes. What changes is that
32+
a secret the endpoint hands out is no longer accepted at the next sign-in until
33+
it has been confirmed. Clients that re-enroll and then rely on the new
34+
authenticator working immediately at sign-in must call `/two-factor/verify-totp`
35+
with the live session first, which is the flow first-time enrollment already
36+
uses.

content/docs/permissions/authentication.mdx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,18 @@ A complete opt-in 2FA UX still needs to handle:
603603
- the `twoFactorRedirect` response returned by password sign-in, and
604604
- backup-code recovery.
605605

606+
<Callout type="warn">
607+
**`/two-factor/enable` always issues a secret that must be confirmed — including
608+
on re-enrollment.** Calling it on an account that already has 2FA active replaces
609+
the stored secret and marks the enrollment unconfirmed, so the new secret is
610+
**not** accepted at the sign-in challenge (`400 TOTP_NOT_ENABLED`) and `totp` is
611+
not offered in `twoFactorMethods` until `/two-factor/verify-totp` succeeds with
612+
the live session. The replaced secret stops working as soon as `enable` returns,
613+
so a re-enrollment UI must run the confirmation step in the same session, and
614+
must show the backup codes from that response — they are the recovery path if
615+
the authenticator was never captured.
616+
</Callout>
617+
606618
For custom account UIs, enable the backend plugin in configuration:
607619

608620
```typescript

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
withBearerAdminSessionRecovery,
3939
} from './impersonation-bearer-rotation.js';
4040
import { echoInstalledSessionToken } from './two-factor-rotated-token-echo.js';
41+
import { resetVerifiedOnTwoFactorReenrollment } from './two-factor-reenrollment-verified-reset.js';
4142
import {
4243
applyPlatformAdminImpersonation,
4344
} from './admin-impersonate-endpoint.js';
@@ -1716,6 +1717,18 @@ export class AuthManager {
17161717
// corrects the echoed VALUE only; resolver precedence is untouched.
17171718
await echoInstalledSessionToken(ctx);
17181719

1720+
// ── #10700: `verified` must describe the secret stored beside it ──
1721+
// A second `/two-factor/enable` on an already-confirmed account
1722+
// rewrites the TOTP secret on the one `sys_two_factor` row the
1723+
// account has and INHERITS `verified` from the enrollment before it,
1724+
// so a secret nobody confirmed is honoured at the sign-in challenge.
1725+
// better-auth already gates that challenge on the flag — a first
1726+
// enrollment is inert until the session-lane verify flips it — so
1727+
// restoring the flag restores the gate rather than adding a second
1728+
// one. See `two-factor-reenrollment-verified-reset.ts`, whose header
1729+
// also states what this deliberately does NOT do.
1730+
await resetVerifiedOnTwoFactorReenrollment(ctx);
1731+
17191732
// ── ADR-0069 D2: account lockout (counter) ──────────────────
17201733
// better-auth catches an INVALID_EMAIL_OR_PASSWORD APIError and runs
17211734
// the after-hook with it on `ctx.context.returned`; a success leaves

0 commit comments

Comments
 (0)