Skip to content

Commit 27f04ea

Browse files
committed
refactor(plugin-auth): move AuthManager's request routing into auth-manager-request-routing.ts
Three method bodies move unedited: handleRequest, the one seam every better-auth request passes through on its way to the vendor handler and back; ownsRoute, which answers whether better-auth routes a request at all; and betterAuthEndpointPath, the endpoint path both key on. The class keeps all three methods, their docblocks and signatures, and hands each body the members it reads; the route-ownership memo stays on the manager. isScimProtocolPath stays in auth-manager.ts, where the SCIM provisioning decision record anchors it, and is exported there for the new module; index.ts does not publish it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LSXBc2GCBwiAL7vV4kWkyr
1 parent 05cc3ee commit 27f04ea

2 files changed

Lines changed: 361 additions & 233 deletions

File tree

Lines changed: 318 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,318 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#25224 cut 10] The request routing of `AuthManager` — the one seam every
5+
* better-auth request passes through on its way to the vendor handler and
6+
* back, the question of whether better-auth routes a request at all, and the
7+
* endpoint path both of them key on — moved out of `auth-manager.ts` whole.
8+
*
9+
* A MOVE, not a redesign. Each function below is the body of the
10+
* `AuthManager` method of the same name as it stood, re-indented one level
11+
* and otherwise unedited: the same client address established once, the same
12+
* vendor header and dev-only `Origin`, the same admin-session recovery on the
13+
* way out of impersonation, the same request-state and actor-attribution
14+
* scopes, the same SCIM scope and atomic erasure, the same refusal envelopes
15+
* and the same server-error log, the same ownership table and its memo.
16+
*
17+
* ## How the moved bodies reach the class
18+
*
19+
* The shape #23050 set for `rest-server.ts` (its `data-action-routes.ts`
20+
* header carries the full reasoning), which `auth-manager-actor-guards.ts`,
21+
* `auth-manager-sign-in-lockout.ts`, `auth-manager-password-session-policy.ts`,
22+
* `auth-manager-database-hooks.ts`, `auth-manager-email-sms-delivery.ts`,
23+
* `auth-manager-plugin-assembly.ts`, `auth-manager-database-secret.ts`,
24+
* `auth-manager-membership-admission.ts` and `auth-manager-mcp-access-token.ts`
25+
* follow, applied per method. The class keeps all three methods — the
26+
* published `dist/*.d.ts` lists each one — and each body is one literal of
27+
* exactly the {@link RequestRoutingHost} members that method reads, plus a
28+
* `.call`. `handleRequest` and `ownsRoute` are no longer `async`: each returns
29+
* the moved function's own promise, so a caller awaits the same promise the
30+
* body settles, with no extra wrapper.
31+
*
32+
* A method goes as a closure that calls it on the manager at call time, so a
33+
* test double or spy set on the manager is still the one a body reaches —
34+
* `betterAuthEndpointPath` included, which `handleRequest` and `ownsRoute` ask
35+
* through the manager rather than calling the function here directly.
36+
* `routeOwnership` and `routeOwnershipFor` go as get/set pairs, because
37+
* `ownsRoute` stores the table it builds, and the instance it built it from,
38+
* on the manager: the memo stays the one the manager holds, rebuilt when that
39+
* instance is replaced, as before.
40+
*
41+
* `isScimProtocolPath` stays: the SCIM provisioning decision record anchors
42+
* it, and the prefix it reads, on `auth-manager.ts`. So that file exports it
43+
* and this module imports it by VALUE, for one read inside `handleRequest`,
44+
* when a request is handled. `index.ts` lists `auth-manager.ts`'s published
45+
* names one by one, so that export is not published. The two modules import
46+
* each other, and that is safe because nothing reads the binding while they
47+
* are first evaluated: this module's top level is imports, an interface and
48+
* function declarations.
49+
* `ORGANIZATION_DELETE_PATH` is imported by both modules, because the
50+
* before-hook in `auth-manager.ts` reads it too.
51+
*
52+
* Nothing re-exports this module, so the published surface does not move.
53+
*/
54+
55+
import { refuseAnonymousSession } from './anonymous-session-refusal.js';
56+
import { runWithAuthActorScope } from './auth-actor-attribution.js';
57+
import { isScimProtocolPath, type AuthManager } from './auth-manager.js';
58+
import { buildBetterAuthRouteOwnership } from './better-auth-route-ownership.js';
59+
import { withVendorClientAddress } from './client-address-vendor.js';
60+
import {
61+
ADMIN_SESSION_COOKIE_KEY,
62+
STOP_IMPERSONATING_PATH,
63+
withBearerAdminSessionRecovery,
64+
} from './impersonation-bearer-rotation.js';
65+
import { ORGANIZATION_DELETE_PATH } from './organization-delete.js';
66+
import { clientAddressStateEntries } from './request-client-address.js';
67+
import { SESSION_ERASURE_PATHS } from './session-tombstone.js';
68+
import { envelopeVendorAdminRefusal } from './vendor-admin-refusal-envelope.js';
69+
70+
/**
71+
* The `AuthManager` members the functions below read — every one of them, and
72+
* no other; each function's `this` is the `Pick` of the members IT reads. Each
73+
* is typed AS the class member (an indexed access on the class), so a change
74+
* to a member's signature on the class is a change here too, and naming a
75+
* member the class does not have is a compile error. See the module header for
76+
* why `routeOwnership` and `routeOwnershipFor` are accessors.
77+
*/
78+
export interface RequestRoutingHost {
79+
routeOwnership: AuthManager['routeOwnership'];
80+
routeOwnershipFor: AuthManager['routeOwnershipFor'];
81+
betterAuthEndpointPath: AuthManager['betterAuthEndpointPath'];
82+
getBasePath: AuthManager['getBasePath'];
83+
getOrCreateAuth: AuthManager['getOrCreateAuth'];
84+
resolveClientAddress: AuthManager['resolveClientAddress'];
85+
runSubjectErasureAtomically: AuthManager['runSubjectErasureAtomically'];
86+
}
87+
88+
/** The body of `AuthManager#handleRequest`; the method's docblock there is its contract. */
89+
export async function handleRequest(this: Pick<RequestRoutingHost, 'betterAuthEndpointPath' | 'getOrCreateAuth' | 'resolveClientAddress' | 'runSubjectErasureAtomically'>, request: Request, transport?: { remoteAddress?: string }): Promise<Response> {
90+
// ADR-0069 D7 — the client address this request's sign-in audit stamps,
91+
// established ONCE, here, by the same trust rule the D5 allow-list judges
92+
// with (`resolveClientAddress`). Read before any rewrite below, and carried
93+
// to the after-hook in better-auth's request state (`request-client-address.ts`).
94+
const clientHeaders: Record<string, string> = {};
95+
request.headers.forEach((value, name) => {
96+
clientHeaders[name] = value;
97+
});
98+
const clientAddress = this.resolveClientAddress({
99+
headers: clientHeaders,
100+
...(transport?.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}),
101+
});
102+
103+
// The vendored library's OWN client-address reader (session `ip_address`,
104+
// the sign-in audit fingerprint, its per-address rate limiter) reads only
105+
// the platform-owned header (`vendorClientAddressOptions`, composed in
106+
// `createAuthInstance`). Written HERE, from the address just established,
107+
// after dropping any copy the caller sent — so the library answers the same
108+
// address the D5 allow-list judged, or none. See `client-address-vendor.ts`.
109+
request = withVendorClientAddress(request, clientAddress);
110+
111+
// Dev DX: better-auth's CSRF protection rejects state-changing requests
112+
// (e.g. `/sign-in/email`) with a 403 when neither `Origin` nor `Referer`
113+
// is present. Browsers always send one; non-browser API clients (curl,
114+
// fetch from a script, integration tests) often don't, so they hit an
115+
// opaque 403 in local dev. A request with *no* Origin header is never a
116+
// browser-driven cross-site attack — CSRF is fundamentally cross-origin —
117+
// so in non-production we synthesize a same-origin `Origin` from the
118+
// request URL. It matches the dev localhost trusted-origins set, so the
119+
// CSRF check passes without weakening protection in production (gated on
120+
// NODE_ENV, the same dev signal used for the fallback auth secret above).
121+
if (
122+
process.env.NODE_ENV !== 'production' &&
123+
!request.headers.get('origin') &&
124+
!request.headers.get('referer')
125+
) {
126+
try {
127+
const headers = new Headers(request.headers);
128+
headers.set('origin', new URL(request.url).origin);
129+
request = new Request(request, { headers });
130+
} catch {
131+
/* malformed URL — leave the request untouched */
132+
}
133+
}
134+
135+
const auth = await this.getOrCreateAuth();
136+
137+
// [#8243] Let a bearer client carry the `admin_session` recovery credential
138+
// back out of impersonation. better-auth's `/admin/stop-impersonating`
139+
// resolves the admin through the `admin_session` COOKIE alone, so in a
140+
// cookie-blocked deployment — the exact context `bearer()` exists for — the
141+
// exit path is dead. We accept the credential on a header and write it into
142+
// the request's own `Cookie` before better-auth sees it; the vendor route
143+
// then runs completely unmodified, checking everything it always checked.
144+
//
145+
// The REQUEST seam, not a before-hook: `bearer()`'s before-hook rebuilds
146+
// the header set from `c.request.headers`, so a `Cookie` injected by any
147+
// hook is clobbered by whichever hook sorts after it. Written into the
148+
// request itself, `bearer()`'s parse-mutate-serialize keeps it.
149+
if (this.betterAuthEndpointPath(request) === STOP_IMPERSONATING_PATH) {
150+
try {
151+
const authContext: any = await (auth as any).$context;
152+
const adminCookieName: string | undefined =
153+
authContext?.createAuthCookie?.(ADMIN_SESSION_COOKIE_KEY)?.name;
154+
if (adminCookieName) {
155+
request = await withBearerAdminSessionRecovery(request, adminCookieName);
156+
}
157+
} catch {
158+
// Cookie name unresolvable (e.g. a dynamic-baseURL context we cannot
159+
// reach here) → leave the request alone. The vendor route then answers
160+
// exactly as it does today for a missing cookie: a loud failure, never
161+
// a silent wrong identity.
162+
}
163+
}
164+
165+
// better-auth's HTTP entrypoint (`createBetterAuth.handler`) wraps execution
166+
// in `runWithAdapter` but NOT `runWithRequestState`. Endpoints that read
167+
// request-state via `defineRequestState()` (e.g. `should-session-refresh`,
168+
// `oauth`) therefore throw "No request state found" when reached via HTTP.
169+
// The `customSession` plugin triggers this by invoking the inner
170+
// `getSession()` endpoint directly, bypassing `to-auth-endpoints`'
171+
// auto-wrap. We establish the ALS store here so all downstream endpoint
172+
// calls inherit a valid request-state WeakMap.
173+
const { runWithRequestState } = await import('@better-auth/core/context');
174+
// [#4586] Open the actor-attribution scope around the WHOLE request, so
175+
// every write better-auth makes on the way — adapter writes, the
176+
// membership reconciler in `user.create.after`, anything a plugin hook
177+
// triggers — can be credited to the human who made the request. Opening it
178+
// costs nothing: the scope starts empty, the before-hook drops a resolver
179+
// in, and the session is looked up only if some write asks. Attribution
180+
// only — the authorization subject of those writes is unchanged (system).
181+
// `await`, not a bare return: both scope helpers are generic over their
182+
// callback, so the composed call is typed `Promise< Promise< Response > >`.
183+
// The previous single call site flattened it with the `await` below.
184+
const runHandler = async (): Promise<Response> =>
185+
await runWithAuthActorScope(() =>
186+
runWithRequestState(
187+
new WeakMap<object, unknown>(clientAddressStateEntries(clientAddress)),
188+
() => auth.handler(request),
189+
),
190+
);
191+
192+
// [#7724] A subject-erasure request is ONE unit of work, and better-auth
193+
// does not treat it as one: `internalAdapter.deleteUser` deletes the
194+
// sessions, then the accounts, then the user, in three unrelated adapter
195+
// calls with no transaction. Measured 2026-08-20 against the installed
196+
// better-auth 1.7.1: `deleteUser` at `dist/db/internal-adapter.mjs:233-247`
197+
// is three separate `deleteManyWithHooks` / `deleteWithHooks` calls
198+
// (session `:235`, account `:239`, user `:243`), and the string
199+
// "transaction" does not occur anywhere in that file — a zero result that
200+
// means something because "deleteManyWithHooks" occurs 8 times in it, so
201+
// the search does reach the text. (The session call is now conditional on
202+
// `!secondaryStorage || storeSessionInDatabase`; ObjectStack wires no
203+
// `secondaryStorage` — deliberately, see `session-tombstone.ts` — so all
204+
// three fire here.) Anything
205+
// that refuses the LAST of those three leaves the first two committed: the
206+
// credential rows are gone, the `sys_user` row is not, and the deployment
207+
// is left with an identity that still occupies the org roster and can no
208+
// longer sign in. Nothing tells the operator, and there is no way back.
209+
const endpointPath = this.betterAuthEndpointPath(request);
210+
211+
// [#3653 / #14522] A SCIM protocol request (`/scim/v2/*`) runs inside
212+
// `scimRequestScope`, which is what makes the adapter's `transaction`
213+
// config open a REAL engine transaction around the vendor's provisioning
214+
// multi-writes (`objectql-adapter.ts`, the scoping note there). Opened
215+
// HERE, with `run(...)` around the whole request, for the same reason the
216+
// actor-attribution scope above is: `run` has a callback boundary that
217+
// every `als.run` the vendor performs underneath nests inside. The stamp
218+
// used to be an `enterWith` inside the SCIM plugin's `verifyBearerToken`
219+
// callback, and it never reached the writes — the vendor resumes the
220+
// endpoint handler from a continuation captured before the verifier ran
221+
// (measured on 1.7.2: zero `engine.transaction` calls across
222+
// `POST /Users` + `PATCH /Users/{id}`). Keyed on the endpoint path prefix
223+
// so it is exactly as narrow as before — SCIM protocol requests only; the
224+
// non-SCIM flows keep their sequential posture, which the scoping note
225+
// records as load-bearing. Pinned by `scim-transaction-scope.test.ts`.
226+
const runRequest = isScimProtocolPath(endpointPath)
227+
? async (): Promise<Response> => {
228+
const { scimRequestScope } = await import('./scim-connection-service.js');
229+
return scimRequestScope.run({ scim: true }, runHandler);
230+
}
231+
: runHandler;
232+
// [#24948] An organization delete is one unit of work on the same terms:
233+
// better-auth clears the caller's active organization and deletes the
234+
// members and invitations before the organization row, with no
235+
// transaction, and the engine's refusal on that last row used to leave the
236+
// organization standing with no owner.
237+
const vendorResponse =
238+
endpointPath !== undefined &&
239+
(SESSION_ERASURE_PATHS.has(endpointPath) || endpointPath === ORGANIZATION_DELETE_PATH)
240+
? await this.runSubjectErasureAtomically(runRequest)
241+
: await runRequest();
242+
243+
// [#10349] The better-auth-native `/admin/` routes refuse an anonymous
244+
// caller through the vendor's `adminMiddleware`
245+
// (`APIError.fromStatus('UNAUTHORIZED')`, no body argument), so the refusal
246+
// reaches the client as a 401 that announces `application/json` and carries
247+
// the EMPTY STRING — no envelope, nothing to branch on. The ObjectStack raw
248+
// `/admin/*` mounts answer the identical question with the ADR-0112
249+
// envelope and `code: 'UNAUTHENTICATED'` (`platform-admin-gate.ts`), and
250+
// which of the two a caller gets depends only on which implementation
251+
// happens to serve that route — an implementation detail, not a contract.
252+
//
253+
// This is the ONE seam every vendor route passes through, which is why the
254+
// normalization belongs here and not in ten routes we do not own. It is
255+
// scoped to the `/admin/` NAMESPACE (option C): the prefix test costs no new
256+
// concept, because this method already discriminates on `endpointPath` twice
257+
// above — `STOP_IMPERSONATING_PATH` and `SESSION_ERASURE_PATHS`.
258+
//
259+
// Status and admission are untouched; see the module header for the three
260+
// narrowings and the measurement behind each.
261+
const enveloped = await envelopeVendorAdminRefusal(endpointPath, vendorResponse);
262+
263+
// [#17238] And the anonymous `/get-session` answer becomes the same
264+
// declared refusal. better-auth serves `200` + the literal JSON `null`
265+
// when no session backs the request, which is a value no
266+
// `SessionResponse` can express — so `ObjectStackClient.auth.me()`, which
267+
// declares `Promise<SessionResponse>`, resolved OUTSIDE its own type on
268+
// the most ordinary call a logged-out caller makes. Ruled by the director
269+
// seat (batch #117 item 4): the code moves, the published schema does not.
270+
//
271+
// Same seam and same reason as the `/admin/` normalization above — this is
272+
// the ONE place every vendor route passes through — but ⚠️ NOT the same
273+
// kind of change: that one is forbidden to move admission and this one IS
274+
// the admission move (`200` -> `401`). `anonymous-session-refusal.ts`
275+
// carries the three narrowings that keep every other answer identical.
276+
const response = await refuseAnonymousSession(endpointPath, enveloped);
277+
278+
if (response.status >= 500) {
279+
try {
280+
const body = await response.clone().text();
281+
console.error('[AuthManager] better-auth returned error:', response.status, body);
282+
} catch {
283+
console.error('[AuthManager] better-auth returned error:', response.status, '(unable to read body)');
284+
}
285+
}
286+
287+
return response;
288+
}
289+
290+
/** The body of `AuthManager#ownsRoute`; the method's docblock there is its contract. */
291+
export async function ownsRoute(this: Pick<RequestRoutingHost, 'betterAuthEndpointPath' | 'getOrCreateAuth' | 'routeOwnership' | 'routeOwnershipFor'>, request: Request): Promise<boolean> {
292+
const endpointPath = this.betterAuthEndpointPath(request);
293+
if (endpointPath === undefined) return false;
294+
try {
295+
const auth = await this.getOrCreateAuth();
296+
if (this.routeOwnershipFor !== auth || !this.routeOwnership) {
297+
this.routeOwnership = buildBetterAuthRouteOwnership((auth as any)?.api);
298+
this.routeOwnershipFor = auth;
299+
}
300+
return this.routeOwnership.owns(request.method, endpointPath);
301+
} catch {
302+
return false;
303+
}
304+
}
305+
306+
/** The body of `AuthManager#betterAuthEndpointPath`; the method's docblock there is its contract. */
307+
export function betterAuthEndpointPath(this: Pick<RequestRoutingHost, 'getBasePath'>, request: Request): string | undefined {
308+
let pathname: string;
309+
try {
310+
pathname = new URL(request.url).pathname;
311+
} catch {
312+
return undefined;
313+
}
314+
const base = this.getBasePath();
315+
if (!pathname.startsWith(base)) return undefined;
316+
const endpoint = pathname.slice(base.length).replace(/\/+$/, '');
317+
return endpoint.startsWith('/') ? endpoint : undefined;
318+
}

0 commit comments

Comments
 (0)