Skip to content

Commit 43754d7

Browse files
os-zhuangclaude
andauthored
test(auth): cover the lock itself and its expiry, not just the counter (#3624 follow-up) (#3667)
#3659 pinned the two ends of the 2FA failure budget — a wrong code is counted, a correct one resets it. It stopped short of the transitions that make it a LOCKOUT rather than a counter, which is the part that actually protects the account. Three more assertions: 1. **The lock engages.** Spending the budget stamps `locked_until` in the future. Reaching it means respecting two stacked budgets: better-auth allows 5 verifications per two-factor cookie (`beginAttempt(5)`) and 10 consecutive failures per ACCOUNT (`accountLockout.maxFailedAttempts`). Only the second touches the columns under test, so the loop takes a fresh challenge every 5 tries. The counter is asserted after every single failure, not just at the end. 2. **The lock bites before the code is checked.** A locked account refuses even a VALID TOTP with 429. Without this the "lockout" would just be a slower guess loop. 3. **An expired lock clears itself.** better-auth clears it through a GUARDED `incrementOne` — `where locked_until <= now`, `set { failedVerificationCount: 0, lockedUntil: null }`. That is the only place the ObjectQL adapter has to handle `lte` against a datetime, so this covers the operator as much as the columns: a driver mishandling it would leave the user locked out forever with no error anywhere. The test ages the lock through the data engine because no endpoint can — the default duration is 900s. Test-only, and `@objectstack/dogfood` is private, hence the empty changeset. Claude-Session: https://claude.ai/code/session_01UYLC8TfjzHGwatNxZKdX7H Co-authored-by: Claude <noreply@anthropic.com>
1 parent adabaa8 commit 43754d7

2 files changed

Lines changed: 78 additions & 0 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
---
3+
4+
test-only: extends the 2FA lockout dogfood cover (#3624 follow-up). No package changes, nothing to release.

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,4 +213,78 @@ describe('#3624 follow-up: better-auth 2FA lockout counts wrong codes', () => {
213213
).toBe(0);
214214
expect(after.lockedUntil ?? null).toBeNull();
215215
});
216+
217+
it('locks the account once the budget is spent, and refuses even a correct code', async () => {
218+
// Two budgets stack here, and the test has to respect both. better-auth
219+
// allows MAX_PER_CHALLENGE verifications per two-factor cookie
220+
// (`beginAttempt(5)`) and MAX_FAILURES consecutive failures per ACCOUNT
221+
// (`accountLockout.maxFailedAttempts`, default 10). Only the second one
222+
// touches the columns under test, so reaching it takes a fresh challenge
223+
// every MAX_PER_CHALLENGE tries. Both are better-auth's defaults — the
224+
// auth manager passes no `accountLockout` config.
225+
const MAX_PER_CHALLENGE = 5;
226+
const MAX_FAILURES = 10;
227+
228+
expect((await lockoutState()).count, 'precondition: a clean budget').toBe(0);
229+
230+
let failures = 0;
231+
while (failures < MAX_FAILURES) {
232+
const cookie = await beginChallenge();
233+
for (let i = 0; i < MAX_PER_CHALLENGE && failures < MAX_FAILURES; i++) {
234+
const res = await verifyWithCookie(cookie, '000000');
235+
expect(res.status, `wrong code #${failures + 1} produced a server error`).toBeLessThan(500);
236+
failures++;
237+
expect(
238+
(await lockoutState()).count,
239+
`the counter must advance on every failure (after #${failures})`,
240+
).toBe(failures);
241+
}
242+
}
243+
244+
const locked = await lockoutState();
245+
expect(
246+
locked.lockedUntil ?? null,
247+
`budget spent (${MAX_FAILURES} failures) but locked_until was never stamped`,
248+
).not.toBeNull();
249+
expect(new Date(String(locked.lockedUntil)).getTime()).toBeGreaterThan(Date.now());
250+
251+
// The lock has to bite BEFORE the code is checked — otherwise it is not a
252+
// lockout, just a slower guess loop.
253+
const cookie = await beginChallenge();
254+
const rejected = await verifyWithCookie(cookie, totp(secret));
255+
expect(
256+
rejected.status,
257+
`a locked account must refuse even a valid code: ${await rejected.clone().text()}`,
258+
).toBe(429);
259+
});
260+
261+
it('lazily clears an expired lock on the next attempt', async () => {
262+
const before = await lockoutState();
263+
expect(before.lockedUntil ?? null, 'precondition: carries the lock from the previous test').not.toBeNull();
264+
265+
// Expire the lock rather than waiting out `durationSeconds` (900 by
266+
// default). This is the one place the test reaches past the API: there is
267+
// no endpoint that ages a lock.
268+
const users = await ql.find('sys_user', { where: { email: adminEmail }, limit: 1 }, SYS);
269+
const rows = await ql.find('sys_two_factor', { where: { user_id: String(users[0]?.id) }, limit: 1 }, SYS);
270+
await ql.update(
271+
'sys_two_factor',
272+
{ id: rows[0].id, locked_until: new Date(Date.now() - 60_000).toISOString() },
273+
SYS,
274+
);
275+
276+
// better-auth clears an expired lock through a GUARDED incrementOne —
277+
// `where locked_until <= now`, `set { failedVerificationCount: 0,
278+
// lockedUntil: null }`. That is the only place the adapter has to handle
279+
// `lte` against a datetime, so this covers the operator as much as the
280+
// columns: a driver that mishandled it would leave the user locked out
281+
// forever with no error anywhere.
282+
const cookie = await beginChallenge();
283+
const ok = await verifyWithCookie(cookie, totp(secret));
284+
expect(ok.status, `expired lock was not cleared: ${await ok.clone().text()}`).toBe(200);
285+
286+
const after = await lockoutState();
287+
expect(after.lockedUntil ?? null, 'the expired lock must be cleared, not merely ignored').toBeNull();
288+
expect(after.count, 'clearing an expired lock must reset the failure budget too').toBe(0);
289+
});
216290
});

0 commit comments

Comments
 (0)