From 5049380097cb28514aae0020bee5b63318704de6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 14:58:21 +0000 Subject: [PATCH 1/3] test(plugin-auth): the SCIM expiry faces move the clock instead of waiting 5.5 s each The two timed-ban faces in scim-deactivation-reconcile-user.test.ts let their administrator's expiry pass by advancing a fake Date (the clock the vendor's session.create hook reads) instead of a real 5.5 s setTimeout. The positive control still proves the moved clock reaches the vendor. Also: the per-file scrypt memo for the sign-in verifies, one password hash per file, and the vendor modules loaded at module top. No case, it line or expect line changes. Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../scim-deactivation-reconcile-user.test.ts | 69 +++++++++++++++++-- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts b/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts index 7c03510f889..e8f29e54677 100644 --- a/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts +++ b/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts @@ -63,7 +63,7 @@ * principal. */ -import { describe, it, expect, afterEach } from 'vitest'; +import { describe, it, expect, afterEach, afterAll, vi } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; @@ -76,6 +76,37 @@ import { registerIdentityWriteGuard, registerManagedUpdateWhitelist } from './id import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js'; import { SCIM_DEACTIVATION_BAN_REASON } from './user-ban-write.js'; import { bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js'; +import { memoizeScrypt } from './__tests__/scrypt-memo.testkit.js'; +// [#23184] better-auth's first module load is paid HERE, at module load, and +// not inside the first clocked case: clocked windows measure behaviour, never +// loading (AGENTS.md, Build & Test). `AuthManager` reaches every vendor module +// through a lazy `await import()`, so without these lines the file's first case +// paid most of a second of vendor module evaluation. The list is what the +// SCIM + organization + admin managers below boot through; a module it misses +// is still loaded in the first case, as before. The module cache hands +// `AuthManager` these same instances, and this file changes no environment the +// vendor reads at load, so nothing a case runs is different. +import 'better-auth'; +import 'better-auth/api'; +import 'better-auth/db'; +import 'better-auth/plugins'; +import 'better-auth/plugins/admin'; +import 'better-auth/plugins/bearer'; +import 'better-auth/plugins/organization'; +import 'better-auth/plugins/organization/access'; +import '@better-auth/core/context'; +import '@better-auth/core/error'; +import '@better-auth/oauth-provider'; +import '@better-auth/scim'; + +/** + * [#23184] Every sign-in below verifies the password the case attached, which + * re-derives the same production-parameter scrypt against the same salt; the + * memo answers that repeat with the bytes the real derivation produced. A + * wrong password, a fresh salt or other parameters still run the real call. + */ +const scrypt = memoizeScrypt(); +afterAll(scrypt.restore); const BASE = 'http://localhost:3000'; const AUTH = `${BASE}/api/v1/auth`; @@ -136,6 +167,11 @@ const sysUserPermissionSet = { const PS_ADMIN = 'ps_admin_full_access'; const engines: ObjectQL[] = []; +// [#23184] A case that moved the wall clock with `passTime()` gets the real +// clock back before anything else runs; a no-op for every other case. +afterEach(() => { + vi.useRealTimers(); +}); afterEach(async () => { while (engines.length) { const e = engines.pop(); @@ -302,10 +338,19 @@ async function sessionCount(h: Harness, userId: string): Promise { * (`admin-user-endpoints.ts`: hash, then `createAccount` with the local * credential issuer — 1.7.2's sign-in accepts no other credential row). The * address is marked verified because the IdP asserted it. + * + * [#23184] The hash is made once per file, by the first case's manager, and + * the same stored value is written into each case's own database: every + * manager here is built with the same options and the vendor's default + * hasher, so the value is the one each would have produced, up to its random + * salt. That salt is the only thing shared, and no case reads it; each + * sign-in still verifies the stored hash through the vendor, for real. */ +let passwordHash: Promise | undefined; async function attachPassword(h: Harness, user: Provisioned): Promise { const ctx = await h.manager.getAuthContext(); - const hashed = await ctx.password.hash(PASSWORD); + passwordHash ??= ctx.password.hash(PASSWORD); + const hashed = await passwordHash; await ctx.internalAdapter.createAccount({ userId: user.userId, providerId: 'credential', @@ -603,6 +648,22 @@ describe('[#14360] a SCIM update that does not change `active` touches no ban co /** `expiry + 500 ms` — puts both faces past the expiry with margin to spare. */ const TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500; + /** + * [#23184] Let `ms` of wall-clock time pass — by moving the clock, not by + * waiting. The expiry is judged by the vendor's `session.create` hook against + * `Date.now()` (better-auth's admin plugin: `banExpires < Date.now()`), the + * global clock, so faking `Date` alone and setting it `ms` ahead is what the + * real-time wait this replaces did, minus the 5.5 s of idle wall per face. + * Timers and everything else stay real, and the clock keeps running from + * there; the `afterEach` above puts the real one back. The positive control + * below is the proof that the moved clock reaches the vendor: its sign-in is + * admitted only because the hook read the expiry as past. + */ + const passTime = (ms: number): void => { + vi.useFakeTimers({ toFake: ['Date'], shouldAdvanceTime: true }); + vi.setSystemTime(Date.now() + ms); + }; + it("(d) a deactivation makes an administrator's EXPIRING ban permanent — the expiry cannot re-admit a deactivated principal", async () => { const h = await boot(); const hana = await provision(h, 'hana'); @@ -633,7 +694,7 @@ describe('[#14360] a SCIM update that does not change `active` touches no ban co // Let the administrator's expiry pass, then prove the refusal still holds // (status AND code): without the clearing above the vendor would have // auto-unbanned here and answered 2xx. - await new Promise((resolve) => setTimeout(resolve, TIMED_BAN_WAIT_MS)); + passTime(TIMED_BAN_WAIT_MS); expect(Date.now()).toBeGreaterThan(expiresAt.getTime()); await expectSignInBanned(h, hana.email); row = await userRow(h, hana.email); @@ -660,7 +721,7 @@ describe('[#14360] a SCIM update that does not change `active` touches no ban co ); await expectSignInBanned(h, hana.email); - await new Promise((resolve) => setTimeout(resolve, TIMED_BAN_WAIT_MS)); + passTime(TIMED_BAN_WAIT_MS); expect(Date.now()).toBeGreaterThan(expiresAt.getTime()); // The vendor's `session.create` hook auto-lifts a ban whose `banExpires` From 1476f4059f19bf21ff4a5c728c909ba8c2569e52 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:14:37 +0000 Subject: [PATCH 2/3] test(plugin-auth): last-admin-guard takes its default engine from a per-file pool Every boot() with no options in last-admin-guard.test.ts now comes from sqlite-engine-pool.testkit.ts: one booted sqlite ObjectQL engine reused across cases, its rows deleted and its catalog emptied before each use, instead of 110 per-case boots. Engines built with any option still boot fresh per call. The testkit refuses what a raw row reset cannot undo (autonumber counters, organization rows, a changed schema). No case, it line or expect line changes. Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../__tests__/sqlite-engine-pool.testkit.ts | 185 ++++++++++++++++++ .../plugin-auth/src/last-admin-guard.test.ts | 41 ++-- 2 files changed, 215 insertions(+), 11 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/__tests__/sqlite-engine-pool.testkit.ts diff --git a/packages/plugins/plugin-auth/src/__tests__/sqlite-engine-pool.testkit.ts b/packages/plugins/plugin-auth/src/__tests__/sqlite-engine-pool.testkit.ts new file mode 100644 index 00000000000..553091fb6f6 --- /dev/null +++ b/packages/plugins/plugin-auth/src/__tests__/sqlite-engine-pool.testkit.ts @@ -0,0 +1,185 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Test scaffolding [#23184]: one booted sqlite `ObjectQL` engine per test file, + * emptied before each case that takes it, instead of one boot per case. + * + * A suite in this package that builds a real engine over better-sqlite3 + * `:memory:` pays `init()` plus `syncSchemas()` (the DDL for every object it + * registers) once per case: about 80 ms for the 32 objects a SCIM deployment + * registers, and 6 to 10 ms for a small fixture set, so a file of 110 cases + * spends most of a second booting. The engine a case receives from this pool + * was built by the same `boot` function, so its registry, its hooks and its + * driver are the ones a fresh boot gives; what the previous case left behind + * is its ROWS, and those are deleted before the engine is handed out again. + * + * What is NOT reset, and why that is safe or refused: + * + * - the engine's in-memory autonumber counters: a pool whose objects declare + * an `autonumber` field is refused when it first boots, since a counter + * seeded by an earlier case would not restart where a fresh engine does; + * - the engine's memo of the install's organizations, which only an engine + * write to `sys_organization` invalidates: the rows are deleted below the + * engine, so a database that holds an organization row is refused rather + * than emptied; + * - the schema itself: a case that created or dropped a table leaves an + * engine a fresh boot would not give, and the next `take()` refuses it; + * - the write epoch and the log-once sets: counters and logging only, read by + * nothing these suites assert on. + * + * A case may hold several engines at once: `take()` hands out a pooled engine + * no case holds, or boots another. Engines that were taken come back with + * `release()` in the file's `afterEach`, and `destroy()` closes them all in + * its `afterAll`. A suite that needs a differently-built engine (other hooks, + * other objects) boots it itself, as before; only the one `boot` shape is + * pooled. + * + * ```ts + * const pool = sqliteEnginePool(async () => ({ engine, driver })); + * afterEach(pool.release); + * afterAll(pool.destroy); + * const engine = await pool.take(); + * ``` + */ + +import type { ObjectQL } from '@objectstack/objectql'; +import type { SqlDriver } from '@objectstack/driver-sql'; + +/** What the pool's `boot` returns: the engine, and the driver it registered as the default. */ +export interface BootedSqliteEngine { + engine: E; + driver: SqlDriver; +} + +export interface SqliteEnginePoolOptions { + /** Per-engine fixture state the suite keeps beside the database (a catalog set, say), reset with it. */ + onReset?: (engine: E) => void | Promise; +} + +export interface SqliteEnginePool { + /** An emptied pooled engine no case holds, or a fresh boot when every pooled engine is held. */ + take(): Promise; + /** Hand back every engine taken since the last release; the file's `afterEach`. */ + release(): void; + /** Destroy every engine the pool booted; the file's `afterAll`. */ + destroy(): Promise; + /** Boots run, and resets performed, since the pool was made. */ + counts(): { boots: number; resets: number }; +} + +interface Entry { + engine: E; + driver: SqlDriver; + /** The tables the boot created, sorted. */ + tables: readonly string[]; + held: boolean; +} + +/** The engine's in-memory organization memo is keyed on this object's rows (see the header). */ +const ORGANIZATION_TABLE = 'sys_organization'; + +type Knex = ReturnType; + +async function tableNames(knex: Knex): Promise { + const rows = (await knex('sqlite_master').where({ type: 'table' }).select('name')) as Array<{ name: string }>; + return rows + .map((row) => row.name) + .filter((name) => !name.startsWith('sqlite_')) + .sort(); +} + +function autonumberFields(engine: ObjectQL): string[] { + const objects = engine.registry.getAllObjects() as Array<{ name: string; fields?: Record }>; + return objects.flatMap((object) => + Object.entries(object.fields ?? {}) + .filter(([, field]) => field?.type === 'autonumber') + .map(([name]) => `${object.name}.${name}`), + ); +} + +/** Make a pool over `boot`; see the module header for what a reused engine does and does not carry. */ +export function sqliteEnginePool( + boot: () => Promise>, + options: SqliteEnginePoolOptions = {}, +): SqliteEnginePool { + const entries: Array> = []; + let boots = 0; + let resets = 0; + + async function bootEntry(): Promise> { + const { engine, driver } = await boot(); + boots += 1; + const counted = autonumberFields(engine); + if (counted.length > 0) { + try { + await engine.destroy(); + } catch { + /* noop */ + } + throw new Error( + `sqliteEnginePool: ${counted.join(', ')} declare an autonumber field, whose counter the engine keeps in ` + + 'memory across a reset; boot that engine per case instead of pooling it.', + ); + } + const entry: Entry = { engine, driver, tables: await tableNames(driver.getKnex()), held: true }; + entries.push(entry); + return entry; + } + + async function empty(entry: Entry): Promise { + const knex = entry.driver.getKnex(); + const tables = await tableNames(knex); + if (tables.join('\n') !== entry.tables.join('\n')) { + const added = tables.filter((t) => !entry.tables.includes(t)); + const dropped = entry.tables.filter((t) => !tables.includes(t)); + throw new Error( + `sqliteEnginePool: a case changed this engine's schema (created: ${added.join(', ') || 'none'}; ` + + `dropped: ${dropped.join(', ') || 'none'}), so it is no longer the engine a fresh boot gives. ` + + 'Boot that case its own engine.', + ); + } + if (tables.includes(ORGANIZATION_TABLE)) { + const [row] = (await knex(ORGANIZATION_TABLE).count({ n: '*' })) as Array<{ n: number | string }>; + if (Number(row?.n ?? 0) > 0) { + throw new Error( + `sqliteEnginePool: a case left ${row?.n} ${ORGANIZATION_TABLE} row(s), and the engine memoizes the ` + + 'organization set until an engine write to that object invalidates it; deleting them below the ' + + 'engine would hand the next case a stale memo. Boot that case its own engine.', + ); + } + } + await knex.transaction(async (trx) => { + // Checked at commit, when every table is empty: the delete order does not matter. + await trx.raw('PRAGMA defer_foreign_keys = ON'); + for (const table of tables) await trx(table).del(); + const [sequence] = (await trx('sqlite_master').where({ type: 'table', name: 'sqlite_sequence' })) as unknown[]; + if (sequence) await trx('sqlite_sequence').del(); + }); + await options.onReset?.(entry.engine); + resets += 1; + } + + return { + async take() { + const free = entries.find((entry) => !entry.held); + if (!free) return (await bootEntry()).engine; + free.held = true; + await empty(free); + return free.engine; + }, + release() { + for (const entry of entries) entry.held = false; + }, + async destroy() { + while (entries.length) { + const entry = entries.pop()!; + try { + await entry.engine.destroy(); + } catch { + /* noop */ + } + } + }, + counts: () => ({ boots, resets }), + }; +} diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.test.ts b/packages/plugins/plugin-auth/src/last-admin-guard.test.ts index 0af2d833801..283db61973f 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.test.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.test.ts @@ -51,7 +51,7 @@ * file re-run it on the same fixtures rather than describing it. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, afterAll } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { isAPIError } from 'better-auth/api'; @@ -63,6 +63,7 @@ import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js'; import { createObjectQLAdapterFactory } from './objectql-adapter.js'; import { buildAdminPluginSchema, buildOrganizationPluginSchema } from './auth-schema-config.js'; import { bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js'; +import { sqliteEnginePool, type BootedSqliteEngine } from './__tests__/sqlite-engine-pool.testkit.js'; import { admin } from 'better-auth/plugins/admin'; import { organization } from 'better-auth/plugins/organization'; @@ -246,6 +247,7 @@ afterEach(async () => { else process.env[POSTURE_ENV] = ambientPosture; if (ambientMultiOrg === undefined) delete process.env[MULTI_ORG_ENV]; else process.env[MULTI_ORG_ENV] = ambientMultiOrg; + defaultEngines.release(); // `:memory:` dies with its connection; closing keeps one live database per // test from piling up in a file this size. const open = engines; @@ -255,17 +257,34 @@ afterEach(async () => { } }); +/** + * [#23184] Every `boot()` with no options — the engine most cases take, often + * twice in one case — comes from a per-file pool instead of a fresh boot: the + * same six objects, the same guard registration, its rows deleted and its + * catalog emptied before a case receives it (`sqlite-engine-pool.testkit.ts` + * says what a reused engine does and does not carry). 110 cases spent about a + * second booting engines they never told apart. Any other option set boots a + * fresh engine per call, as before, because its hooks differ. + */ +const defaultEngines = sqliteEnginePool(() => bootEngine({}), { + onReset: (engine) => catalogOf(engine).clear(), +}); +afterAll(defaultEngines.destroy); + async function boot(opts: BootOptions = {}): Promise { + if (Object.keys(opts).length === 0) return defaultEngines.take(); + return (await bootEngine(opts, engines)).engine; +} + +async function bootEngine(opts: BootOptions, track?: ObjectQL[]): Promise { const engine = new ObjectQL(); - engines.push(engine); - engine.registerDriver( - new SqlDriver({ - client: 'better-sqlite3', - connection: { filename: ':memory:' }, - useNullAsDefault: true, - }), - true, - ); + track?.push(engine); + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + engine.registerDriver(driver, true); await engine.init(); for (const o of [sysUser, sysMember, sysPermissionSet, sysUserPermissionSet, sysAccount, sysMetadataActivation]) { engine.registry.registerObject(o as never); @@ -288,7 +307,7 @@ async function boot(opts: BootOptions = {}): Promise { ...(opts.maxScan !== undefined ? { maxScan: opts.maxScan } : {}), }); } - return engine; + return { engine, driver }; } /** A user row, plus whatever standing the case needs. */ From 0d97b879a6dd5f66ba009b758970057996b3d76b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:16:43 +0000 Subject: [PATCH 3/3] test(plugin-auth): the SCIM deactivation suite boots its engine once per file scim-deactivation-reconcile-user.test.ts takes its sqlite engine (the 32 identity objects and the two write guards) from sqlite-engine-pool.testkit.ts, emptied before each case, instead of a schema sync per case. The AuthManager and the SCIM credential stay per case. No case, it line or expect line changes. Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../scim-deactivation-reconcile-user.test.ts | 73 ++++++++++--------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts b/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts index e8f29e54677..0fbcf875db0 100644 --- a/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts +++ b/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts @@ -77,6 +77,7 @@ import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js'; import { SCIM_DEACTIVATION_BAN_REASON } from './user-ban-write.js'; import { bindTestSecurityCatalog } from './__tests__/security-catalog.testkit.js'; import { memoizeScrypt } from './__tests__/scrypt-memo.testkit.js'; +import { sqliteEnginePool, type BootedSqliteEngine } from './__tests__/sqlite-engine-pool.testkit.js'; // [#23184] better-auth's first module load is paid HERE, at module load, and // not inside the first clocked case: clocked windows measure behaviour, never // loading (AGENTS.md, Build & Test). `AuthManager` reaches every vendor module @@ -166,22 +167,23 @@ const sysUserPermissionSet = { const PS_ADMIN = 'ps_admin_full_access'; -const engines: ObjectQL[] = []; // [#23184] A case that moved the wall clock with `passTime()` gets the real // clock back before anything else runs; a no-op for every other case. afterEach(() => { vi.useRealTimers(); }); -afterEach(async () => { - while (engines.length) { - const e = engines.pop(); - try { - await (e as unknown as { destroy?(): Promise })?.destroy?.(); - } catch { - /* noop */ - } - } -}); + +/** + * [#23184] The engine every case boots on, from a per-file pool: built once by + * {@link bootEngine} — the same objects, the same two guards — and handed to + * each case with its rows deleted (`sqlite-engine-pool.testkit.ts` says what a + * reused engine does and does not carry). Its schema sync was about 80 ms of + * every case. The manager and the SCIM credential stay per case, built fresh + * by {@link boot} on the emptied database. + */ +const enginePool = sqliteEnginePool(bootEngine); +afterEach(enginePool.release); +afterAll(enginePool.destroy); interface Harness { engine: ObjectQL; @@ -198,16 +200,32 @@ interface Harness { * nothing here names it. */ async function boot(): Promise { + const engine = await enginePool.take(); + + const manager = new AuthManager({ + secret: SECRET, + baseUrl: BASE, + dataEngine: engine as never, + getTenancy: () => createTenancyService({ requested: 'isolated', probeIsolation: () => true }), + plugins: { scim: true, organization: true }, + } as never); + + const { token } = await mintScimConnectionCredential(engine as never, SECRET, { + connectionId: 'okta-14360', + }); + + return { engine, manager, token, send: (request) => manager.handleRequest(request) }; +} + +/** The engine a SCIM deployment's identity objects live on, with its two write guards. */ +async function bootEngine(): Promise { const engine = new ObjectQL(); - engines.push(engine); - engine.registerDriver( - new SqlDriver({ - client: 'better-sqlite3', - connection: { filename: ':memory:' }, - useNullAsDefault: true, - }), - true, - ); + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + engine.registerDriver(driver, true); await engine.init(); for (const object of AUTH_OBJECTS) { engine.registry.registerObject(object as never, '@objectstack/plugin-auth'); @@ -227,20 +245,7 @@ async function boot(): Promise { registerLastAdminGuard(engine as unknown as LastAdminGuardEngine, { packageId: 'test.last-admin-guard', }); - - const manager = new AuthManager({ - secret: SECRET, - baseUrl: BASE, - dataEngine: engine as never, - getTenancy: () => createTenancyService({ requested: 'isolated', probeIsolation: () => true }), - plugins: { scim: true, organization: true }, - } as never); - - const { token } = await mintScimConnectionCredential(engine as never, SECRET, { - connectionId: 'okta-14360', - }); - - return { engine, manager, token, send: (request) => manager.handleRequest(request) }; + return { engine, driver }; } // ---------------------------------------------------------------------------