Skip to content

Commit dba7c1d

Browse files
committed
fix(auth): scope the adapter's native transactions to SCIM protocol requests (#3653)
Measured twice on the unscoped variant: better-auth wraps whole request flows in adapter.transaction (runWithTransaction), so opening a real driver transaction around every sign-in/sign-up starved the single-connection sqlite pools — the dogfood showcase boot deadlocked on 'Acquire connection error' until the 180s hook timeout, in CI and reproduced locally on this branch, with 337 sibling dogfood tests green. The scim verifier now marks its request's async chain (AsyncLocalStorage enterWith), and config.transaction opens a real engine.transaction only inside that scope — exactly where assertNativeSCIMTransactions demands atomicity. Every other better-auth flow keeps the sequential behaviour it has always had under the factory's as-is fallback, so nothing existing weakens. Re-measured after the fix: the deadlocked dogfood file passes in 19s (13/13), the credential-at-rest suite (which drives a real SCIM 2.0 request through the scoped transaction over better-sqlite3) stays green, and the full plugin-auth suite is 81 files / 1660 tests green. Co-authored-by: Claude <noreply@anthropic.com>
1 parent da00d48 commit dba7c1d

3 files changed

Lines changed: 67 additions & 25 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3120,12 +3120,17 @@ export class AuthManager {
31203120
if (enabled.scim) {
31213121
await this.addOptionalPlugin(plugins, 'scim', async () => {
31223122
const { scim } = await import('@better-auth/scim');
3123-
const { verifyScimBearerToken } = await import('./scim-connection-service.js');
3123+
const { verifyScimBearerToken, scimRequestScope } = await import('./scim-connection-service.js');
31243124
const secret = this.resolveAuthSecret();
31253125
return scim({
31263126
connections: [],
31273127
authentication: {
31283128
verifyBearerToken: async (input) => {
3129+
// Mark the remainder of this request's async chain as a SCIM
3130+
// protocol request, so the adapter runs its provisioning writes
3131+
// inside a REAL engine transaction (see scimRequestScope's
3132+
// rationale in scim-connection-service.ts).
3133+
scimRequestScope.enterWith({ scim: true });
31293134
const engine = this.config.dataEngine;
31303135
if (!engine) return null; // no store to verify against — fail closed
31313136
return verifyScimBearerToken(engine as never, secret, input.token);

packages/plugins/plugin-auth/src/objectql-adapter.ts

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { createAdapterFactory } from 'better-auth/adapters';
55
import type { CleanedWhere, WhereOperator } from 'better-auth/adapters';
66
import { SystemObjectName } from '@objectstack/spec/system';
77
import { resolveAttributedUserId } from './auth-actor-attribution.js';
8+
import { inScimRequestScope } from './scim-connection-service.js';
89
import { adoptExistingMembership } from './adopt-membership.js';
910
import {
1011
filterRevokedSessionRows,
@@ -768,30 +769,34 @@ export function createObjectQLAdapterFactory(rawDataEngine: IDataEngine) {
768769
const remapWhere = (where: CleanedWhere[]): CleanedWhere[] =>
769770
where.map((c) => ({ ...c, field: camelToSnake(c.field) }));
770771

771-
// [#3653] NATIVE transactions. Stable `@better-auth/scim` refuses to mount
772-
// on an adapter whose `transaction` is the factory's sequential fallback
773-
// (`assertNativeSCIMTransactions` reads `adapterConfig.transaction` and
774-
// demands a function) — provisioning multi-writes must be atomic. The
775-
// implementation is ObjectQL's own `engine.transaction()`: it publishes the
776-
// handle into the engine's AMBIENT transaction store (ADR-0034), so every
777-
// engine call the raw methods below make inside the callback automatically
778-
// binds to the same connection/rollback scope — the trx adapter handed to
779-
// the callback is therefore the SAME wrapped adapter, captured at factory
780-
// time below.
772+
// [#3653] NATIVE transactions, SCOPED to SCIM protocol requests. Stable
773+
// `@better-auth/scim` refuses to mount on an adapter whose `transaction` is
774+
// the factory's sequential fallback (`assertNativeSCIMTransactions` reads
775+
// `adapterConfig.transaction` and demands a function) — provisioning
776+
// multi-writes must be atomic. The implementation is ObjectQL's own
777+
// `engine.transaction()`: it publishes the handle into the engine's AMBIENT
778+
// transaction store (ADR-0034), so every engine call the raw methods below
779+
// make inside the callback automatically binds to the same
780+
// connection/rollback scope — the trx adapter handed to the callback is
781+
// therefore the SAME wrapped adapter, captured at factory time below.
781782
//
782-
// ⚠️ Scope this honestly: better-auth routes its OWN multi-writes through
783-
// `adapter.transaction` too — sign-up (user + account) included, measured —
784-
// so this is the transaction path for EVERY better-auth flow, not a
785-
// scim-only seam, and it must keep the same degrade contract those flows
786-
// had under the factory's sequential fallback. Two declared degrades:
787-
// - an engine with no `transaction` API at all (test doubles, minimal
788-
// IDataEngine implementations) runs the callback directly — exactly the
789-
// factory's own `createAsIsTransaction` behaviour;
790-
// - a driver without `beginTransaction` follows the engine's OWN declared
791-
// contract (ADR-0119 D1): run directly, warn once (#4619). Every SQL
792-
// production driver has `beginTransaction`, so a real deployment's scim
793-
// provisioning is genuinely atomic; fail-closed here (`require: true`)
794-
// was measured to 500 every sign-up on the memory engine.
783+
// ⚠️ The scoping is load-bearing, measured twice, not a hedge. better-auth
784+
// wraps its OWN whole request flows in `adapter.transaction` too
785+
// (`runWithTransaction` — sign-in/sign-up included), and two unscoped
786+
// variants each broke a measured surface:
787+
// - `require: true` (fail closed on non-transactional drivers) 500'd every
788+
// sign-up on the memory engine — 275 plugin-auth tests red;
789+
// - unconditional real transactions starved the single-connection sqlite
790+
// pools: the dogfood showcase boot deadlocked on `Acquire connection
791+
// error` until the 180s hook timeout, in CI and reproduced locally.
792+
// Core better-auth flows never had native DB transactions here (the factory
793+
// default is the sequential as-is fallback), so they KEEP that historical
794+
// posture; the real transaction opens exactly where upstream's assertion
795+
// demands it — inside an authenticated SCIM protocol request, marked by the
796+
// auth manager's `verifyBearerToken` via `scimRequestScope`. Remaining
797+
// declared degrades on that path: an engine with no `transaction` API runs
798+
// the callback directly, and a driver without `beginTransaction` follows
799+
// the engine's ADR-0119 D1 warn-once degrade.
795800
let wrappedAdapter: unknown = null;
796801
const engineWithTx = rawDataEngine as unknown as {
797802
transaction?<T>(
@@ -819,8 +824,12 @@ export function createObjectQLAdapterFactory(rawDataEngine: IDataEngine) {
819824
// rather than hand the callback a null adapter.
820825
throw new Error('[objectql-adapter] transaction requested before the adapter was constructed');
821826
}
827+
// Non-SCIM better-auth flows keep their historical sequential
828+
// behaviour — see the #3653 scoping note above for the two measured
829+
// breakages that make this conditional load-bearing.
830+
if (!inScimRequestScope()) return cb(wrappedAdapter as never);
822831
if (typeof engineWithTx.transaction !== 'function') {
823-
// Declared degrade #1 (see the #3653 note above): no transaction API
832+
// Declared degrade (see the #3653 note above): no transaction API
824833
// on this engine — run directly, as the factory fallback would.
825834
return cb(wrappedAdapter as never);
826835
}

packages/plugins/plugin-auth/src/scim-connection-service.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,34 @@
3838
*/
3939

4040
import { createHmac, randomBytes } from 'node:crypto';
41+
import { AsyncLocalStorage } from 'node:async_hooks';
42+
43+
/**
44+
* Request-scoped marker: "the current async chain is a SCIM protocol
45+
* request". Entered by the auth manager's `verifyBearerToken` wrapper (the
46+
* first application code every authenticated SCIM request runs) via
47+
* `enterWith`, so it holds for the remainder of that request's async chain —
48+
* including the provisioning writes the plugin performs afterwards.
49+
*
50+
* Read by `objectql-adapter.ts`'s `config.transaction`: SCIM requests get a
51+
* REAL engine transaction (the atomicity upstream's
52+
* `assertNativeSCIMTransactions` exists to demand), while every other
53+
* better-auth flow keeps the sequential behaviour it has always had. The
54+
* scoping is load-bearing, measured, not a convenience: better-auth wraps
55+
* WHOLE request flows in `adapter.transaction` (`runWithTransaction`), and
56+
* opening a real driver transaction around every sign-in/sign-up starved the
57+
* single-connection sqlite pools — the dogfood showcase boot deadlocked on
58+
* `Acquire connection error` until the hook timeout, reproduced in CI and
59+
* locally (#3653). Core flows never had native DB transactions before (the
60+
* adapter factory's default is the sequential as-is fallback), so this keeps
61+
* them at their historical posture rather than weakening anything.
62+
*/
63+
export const scimRequestScope = new AsyncLocalStorage<{ scim: true }>();
64+
65+
/** Is the current async chain inside an authenticated SCIM protocol request? */
66+
export function inScimRequestScope(): boolean {
67+
return scimRequestScope.getStore()?.scim === true;
68+
}
4169

4270
/** The ObjectStack-owned credential store (see platform-objects/identity). */
4371
export const SCIM_CREDENTIAL_OBJECT = 'sys_scim_connection_credential';

0 commit comments

Comments
 (0)