fix(auth): make Unlock Account release the second factor too (#3690)

`unlockUser` reset only `sys_user.failed_login_count` / `locked_until`. Sign-in
can lock at either stage and the two counters are independent, so a user locked
at the second factor had no admin escape hatch at all — the only way out was
waiting the duration out.

That was survivable while the second-factor lock needed better-auth's 10
failures. Now that the threshold is operator-configurable and 3 is a reasonable
choice, it is a lock admins will hit routinely, so the action has to cover it.

The second-factor clear is best-effort and runs after the primary write: an
account with no enrolment, or an environment where 2FA was never switched on,
must still get the unlock the admin asked for.

Covered by two unit tests (both enrolments cleared; a failing lookup does not
sink the unlock) and an end-to-end one that re-locks the account, unlocks it
through `/auth/admin/unlock-user`, and signs in again. The end-to-end test takes
its admin session BEFORE re-locking — after the lock there is no way to obtain
one, which is the whole reason the escape hatch has to exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYLC8TfjzHGwatNxZKdX7H

Author: claude

.changeset/two-factor-lockout-follows-settings.md

@@ -30,6 +30,13 @@ The threshold field is also no longer hidden behind `email_password_enabled` —
 two-factor verification exists in passwordless deployments, where the setting was
 previously unreachable.
 
+The admin **Unlock Account** action now clears both stages. It only ever reset
+`sys_user`, so a user locked at the second factor had no admin escape hatch and
+had to wait the duration out — survivable while that lock needed 10 failures,
+routine once an operator can set the threshold to 3. The second-factor clear is
+best-effort and runs after the primary write, so an account with no enrolment
+still unlocks normally.
+
 Note the plugin caps attempts at 5 per challenge (`beginAttempt(5)`), which no
 option reaches; a threshold above 5 forces a fresh challenge rather than raising
 that cap.

content/docs/permissions/administrator-guide.mdx

@@ -83,7 +83,8 @@ only on reorgs. Why the tree matters:
 
 Day-to-day lifecycle actions live on each user's row menu and record header:
 **Ban / Unban** (blocks sign-in), **Unlock Account** (clears a brute-force
-lockout early), **Set Password** (also mints one-time temporary passwords),
+lockout early, at both the password and two-factor stages), **Set Password**
+(also mints one-time temporary passwords),
 and **Impersonate User** (see [Step 4](#step-4--verify)). *(Rough edge today:
 users' own self-service password recovery depends on a configured email or
 SMS delivery service — wiring tracked in cloud#580. Until then, admin **Set

content/docs/permissions/authentication.mdx

@@ -456,13 +456,14 @@ code that reads it). Changes take effect immediately; no restart is required.
 | Setting | Effect |
 |---|---|
 | `lockout_threshold` | Lock an account after this many consecutive failed sign-in attempts. Counts **both** stages: wrong passwords and wrong two-factor codes. |
-| `lockout_duration_minutes` | How long a lockout lasts, at either stage. Admins can clear a password-stage lockout early with the **Unlock** action on the user record. |
+| `lockout_duration_minutes` | How long a lockout lasts, at either stage. Admins can clear it early with the **Unlock** action on the user record, which releases both stages. |
 | `rate_limit_max` / `rate_limit_window_seconds` | Per-IP request throttle on auth endpoints. |
 
 The two stages keep **separate counters** — `sys_user.failed_login_count` for the
 password check, `sys_two_factor.failed_verification_count` for the second factor —
 so a locked password stage and a locked second factor are independent states. Each
-counter resets on a success at its own stage.
+counter resets on a success at its own stage, and the admin **Unlock Account**
+action clears both at once.
 
 Setting `lockout_threshold` to `0` disables the **password-stage** lockout only.
 Two-factor verification keeps a built-in limit of 10 attempts per 15 minutes,

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

@@ -2472,6 +2472,46 @@ describe('AuthManager', () => {
       );
     });
 
