diff --git a/.changeset/scim-transaction-scope-at-request-door.md b/.changeset/scim-transaction-scope-at-request-door.md new file mode 100644 index 0000000000..1eefb1c6dd --- /dev/null +++ b/.changeset/scim-transaction-scope-at-request-door.md @@ -0,0 +1,5 @@ +--- +'@objectstack/plugin-auth': patch +--- + +SCIM provisioning multi-writes now run inside one engine transaction, as the adapter's `#3653` scoping note already declared. On `@better-auth/scim` 1.7.2 the SCIM request scope was stamped with `AsyncLocalStorage.enterWith` inside the `verifyBearerToken` callback and was not observed at write time (measured: zero `engine.transaction` calls across `POST /scim/v2/Users` and `PATCH /scim/v2/Users/{id}`), so `sys_user`, `sys_scim_subject` and `sys_scim_user` landed as separate autocommits, and a refused deactivation left the SCIM resource reporting `active: false` for an account that was still enabled. `AuthManager.handleRequest` now opens the scope with `run(...)` around every request under `/scim/v2` — exactly as narrow as before; non-SCIM better-auth flows keep their sequential posture. A refused last-administrator deactivation now rolls the vendor's own `scimUser.active = false` write back, so the SCIM resource keeps reading `active: true`. The pin the #14360 suite held on that residual (`scim-deactivation-reconcile-user.test.ts`, face (c)) is flipped from `false` to `true` deliberately with this change, and a new runtime pin (`scim-transaction-scope.test.ts`) observes each SCIM mutation calling `engine.transaction`. diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 2ae509b93c..4e1bede819 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -923,6 +923,22 @@ export function ipMatchesRange(ip: string, range: string): boolean { */ const SMS_QUOTA_EXCEEDED_CODE = 'TOO_MANY_REQUESTS'; +/** + * [#14522] The better-auth endpoint path prefix every SCIM 2.0 protocol + * endpoint lives under (`/scim/v2/Users`, `/scim/v2/Groups/:groupId`, …) — + * the same predicate `@better-auth/scim` uses for its own after-hook matcher + * (`context.path?.startsWith("/scim/v2")`). A request under it runs inside + * `scimRequestScope`; see `handleRequest`. + */ +const SCIM_PROTOCOL_PATH_PREFIX = '/scim/v2'; + +function isScimProtocolPath(endpointPath: string | undefined): boolean { + return ( + endpointPath === SCIM_PROTOCOL_PATH_PREFIX || + endpointPath?.startsWith(`${SCIM_PROTOCOL_PATH_PREFIX}/`) === true + ); +} + /** * #6039 — is this `SendSmsResult.error` the quota wall's refusal? * @@ -3266,17 +3282,22 @@ export class AuthManager { if (enabled.scim) { await this.addOptionalPlugin(plugins, 'scim', async () => { const { scim } = await import('@better-auth/scim'); - const { verifyScimBearerToken, scimRequestScope } = await import('./scim-connection-service.js'); + const { verifyScimBearerToken } = await import('./scim-connection-service.js'); const secret = this.resolveAuthSecret(); return scim({ connections: [], authentication: { verifyBearerToken: async (input) => { - // Mark the remainder of this request's async chain as a SCIM - // protocol request, so the adapter runs its provisioning writes - // inside a REAL engine transaction (see scimRequestScope's - // rationale in scim-connection-service.ts). - scimRequestScope.enterWith({ scim: true }); + // ⛔ No `scimRequestScope.enterWith(...)` here. The SCIM request + // scope that makes the adapter open a REAL engine transaction is + // opened by `handleRequest` with `run(...)` around the whole + // request (see `SCIM_PROTOCOL_PATH_PREFIX`). It used to be + // stamped from this callback and never reached the writes: an + // `enterWith` marks only the async resource it runs in and that + // resource's descendants, and the vendor resumes the endpoint + // handler from a continuation captured BEFORE this verifier ran + // (measured on 1.7.2 — zero engine transactions across a SCIM + // POST + PATCH; pinned by `scim-transaction-scope.test.ts`). const engine = this.config.dataEngine; if (!engine) return null; // no store to verify against — fail closed return verifyScimBearerToken(engine as never, secret, input.token); @@ -4732,10 +4753,32 @@ export class AuthManager { // is left with an identity that still occupies the org roster and can no // longer sign in. Nothing tells the operator, and there is no way back. const endpointPath = this.betterAuthEndpointPath(request); + + // [#3653 / #14522] A SCIM protocol request (`/scim/v2/*`) runs inside + // `scimRequestScope`, which is what makes the adapter's `transaction` + // config open a REAL engine transaction around the vendor's provisioning + // multi-writes (`objectql-adapter.ts`, the scoping note there). Opened + // HERE, with `run(...)` around the whole request, for the same reason the + // actor-attribution scope above is: `run` has a callback boundary that + // every `als.run` the vendor performs underneath nests inside. The stamp + // used to be an `enterWith` inside the SCIM plugin's `verifyBearerToken` + // callback, and it never reached the writes — the vendor resumes the + // endpoint handler from a continuation captured before the verifier ran + // (measured on 1.7.2: zero `engine.transaction` calls across + // `POST /Users` + `PATCH /Users/{id}`). Keyed on the endpoint path prefix + // so it is exactly as narrow as before — SCIM protocol requests only; the + // non-SCIM flows keep their sequential posture, which the scoping note + // records as load-bearing. Pinned by `scim-transaction-scope.test.ts`. + const runRequest = isScimProtocolPath(endpointPath) + ? async (): Promise => { + const { scimRequestScope } = await import('./scim-connection-service.js'); + return scimRequestScope.run({ scim: true }, runHandler); + } + : runHandler; const vendorResponse = endpointPath !== undefined && SESSION_ERASURE_PATHS.has(endpointPath) - ? await this.runSubjectErasureAtomically(runHandler) - : await runHandler(); + ? await this.runSubjectErasureAtomically(runRequest) + : await runRequest(); // [#10349] The better-auth-native `/admin/` routes refuse an anonymous // caller through the vendor's `adminMiddleware` @@ -4917,15 +4960,15 @@ export class AuthManager { * so it never half-lands: the account stays enabled and nothing is * skipped silently. * - * ⚠️ What does NOT roll back today: the vendor runs this callback inside + * What ALSO rolls back: the vendor runs this callback inside * `runWithTransaction`, which on this adapter is a real engine transaction - * only while `scimRequestScope` is set — and that scope, stamped inside - * `verifyBearerToken`, is not observed at write time on 1.7.2 (measured: - * zero `engine.transaction` calls across a SCIM POST + PATCH; #14522). So + * while `scimRequestScope` is set — and `handleRequest` opens that scope + * around every SCIM protocol request (#14522; it was once stamped inside + * `verifyBearerToken` with `enterWith` and never reached the writes). So * the vendor's own `scimUser.active = false` write, made before this - * callback, survives a refusal and the SCIM resource reads inactive while - * the account is enabled. #14522 owns that seam; the #14360 suite pins the - * residual so its fix flips the pin deliberately. + * callback, is rolled back with the refusal, and the SCIM resource keeps + * reading `active: true` for the account that stayed enabled — pinned by + * the #14360 suite's face (c). * * Deliberately NOT applied here: the last-LOCAL-credential guard the admin * mount re-runs (`isLastLocalCredentialHolder`). That guard protects the @@ -4938,8 +4981,8 @@ export class AuthManager { * * Every read and write goes through `context.database` — the adapter the * vendor bound to its transaction — never through an `internalAdapter` - * resolved outside it, so the moment #14522 makes that transaction real, - * the ban commits or rolls back with the SCIM mutation it belongs to. + * resolved outside it, so the ban commits or rolls back with the SCIM + * mutation it belongs to. */ private async reconcileScimUserLifecycle( state: SCIMIdentityState, diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.ts b/packages/plugins/plugin-auth/src/objectql-adapter.ts index 4b268e9968..956d23c99d 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.ts +++ b/packages/plugins/plugin-auth/src/objectql-adapter.ts @@ -791,8 +791,16 @@ export function createObjectQLAdapterFactory(rawDataEngine: IDataEngine) { // Core better-auth flows never had native DB transactions here (the factory // default is the sequential as-is fallback), so they KEEP that historical // posture; the real transaction opens exactly where upstream's assertion - // demands it — inside an authenticated SCIM protocol request, marked by the - // auth manager's `verifyBearerToken` via `scimRequestScope`. Remaining + // demands it — inside a SCIM protocol request (`/scim/v2/*`), the scope + // `AuthManager.handleRequest` opens with `scimRequestScope.run(...)` around + // the whole request. ⚠️ It was once stamped with `enterWith` inside the + // `verifyBearerToken` callback and never reached this seam: an `enterWith` + // marks only the async resource it runs in and that resource's descendants, + // and the vendor resumes the endpoint handler from a continuation captured + // before the verifier ran — measured on 1.7.2 as zero engine transactions + // across POST + PATCH /Users while the mount-time assertion stayed green. + // The scope is therefore pinned at RUN time (`scim-transaction-scope.test.ts`: + // a SCIM mutation observed to call `engine.transaction`). Remaining // declared degrades on that path: an engine with no `transaction` API runs // the callback directly, and a driver without `beginTransaction` follows // the engine's ADR-0119 D1 warn-once degrade. diff --git a/packages/plugins/plugin-auth/src/scim-connection-service.ts b/packages/plugins/plugin-auth/src/scim-connection-service.ts index 525a4225dd..6e09a852b6 100644 --- a/packages/plugins/plugin-auth/src/scim-connection-service.ts +++ b/packages/plugins/plugin-auth/src/scim-connection-service.ts @@ -42,10 +42,21 @@ import { AsyncLocalStorage } from 'node:async_hooks'; /** * Request-scoped marker: "the current async chain is a SCIM protocol - * request". Entered by the auth manager's `verifyBearerToken` wrapper (the - * first application code every authenticated SCIM request runs) via - * `enterWith`, so it holds for the remainder of that request's async chain — - * including the provisioning writes the plugin performs afterwards. + * request". Opened by `AuthManager.handleRequest` with `run(...)` around every + * request whose better-auth endpoint path is under `/scim/v2`, so it holds + * for that request's whole async chain — the endpoint handler and the + * provisioning writes the plugin performs inside it. + * + * ⛔ Not `enterWith`, and not from inside the `verifyBearerToken` callback: + * that is where it used to be stamped, and the store never reached the + * writes. An `enterWith` marks only the async resource it runs in and that + * resource's descendants; the vendor awaits the verifier from the endpoint's + * own frame and resumes the handler from a continuation captured before the + * verifier ran. Measured on `@better-auth/scim` 1.7.2: zero + * `engine.transaction` calls across `POST /Users` + `PATCH /Users/{id}`, + * `inScimRequestScope()` false inside every identity write. `run(...)` has a + * callback boundary; every `als.run` the vendor performs underneath nests + * inside it. Pinned at run time by `scim-transaction-scope.test.ts`. * * Read by `objectql-adapter.ts`'s `config.transaction`: SCIM requests get a * REAL engine transaction (the atomicity upstream's 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 8a4d445f4e..66be354551 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 @@ -475,16 +475,17 @@ describe('[#14360] deactivating the last administrator is refused through SCIM, expect(row?.ban_reason ?? null).toBeNull(); await expectSignInAccepted(h, owner.email); - // RESIDUAL — pinned as observed, filed as #14522, ⛔ not this card's to - // fix: the vendor's own `scimUser.active = false` write, made BEFORE the - // callback inside what it believes is a transaction, survives the - // refusal, because the adapter's #3653 SCIM transaction scoping never - // opens an engine transaction on 1.7.2 (measured: 0 `engine.transaction` - // and 0 `driver.beginTransaction` calls across POST + PATCH /Users). So - // the SCIM resource reports `active: false` while the account is still - // enabled. When #14522 lands, this line flips to `true` DELIBERATELY — - // that is the whole reason it is asserted rather than left unread. - expect(await scimActive(h, owner.scimId)).toBe(false); + // [#14522] The vendor's own `scimUser.active = false` write, made BEFORE + // the callback inside its transaction, is rolled back WITH the refusal: + // the adapter's #3653 SCIM transaction scoping opens a real engine + // transaction now that the scope is opened at `handleRequest` (it was + // stamped with `enterWith` inside `verifyBearerToken` and never reached + // the writes — measured as 0 `engine.transaction` calls across POST + + // PATCH /Users). So the SCIM resource keeps reporting `active: true` for + // the account that stayed enabled. This line read `false` on purpose + // while that residual was open and was flipped DELIBERATELY with the fix; + // the positive control below is the genuine `false`. + expect(await scimActive(h, owner.scimId)).toBe(true); }, 60_000); it('(c) positive control: with a second administrator left behind, the same request succeeds', async () => { diff --git a/packages/plugins/plugin-auth/src/scim-transaction-scope.test.ts b/packages/plugins/plugin-auth/src/scim-transaction-scope.test.ts new file mode 100644 index 0000000000..b2c01f1d01 --- /dev/null +++ b/packages/plugins/plugin-auth/src/scim-transaction-scope.test.ts @@ -0,0 +1,419 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14522] SCIM provisioning writes run inside ONE engine transaction — the + * RUNTIME pin behind the `#3653` scoping note in `objectql-adapter.ts`. + * + * ## The defect + * + * The adapter's `transaction` config opens a real `engine.transaction()` only + * while `inScimRequestScope()` reads true. That scope used to be stamped with + * `scimRequestScope.enterWith(...)` inside the `verifyBearerToken` callback + * handed to `@better-auth/scim` — and `enterWith` has no callback boundary: + * it marks the async resource it runs in and that resource's descendants. + * The vendor awaits the verifier (an endpoint `use` middleware) from the + * endpoint's own async frame and resumes the handler under its own + * `runWithEndpointContext` (an `als.run`), so by the time the handler asked + * the adapter for a transaction the store was gone. Measured on 1.7.2 before + * this card: zero `engine.transaction` and zero `driver.beginTransaction` + * calls across `POST /Users` + `PATCH /Users/{id}`, `inScimRequestScope()` + * false inside every `sys_user` / `sys_scim_user` write — while the vendor's + * mount-time `assertNativeSCIMTransactions` stayed satisfied, because it only + * asks whether `transaction` is a function. + * + * The scope is now opened with `scimRequestScope.run(...)` around the WHOLE + * request at `AuthManager.handleRequest`, keyed on the better-auth endpoint + * path prefix `/scim/v2` — a callback boundary that every `als.run` the + * vendor performs underneath nests inside, the same seam the actor-attribution + * scope and the subject-erasure transaction already use. + * + * ## Why these are RUNTIME pins + * + * `credential-at-rest-posture.test.ts` records that the vendor refuses to + * mount on a sequential-fallback `transaction`. That is a mount-time + * assertion, and it passed throughout the defect. Every case here observes a + * SCIM mutation at run time — through `AuthManager.handleRequest()` with a + * real bearer, on a real `ObjectQL` over better-sqlite3, the harness shape of + * the #14360 suite (`scim-deactivation-reconcile-user.test.ts`). That suite + * pins the CONSEQUENCE (face (c): a refused last-administrator deactivation + * no longer leaves the SCIM resource reporting inactive); this file pins the + * MECHANISM. + * + * (a) `POST /Users` and `PATCH /Users/{id}` each open the engine + * transaction (`engine.transaction` ≥ 1, `driver.beginTransaction` ≥ 1) + * and every identity write made inside them sees the SCIM scope. + * (b) atomicity: a failure on a LATER provisioning write leaves NO partial + * identity — `sys_user`, `sys_scim_subject`, `sys_scim_user` all absent. + * (c) negative control — the triage's first scope guard: non-SCIM flows keep + * their historical sequential posture. Sign-up and sign-in open ZERO + * engine transactions, exactly as before this card. + * (d) a SCIM read opens none either: the scope adds no transaction where the + * vendor asks for none. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { + SysUser, + SysSession, + SysAccount, + SysVerification, + SysOrganization, + SysMember, + SysInvitation, + SysTeam, + SysTeamMember, + SysScimConnectionBinding, + SysScimConnectionCredential, + SysScimGroup, + SysScimGroupMember, + SysScimIdentityTombstone, + SysScimProjectionGrant, + SysScimSubject, + SysScimUser, + SysJwks, +} from '@objectstack/platform-objects'; +import { AuthManager } from './auth-manager.js'; +import { createTenancyService } from './tenancy-service.js'; +import { inScimRequestScope, mintScimConnectionCredential } from './scim-connection-service.js'; + +const BASE = 'http://localhost:3000'; +const AUTH = `${BASE}/api/v1/auth`; +const SECRET = 'test-secret-at-least-32-chars-long-14522'; +const PASSWORD = 'correct-horse-battery-staple-14522'; + +const USER_SCHEMA = 'urn:ietf:params:scim:schemas:core:2.0:User'; +const PATCH_SCHEMA = 'urn:ietf:params:scim:api:messages:2.0:PatchOp'; + +/** Every read below is a safety-proof read, never RLS-scoped to a caller. */ +const SYSTEM = { context: { isSystem: true } } as const; + +/** The identity objects a SCIM provisioning request touches. */ +const IDENTITY_OBJECTS = ['sys_user', 'sys_scim_subject', 'sys_scim_user'] as const; + +const AUTH_OBJECTS = [ + SysUser, + SysSession, + SysAccount, + SysVerification, + SysOrganization, + SysMember, + SysInvitation, + SysTeam, + SysTeamMember, + SysScimConnectionBinding, + SysScimConnectionCredential, + SysScimGroup, + SysScimGroupMember, + SysScimIdentityTombstone, + SysScimProjectionGrant, + SysScimSubject, + SysScimUser, + SysJwks, +]; + +const engines: ObjectQL[] = []; +afterEach(async () => { + vi.restoreAllMocks(); + while (engines.length) { + const e = engines.pop(); + try { + await (e as unknown as { destroy?(): Promise })?.destroy?.(); + } catch { + /* noop */ + } + } +}); + +interface Harness { + engine: ObjectQL; + driver: SqlDriver; + manager: AuthManager; + token: string; + send: (request: Request) => Promise; +} + +/** + * The manager under test, built the way a deployment with SCIM turned on + * builds it. The scope stamp and the adapter's `transaction` config are both + * inside the system under test; nothing here names either. + */ +async function boot(): Promise { + const engine = new ObjectQL(); + engines.push(engine); + 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'); + } + await engine.syncSchemas(); + + 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-14522', + }); + + return { engine, driver, manager, token, send: (request) => manager.handleRequest(request) }; +} + +// --------------------------------------------------------------------------- +// Observation — the two spies the card names, plus the scope sampled INSIDE +// every engine write (the only place the answer matters). +// --------------------------------------------------------------------------- + +interface WriteSample { + op: 'insert' | 'update'; + object: string; + scim: boolean; +} + +interface Observation { + transaction: ReturnType; + beginTransaction: ReturnType; + writes: WriteSample[]; + reset(): void; +} + +/** + * Spy `engine.transaction` and `driver.beginTransaction` (call-through), and + * wrap `engine.insert` / `engine.update` to record whether the SCIM scope is + * visible at the moment each write happens. Optionally fail one object's + * insert, for the atomicity case. + */ +function observe(h: Harness, failInsertOn?: string): Observation { + const transaction = vi.spyOn(h.engine, 'transaction'); + const beginTransaction = vi.spyOn(h.driver, 'beginTransaction'); + const writes: WriteSample[] = []; + for (const op of ['insert', 'update'] as const) { + const original = (h.engine as unknown as Record unknown>)[op].bind( + h.engine, + ); + vi.spyOn(h.engine as unknown as Record unknown>, op).mockImplementation( + async (object: unknown, ...rest: unknown[]) => { + writes.push({ op, object: String(object), scim: inScimRequestScope() }); + if (op === 'insert' && failInsertOn !== undefined && object === failInsertOn) { + throw new Error(`[#14522 test] injected failure on insert ${failInsertOn}`); + } + return original(object, ...rest); + }, + ); + } + return { + transaction, + beginTransaction, + writes, + reset() { + transaction.mockClear(); + beginTransaction.mockClear(); + writes.length = 0; + }, + }; +} + +const identityWrites = (o: Observation): WriteSample[] => + o.writes.filter((w) => (IDENTITY_OBJECTS as readonly string[]).includes(w.object)); + +const describeWrites = (o: Observation): string => + o.writes.map((w) => `${w.op}:${w.object}:${w.scim ? 'scim' : 'NO-SCOPE'}`).join(' '); + +// --------------------------------------------------------------------------- +// SCIM 2.0 requests — the shapes an identity provider actually sends +// --------------------------------------------------------------------------- + +function scimRequest(h: Harness, method: string, path: string, body?: unknown): Request { + return new Request(`${AUTH}/scim/v2${path}`, { + method, + headers: { + origin: BASE, + authorization: `Bearer ${h.token}`, + ...(body !== undefined ? { 'content-type': 'application/scim+json' } : {}), + }, + ...(body !== undefined ? { body: JSON.stringify(body) } : {}), + }); +} + +function provisionRequest(h: Harness, localPart: string): Request { + const email = `${localPart}@example.com`; + return scimRequest(h, 'POST', '/Users', { + schemas: [USER_SCHEMA], + userName: email, + name: { givenName: localPart, familyName: 'Example' }, + displayName: `${localPart} Example`, + emails: [{ value: email, primary: true, type: 'work' }], + active: true, + }); +} + +async function provision(h: Harness, localPart: string): Promise<{ scimId: string; email: string }> { + const res = await h.send(provisionRequest(h, localPart)); + expect(res.status, `SCIM POST /Users failed: ${await res.clone().text()}`).toBe(201); + const body = (await res.json()) as { id: string }; + return { scimId: body.id, email: `${localPart}@example.com` }; +} + +const setActive = (h: Harness, scimId: string, active: boolean) => + h.send( + scimRequest(h, 'PATCH', `/Users/${scimId}`, { + schemas: [PATCH_SCHEMA], + Operations: [{ op: 'replace', path: 'active', value: active }], + }), + ); + +async function rowsOf(h: Harness, object: string): Promise { + const rows = await h.engine.find(object, { where: {} }, SYSTEM); + return Array.isArray(rows) ? rows.length : 0; +} + +async function userRow(h: Harness, email: string): Promise | null> { + return h.engine.findOne( + 'sys_user', + { where: { email }, fields: ['id', 'email'] }, + SYSTEM, + ) as Promise | null>; +} + +// --------------------------------------------------------------------------- +// (a) — each SCIM mutation opens the engine transaction and its writes see the scope +// --------------------------------------------------------------------------- + +describe('[#14522] a SCIM mutation runs inside one engine transaction', () => { + it('(a) POST /Users: engine.transaction and driver.beginTransaction each called, every identity write in scope', async () => { + const h = await boot(); + const o = observe(h); + + await provision(h, 'alice'); + + expect(o.transaction, `engine.transaction calls; writes: ${describeWrites(o)}`).toHaveBeenCalled(); + expect(o.beginTransaction, 'driver.beginTransaction calls').toHaveBeenCalled(); + const writes = identityWrites(o); + // The provisioning sequence really is several writes — the reason the + // transaction exists at all. + expect(writes.length, describeWrites(o)).toBeGreaterThanOrEqual(2); + for (const object of IDENTITY_OBJECTS) { + expect( + writes.some((w) => w.object === object), + `expected a write on ${object}; saw: ${describeWrites(o)}`, + ).toBe(true); + } + expect( + writes.filter((w) => !w.scim).map((w) => `${w.op}:${w.object}`), + 'identity writes made OUTSIDE the SCIM scope', + ).toEqual([]); + }, 60_000); + + it('(a) PATCH /Users/{id} active:false: the transaction opens again and the sys_user / sys_scim_user writes see the scope', async () => { + const h = await boot(); + const alice = await provision(h, 'alice'); + const o = observe(h); + + const res = await setActive(h, alice.scimId, false); + expect(res.status, `SCIM PATCH active:false failed: ${await res.clone().text()}`).toBe(200); + + expect(o.transaction, `engine.transaction calls; writes: ${describeWrites(o)}`).toHaveBeenCalled(); + expect(o.beginTransaction, 'driver.beginTransaction calls').toHaveBeenCalled(); + const writes = identityWrites(o); + expect(writes.some((w) => w.object === 'sys_scim_user'), describeWrites(o)).toBe(true); + expect(writes.some((w) => w.object === 'sys_user'), describeWrites(o)).toBe(true); + expect( + writes.filter((w) => !w.scim).map((w) => `${w.op}:${w.object}`), + 'identity writes made OUTSIDE the SCIM scope', + ).toEqual([]); + }, 60_000); +}); + +// --------------------------------------------------------------------------- +// (b) — atomicity: a failed provisioning leaves no partial identity +// --------------------------------------------------------------------------- + +describe('[#14522] a provisioning that fails part-way leaves NO partial identity', () => { + it('(b) a failure on the sys_scim_user write rolls the sys_user and sys_scim_subject writes back', async () => { + const h = await boot(); + // Control: nothing is there before the request. + for (const object of IDENTITY_OBJECTS) expect(await rowsOf(h, object)).toBe(0); + + const o = observe(h, 'sys_scim_user'); + const res = await h.send(provisionRequest(h, 'bob')); + // The vendor reports the failure — it must not be a 201 over a torn write. + expect(res.status, await res.clone().text()).toBeGreaterThanOrEqual(400); + // The failing write was reached, i.e. the earlier ones had already run. + expect(o.writes.some((w) => w.object === 'sys_scim_user'), describeWrites(o)).toBe(true); + expect(o.writes.some((w) => w.object === 'sys_user'), describeWrites(o)).toBe(true); + + // Then the rollback: none of the three survives. + expect(await userRow(h, 'bob@example.com'), 'sys_user survived the failed provisioning').toBeNull(); + for (const object of IDENTITY_OBJECTS) { + expect(await rowsOf(h, object), `${object} rows after the failed provisioning`).toBe(0); + } + }, 60_000); +}); + +// --------------------------------------------------------------------------- +// (c) + (d) — negative controls +// --------------------------------------------------------------------------- + +async function signUp(h: Harness, email: string): Promise { + return h.send( + new Request(`${AUTH}/sign-up/email`, { + method: 'POST', + headers: { 'content-type': 'application/json', origin: BASE }, + body: JSON.stringify({ email, password: PASSWORD, name: 'Carol Example' }), + }), + ); +} + +async function signIn(h: Harness, email: string): Promise { + return h.send( + new Request(`${AUTH}/sign-in/email`, { + method: 'POST', + headers: { 'content-type': 'application/json', origin: BASE }, + body: JSON.stringify({ email, password: PASSWORD }), + }), + ); +} + +describe('[#14522] the scope is SCIM-only — non-SCIM flows keep their sequential posture', () => { + it('(c) sign-up and sign-in write identity rows with ZERO engine transactions and no SCIM scope', async () => { + const h = await boot(); + const o = observe(h); + + const up = await signUp(h, 'carol@example.com'); + expect(up.status, `sign-up failed: ${await up.clone().text()}`).toBeLessThan(300); + const down = await signIn(h, 'carol@example.com'); + expect(down.status, `sign-in failed: ${await down.clone().text()}`).toBeLessThan(300); + + // The flows really wrote (user, account, session) — a control with no + // writes would prove nothing. + expect(o.writes.length, describeWrites(o)).toBeGreaterThanOrEqual(3); + expect(o.writes.some((w) => w.object === 'sys_user'), describeWrites(o)).toBe(true); + expect(o.writes.some((w) => w.object === 'sys_session'), describeWrites(o)).toBe(true); + // ...and none of it inside a transaction or the SCIM scope. + expect(o.transaction, `engine.transaction calls during sign-up/sign-in: ${describeWrites(o)}`).not.toHaveBeenCalled(); + expect(o.beginTransaction).not.toHaveBeenCalled(); + expect(o.writes.filter((w) => w.scim).map((w) => `${w.op}:${w.object}`)).toEqual([]); + }, 60_000); + + it('(d) a SCIM read opens no transaction', async () => { + const h = await boot(); + const alice = await provision(h, 'alice'); + const o = observe(h); + + const res = await h.send(scimRequest(h, 'GET', `/Users/${alice.scimId}`)); + expect(res.status, await res.clone().text()).toBe(200); + expect(o.transaction).not.toHaveBeenCalled(); + expect(o.beginTransaction).not.toHaveBeenCalled(); + expect(o.writes).toEqual([]); + }, 60_000); +}); diff --git a/packages/plugins/plugin-auth/src/user-ban-write.ts b/packages/plugins/plugin-auth/src/user-ban-write.ts index 4df1ffbe68..5aa8dfe3d4 100644 --- a/packages/plugins/plugin-auth/src/user-ban-write.ts +++ b/packages/plugins/plugin-auth/src/user-ban-write.ts @@ -24,7 +24,9 @@ * already hold one: better-auth's `internalAdapter` satisfies it directly; the * SCIM hook adapts the `DBTransactionAdapter` the vendor bound to its * transaction, so the write commits — or rolls back — with the SCIM mutation - * it belongs to once that transaction is real on this adapter (#14522). + * it belongs to (a real engine transaction on this adapter: the SCIM request + * scope is opened at `AuthManager.handleRequest`, pinned by + * `scim-transaction-scope.test.ts`). * * Session revocation is deliberately NOT part of the write: the admin mount * revokes explicitly, and the SCIM vendor revokes after its callback returns