From 3c6028f1b2e3f8586847d9d0387ee3114d4e0503 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 10:59:26 +0000 Subject: [PATCH] test(dogfood): converge two-factor-lockout on the shared TOTP helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `two-factor-lockout.dogfood.test.ts` carried a private `base32Decode` / `totp` pair, byte-for-byte identical to the ones in `test/totp.ts` — the shared module was extracted FROM this file (#10681) without rewriting its internals, so the package held two spellings of RFC 6238. Point the file at `./totp.js`, delete both local functions, and collapse the inline otpauth:// parsing into `secretFromTotpUri`. Mechanical: no assertion, timeout or fixture-semantics changes. The hand-rolled rationale (`@better-auth/utils/otp` is a transitive dependency; taking a direct one to make six digits would tie these fixtures to an internal package's resolution) already lives in the shared module's header, so it survives. That header's stale "the lockout file still carries its own copy" warning is replaced by the fact that this is now the package's single copy. Verified the file really drives the shared helper rather than merely importing it: mutating the shared `totp()` to return a fixed wrong code turns the suite red in `beforeAll` (enrolment confirmation, 401 INVALID_CODE vs the expected 200); restoring it byte-identically returns it to 5 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- packages/qa/dogfood/test/totp.ts | 9 ++-- .../test/two-factor-lockout.dogfood.test.ts | 46 +------------------ 2 files changed, 7 insertions(+), 48 deletions(-) diff --git a/packages/qa/dogfood/test/totp.ts b/packages/qa/dogfood/test/totp.ts index a8489ff2a6..f5b906f557 100644 --- a/packages/qa/dogfood/test/totp.ts +++ b/packages/qa/dogfood/test/totp.ts @@ -10,10 +10,11 @@ * defaults are the RFC's (SHA-1, 6 digits, 30s), and `enable`'s own otpauth:// * URI asserts them. * - * ⚠️ `two-factor-lockout.dogfood.test.ts` still carries its own private copy of - * these two functions — this module was extracted while adding a second caller - * (#10681) and deliberately did NOT rewrite that file's internals, since it pins - * an unrelated card. Consolidating it is filed separately. + * This is the package's SINGLE copy. It was extracted from + * `two-factor-lockout.dogfood.test.ts` while adding a second caller (#10681), + * which kept a private copy of its own until #11111 converged it here. Need a + * TOTP in a new fixture? Import it from this module — do not paste a third + * spelling of RFC 6238 into the package. */ import { createHmac } from 'node:crypto'; diff --git a/packages/qa/dogfood/test/two-factor-lockout.dogfood.test.ts b/packages/qa/dogfood/test/two-factor-lockout.dogfood.test.ts index 313ef73ce5..d010a67a8e 100644 --- a/packages/qa/dogfood/test/two-factor-lockout.dogfood.test.ts +++ b/packages/qa/dogfood/test/two-factor-lockout.dogfood.test.ts @@ -44,10 +44,10 @@ */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { createHmac } from 'node:crypto'; import showcaseStack from '@objectstack/example-showcase'; import { bootStack, type VerifyStack } from '@objectstack/verify'; import { assertArmed, authSettingArmed } from './armed.js'; +import { secretFromTotpUri, totp } from './totp.js'; const SYS = { context: { isSystem: true } }; const ADMIN_PASSWORD = 'admin123'; @@ -65,46 +65,6 @@ const LOCKOUT_DURATION_MINUTES = 40; /** better-auth's per-two-factor-cookie cap. Hardcoded upstream, not configurable. */ const MAX_PER_CHALLENGE = 5; -// ── RFC 6238 TOTP ────────────────────────────────────────────────────────── -// Hand-rolled rather than imported: `@better-auth/utils/otp` is a transitive -// dependency, and adding it as a direct one to generate six digits would tie -// this test to an internal package's resolution. better-auth's defaults are -// the RFC's (SHA-1, 6 digits, 30s), asserted by `enable`'s own otpauth:// URI. - -function base32Decode(input: string): Buffer { - const ALPHABET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'; - const clean = input.replace(/=+$/, '').toUpperCase(); - let bits = 0; - let value = 0; - const out: number[] = []; - for (const char of clean) { - const idx = ALPHABET.indexOf(char); - if (idx === -1) throw new Error(`invalid base32 character: ${char}`); - value = (value << 5) | idx; - bits += 5; - if (bits >= 8) { - out.push((value >>> (bits - 8)) & 0xff); - bits -= 8; - } - } - return Buffer.from(out); -} - -/** The 6-digit TOTP for `secret` at the current 30-second step. */ -function totp(secret: Buffer): string { - const counter = Math.floor(Date.now() / 30_000); - const buf = Buffer.alloc(8); - buf.writeBigUInt64BE(BigInt(counter)); - const digest = createHmac('sha1', secret).update(buf).digest(); - const offset = digest[digest.length - 1] & 0x0f; - const code = - ((digest[offset] & 0x7f) << 24) | - ((digest[offset + 1] & 0xff) << 16) | - ((digest[offset + 2] & 0xff) << 8) | - (digest[offset + 3] & 0xff); - return String(code % 1_000_000).padStart(6, '0'); -} - /** Collect a response's Set-Cookie values into a single request Cookie header. */ function cookieHeader(res: Response): string { const jar = res.headers.getSetCookie?.() ?? []; @@ -180,9 +140,7 @@ describe('#3624 follow-up: better-auth 2FA lockout counts wrong codes', () => { }); expect(enabled.status, `two-factor/enable: ${await enabled.clone().text()}`).toBe(200); const { totpURI } = (await enabled.json()) as { totpURI: string }; - const uriSecret = new URL(totpURI.replace('otpauth://', 'https://')).searchParams.get('secret'); - expect(uriSecret, 'no secret in the otpauth URI').toBeTruthy(); - secret = base32Decode(uriSecret as string); + secret = secretFromTotpUri(totpURI); // better-auth enrols with `verified: false`, and the sign-in path refuses // an unverified enrolment (TOTP_NOT_ENABLED) before it ever reaches the