+    // #3690 — sign-in can lock at either stage, and the counters are
+    // independent. Unlocking only `sys_user` left a 2FA-locked user with no
+    // admin escape hatch, which the now-configurable threshold makes routine.
+    it('unlockUser also clears the second factor\'s lock', async () => {
+      const engine = {
+        ...makeEngine({ id: 'u1' }),
+        find: vi.fn(async () => [{ id: 'tf1' }, { id: 'tf2' }]),
+      };
+      const m = mgr(engine);
+      await expect(m.unlockUser('u1')).resolves.toBe(true);
+      expect(engine.find).toHaveBeenCalledWith(
+        'sys_two_factor',
+        expect.objectContaining({ where: { user_id: 'u1' } }),
+      );
+      for (const id of ['tf1', 'tf2']) {
+        expect(engine.update).toHaveBeenCalledWith(
+          'sys_two_factor',
+          { id, failed_verification_count: 0, locked_until: null },
+          expect.anything(),
+        );
+      }
+    });
+
+    it('unlockUser still succeeds when the second-factor clear fails', async () => {
+      // No enrolment, a store that cannot serve the lookup, 2FA never switched
+      // on — none of that may turn the password-stage unlock the admin asked
+      // for into a failure.
+      const engine = {
+        ...makeEngine({ id: 'u1' }),
+        find: vi.fn(async () => { throw new Error('no such object: sys_two_factor'); }),
+      };
+      const m = mgr(engine);
+      await expect(m.unlockUser('u1')).resolves.toBe(true);
+      expect(engine.update).toHaveBeenCalledWith(
+        'sys_user',
+        { id: 'u1', failed_login_count: 0, locked_until: null },
+        expect.anything(),
+      );
+    });
+
     it('unlockUser returns false for an unknown user', async () => {
       const engine = makeEngine(null);
       const m = mgr(engine);

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

@@ -3961,6 +3961,14 @@ export class AuthManager {
    * ADR-0069 D2 — clear a user's lockout state (admin "Unlock" action).
    * Resets `failed_login_count` and `locked_until`. Returns false when no data
    * engine is wired or the user does not exist.
+   *
+   * [#3690] Clears BOTH stages. Sign-in can lock at the password check
+   * (`sys_user`) or at the second factor (`sys_two_factor`), and the two keep
+   * independent counters — so unlocking only the first left a 2FA-locked user
+   * with no admin escape hatch at all, waiting out the duration. That was
+   * survivable while the second factor sat on better-auth's 10-attempt default;
+   * with the threshold now operator-configurable (3 is a reasonable choice),
+   * it is a lock admins will hit routinely.
    */
   public async unlockUser(userId: string): Promise<boolean> {
     const engine = this.getDataEngine();
@@ -3975,6 +3983,25 @@ export class AuthManager {
       { id: userId, failed_login_count: 0, locked_until: null },
       { context: SYSTEM_CTX } as any,
     );
+    // Best-effort and deliberately after the primary write: a user with no
+    // enrolment (or an environment where 2FA was never switched on) must still
+    // get a successful unlock, and the password-stage clear is the part the
+    // admin asked for.
+    try {
+      const enrolments = await engine.find('sys_two_factor', {
+        where: { user_id: String(userId) }, fields: ['id'], context: SYSTEM_CTX,
+      } as any);
+      for (const row of (enrolments ?? []) as Array<{ id?: string }>) {
+        if (!row?.id) continue;
+        await engine.update(
+          'sys_two_factor',
+          { id: row.id, failed_verification_count: 0, locked_until: null },
+          { context: SYSTEM_CTX } as any,
+        );
+      }
+    } catch {
+      // Never turn a successful password-stage unlock into a failure.
+    }
     return true;
   }
 

packages/qa/dogfood/test/two-factor-lockout.dogfood.test.ts

@@ -116,6 +116,7 @@ describe('#3624 follow-up: better-auth 2FA lockout counts wrong codes', () => {
   let priorTwoFactor: string | undefined;
   let secret: Buffer;
   let adminEmail: string;
+  let adminUserId: string;
 
   beforeAll(async () => {
     // The two-factor plugin is opt-in (`twoFactor: … ?? false`), resolved once
@@ -140,7 +141,9 @@ describe('#3624 follow-up: better-auth 2FA lockout counts wrong codes', () => {
     const token = await stack.signIn();
     const me = await (await stack.apiAs(token, 'GET', '/auth/get-session')).json() as any;
     adminEmail = me?.user?.email;
+    adminUserId = String(me?.user?.id ?? '');
     expect(adminEmail, 'could not resolve the seeded admin email').toBeTruthy();
+    expect(adminUserId, 'could not resolve the seeded admin id').toBeTruthy();
 
     // Enrol. `enable` returns the otpauth:// URI carrying the base32 secret;
     // the stored copy is symmetrically encrypted, so this is the only place
@@ -334,4 +337,49 @@ describe('#3624 follow-up: better-auth 2FA lockout counts wrong codes', () => {
     expect(after.lockedUntil ?? null, 'the expired lock must be cleared, not merely ignored').toBeNull();
     expect(after.count, 'clearing an expired lock must reset the failure budget too').toBe(0);
   });
+
+  // [#3690] The admin escape hatch. `Unlock Account` used to clear only
+  // `sys_user`, so a user locked at the SECOND factor had no way out but to
+  // wait out the duration — tolerable while that lock needed 10 failures, much
+  // less so now that an operator can set the threshold to 3.
+  it('the admin unlock action releases a second-factor lock', async () => {
+    // A session that survives the lock. The one minted in beforeAll does not:
+    // completing enrolment rotates the session, so that bearer is stale by now.
+    // Take a fresh one through the full two-factor sign-in BEFORE re-locking —
+    // afterwards there is no way to obtain one, which is exactly why the admin
+    // escape hatch has to exist.
+    const bootstrap = await verifyWithCookie(await beginChallenge(), totp(secret));
+    expect(bootstrap.status, `sign-in for the admin session: ${await bootstrap.clone().text()}`).toBe(200);
+    const adminSession = ((await bootstrap.json()) as { token?: string }).token;
+    expect(adminSession, 'two-factor sign-in returned no session token').toBeTruthy();
+
+    // Re-lock: the previous test cleared everything.
+    let failures = 0;
+    while (failures < LOCKOUT_THRESHOLD) {
+      const cookie = await beginChallenge();
+      for (let i = 0; i < MAX_PER_CHALLENGE && failures < LOCKOUT_THRESHOLD; i++) {
+        await verifyWithCookie(cookie, '000000');
+        failures++;
+      }
+    }
+    const locked = await lockoutState();
+    expect(locked.lockedUntil ?? null, 'precondition: the account is locked again').not.toBeNull();
+    expect(locked.count).toBe(LOCKOUT_THRESHOLD);
+
+    // The lock gates VERIFICATION, not live sessions — so an already-signed-in
+    // admin can still act, which is what makes the escape hatch reachable.
+    const unlocked = await stack.apiAs(adminSession as string, 'POST', '/auth/admin/unlock-user', {
+      userId: adminUserId,
+    });
+    expect(unlocked.status, `unlock-user: ${await unlocked.clone().text()}`).toBe(200);
+
+    const cleared = await lockoutState();
+    expect(cleared.lockedUntil ?? null, 'unlock must clear the second factor, not just the password stage').toBeNull();
+    expect(cleared.count, 'unlock must reset the failure budget too — otherwise the next wrong code re-locks immediately').toBe(0);
+
+    // And the user can actually sign in again, which is the point of unlocking.
+    const cookie = await beginChallenge();
+    const ok = await verifyWithCookie(cookie, totp(secret));
+    expect(ok.status, `still locked out after unlock: ${await ok.clone().text()}`).toBe(200);
+  });
 });