From 3ec293d86af8f1fc76bdc9dfa14908bc14dd6383 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 21:47:43 +0000 Subject: [PATCH] refactor(plugin-auth): move the better-auth global before-hook into auth-manager-before-hook.ts The body of the arrow createAuthInstance passes to createAuthMiddleware for hooks.before moves unedited into globalBeforeHook, re-indented four levels. hooks.before keeps its place in the config literal; its arrow now builds one host of the nineteen AuthManager members the body reads (eighteen methods as late-bound closures, config as a getter) and returns globalBeforeHook.call(host, ctx, setShouldSkipSessionRefresh). The async moved from the arrow to the function. The one createAuthInstance local the arrow read, setShouldSkipSessionRefresh, is the function's second parameter under the same name; the body's one static read, AuthManager.OPERATOR_PROVISIONING_TICKET_FIELD, is a value import of the class, read at call time. Nine imports leave auth-manager.ts with it. Two readers outside the move follow it: the api-error-code pin counts the new module among the auth manager's files, and the vocabulary gate's APIError argument census names it as the live member's file. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01LSXBc2GCBwiAL7vV4kWkyr --- .../src/api-error-code.pin.test.ts | 1 + .../src/auth-manager-before-hook.ts | 696 ++++++++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 624 +--------------- scripts/check-dispatcher-error-vocabulary.mjs | 10 +- 4 files changed, 734 insertions(+), 597 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/auth-manager-before-hook.ts diff --git a/packages/plugins/plugin-auth/src/api-error-code.pin.test.ts b/packages/plugins/plugin-auth/src/api-error-code.pin.test.ts index d7f65b83e5c..21c9abefcd7 100644 --- a/packages/plugins/plugin-auth/src/api-error-code.pin.test.ts +++ b/packages/plugins/plugin-auth/src/api-error-code.pin.test.ts @@ -181,6 +181,7 @@ describe('every new APIError( in plugin-auth/src carries an ADR-0112 code (#2424 'auth-manager-sign-in-lockout.ts', 'auth-manager-password-session-policy.ts', 'auth-manager-email-sms-delivery.ts', + 'auth-manager-before-hook.ts', ]; expect(sites.length).toBeGreaterThanOrEqual(30); expect(new Set(sites.map((s) => s.file)).size).toBeGreaterThanOrEqual(4); diff --git a/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts b/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts new file mode 100644 index 00000000000..a844b3ca5af --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts @@ -0,0 +1,696 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25224 cut 13] The global before-hook of the better-auth instance + * `AuthManager` builds — the one `hooks.before` middleware + * `createAuthInstance` hands better-auth, which every endpoint's request + * passes through before the endpoint runs — moved out of `auth-manager.ts` + * whole. + * + * A MOVE, not a redesign. The function below is the body of the arrow + * `createAuthInstance` passes to `createAuthMiddleware` for `hooks.before`, as + * it stood, re-indented four levels and otherwise unedited: the same actor + * resolver handed to the attribution scope, the same refresh skip for a minted + * principal session, the same per-number OTP send guard, the same password + * complexity and reuse checks, the same change-email capture, the same + * remove-member, organization-delete and no-record revoke refusals, the same + * held-caller refusal on the admin plugin's role-scalar actions, the same + * no-record admin revoke refusal, the same held-caller refusal on rewriting or + * deleting an identity provider, the same identity-provider registration gate, + * the same cloud-as-IdP authorization gate, the same break-glass + * last-local-credential refusal, the same audience posture and uniqueness + * refusal on password sign-up, the same cloud-managed invite-member refusal, + * the same account lockout gate, and the same first-user `disableSignUp` flip + * — in the same order. + * + * ## How the moved body reaches the class + * + * The shape #23050 set for `rest-server.ts` (its `data-action-routes.ts` + * header carries the full reasoning), which the twelve `auth-manager-*.ts` + * modules split out before this one follow, applied to a closure as + * `auth-manager-after-hook.ts` applies it. `createAuthInstance` keeps its + * place, its docblock and its signature, and `hooks.before` keeps its place in + * the config literal: the arrow there is now one literal of exactly the + * {@link BeforeHookHost} members the body reads, plus a `.call`. The `async` + * moved from that arrow to the function below, and the arrow returns the + * function's own promise, so the middleware awaits the same promise the body + * settles, with no extra wrapper. + * + * The body's `this` was the manager, the arrow's lexical receiver; it is now + * that host. Every member it reads but one is a method, and each goes as a + * closure that calls it on the manager at call time, so a test double or spy + * set on the manager is still the one the body reaches. `config` goes as a + * GETTER, because the class reassigns it after construction + * (`setRuntimeBaseUrl` and `applyConfigPatch`), so the body reads the + * manager's current one. The + * arrow builds the host each time better-auth runs the hook, as every method + * stub in this series does, so nothing the body reads is captured early. + * + * The arrow read one local of `createAuthInstance`: + * `setShouldSkipSessionRefresh`, which the method takes from + * `better-auth/api` before it builds the literal. It is the function's second + * parameter, under the same name, and the arrow passes it on each call. The + * body reads one static, `AuthManager.OPERATOR_PROVISIONING_TICKET_FIELD` + * (public), so this module imports the class by value, as + * `auth-manager-membership-admission.ts` does; the one read is inside the + * function, at call time. + * + * Nothing re-exports this module, so the published surface does not move. + */ + +import { isCloudManaged } from '@objectstack/core'; +import { VENDOR_ROLE_SCALAR_ADMIN_PATHS } from './admin-impersonate-endpoint.js'; +import { setAuthActorResolver } from './auth-actor-attribution.js'; +import { AuthManager } from './auth-manager.js'; +import { SIGN_IN_EMAIL_PATH } from './auth-session-audit.js'; +import { CLOUD_MANAGED_REFUSAL_CODE, CLOUD_MANAGED_REFUSAL_MESSAGES } from './cloud-managed.js'; +import { + isLastLocalCredentialHolder, + LAST_LOCAL_CREDENTIAL_CODE, + LAST_LOCAL_CREDENTIAL_MESSAGE, +} from './last-local-credential.js'; +import { ORGANIZATION_DELETE_PATH } from './organization-delete.js'; +import { carriesPrincipalSession } from './principal-session-mint.js'; + +/** + * The `AuthManager` members the function below reads — every one of them, and + * no other. Each is typed AS the class member (an indexed access on the + * class), so a change to a member's signature on the class is a change here + * too, and naming a member the class does not have is a compile error. + * `config` is read-only here: the host hands it out through a getter. + */ +export interface BeforeHookHost { + assertAccountNotLocked: AuthManager['assertAccountNotLocked']; + assertActorNotHeld: AuthManager['assertActorNotHeld']; + assertAdminRevokeUserSessionIdentifiesRecord: AuthManager['assertAdminRevokeUserSessionIdentifiesRecord']; + assertOrganizationDeletable: AuthManager['assertOrganizationDeletable']; + assertPasswordComplexity: AuthManager['assertPasswordComplexity']; + assertPasswordNotReused: AuthManager['assertPasswordNotReused']; + assertPhoneOtpSendAllowed: AuthManager['assertPhoneOtpSendAllowed']; + assertRemoveMemberPermitted: AuthManager['assertRemoveMemberPermitted']; + assertRevokeSessionIdentifiesRecord: AuthManager['assertRevokeSessionIdentifiesRecord']; + readonly config: AuthManager['config']; + existingAddressSignUpAnswersSuccess: AuthManager['existingAddressSignUpAnswersSuccess']; + hasExistingUserFor: AuthManager['hasExistingUserFor']; + isAuthGateActive: AuthManager['isAuthGateActive']; + isBootstrapCreation: AuthManager['isBootstrapCreation']; + isOperatorProvisioning: AuthManager['isOperatorProvisioning']; + isPlatformAdminUserId: AuthManager['isPlatformAdminUserId']; + resolveActor: AuthManager['resolveActor']; + resolvePasswordChangeUserId: AuthManager['resolvePasswordChangeUserId']; + validateAudienceAdmission: AuthManager['validateAudienceAdmission']; +} + +/** + * The body of `createAuthInstance`'s `hooks.before` arrow; the comments there + * and in it are its contract. `setShouldSkipSessionRefresh` is the + * `better-auth/api` export of that name the arrow read from `createAuthInstance`. + */ +export async function globalBeforeHook( + this: BeforeHookHost, + ctx: any, + setShouldSkipSessionRefresh: typeof import('better-auth/api').setShouldSkipSessionRefresh, +) { + // ── #4586: hand the attribution scope its actor resolver ───── + // FIRST, and unconditionally: this global before-hook is the one + // seam every better-auth endpoint passes through (`matcher: () => + // true`, plugin routes included), and it is the only layer where + // the session actor exists at all. Registering here is O(1) — the + // resolver is STORED, not run; the session lookup happens only if + // a write later asks who to credit, and at most once per request. + // + // What travels is `attributedUserId`, which no security middleware + // reads. The adapter's writes stay `isSystem` (see + // `withSystemContext`): better-auth already authorized them under + // its own ACL, and re-authorizing as the human would open the + // second adjudication track ADR-0095 D3 closed. + setAuthActorResolver(async () => { + const actor = await this.resolveActor(ctx); + return actor?.userId ?? null; + }); + + // ── #23813: a minted principal session is never refreshed ─────── + // Also FIRST: the switch must be set before anything in this + // request reads the session (the actor resolver above may, later). + // A renewal of a minted session writes the row and re-sets its + // cookie, so `bearer()` answers `set-auth-token` on every request + // and the console re-resolves identity forever. Request-scoped + // (better-auth's own state), so it reaches the nested session reads + // of every endpoint on both lanes and in-process — see + // `principal-session-mint.ts` (3b). + if (carriesPrincipalSession(ctx)) await setShouldSkipSessionRefresh(true); + + // ── #2780: per-number OTP send guard (admission control) ───── + // MUST run BEFORE the phone-number endpoints: better-auth's + // send-otp handler stores a fresh code and only THEN invokes + // `sendOTP` — a guard that throws inside the callback would + // still rotate (invalidate) the previously delivered code, so a + // blocked resend (or an attacker spamming the endpoint) could + // keep voiding the user's valid OTP. Rejecting here leaves the + // stored code untouched. Applies uniformly to registered and + // unregistered numbers (no account-existence oracle). + if ( + ctx?.path === '/phone-number/send-otp' || + ctx?.path === '/phone-number/request-password-reset' + ) { + const phone = typeof ctx?.body?.phoneNumber === 'string' ? ctx.body.phoneNumber : ''; + await this.assertPhoneOtpSendAllowed(phone); + } + + // ── ADR-0069 D1: password complexity (validator) ──────────── + // better-auth enforces only min/max length; class-mix is custom. + // Runs on the password-mutating endpoints; reads the candidate from + // the path-appropriate body field (sign-up: `password`; reset / + // change: `newPassword`). + if ( + ctx?.path === '/sign-up/email' || + ctx?.path === '/reset-password' || + ctx?.path === '/change-password' + ) { + const candidate = + (typeof ctx?.body?.password === 'string' && ctx.body.password) || + (typeof ctx?.body?.newPassword === 'string' && ctx.body.newPassword) || + ''; + if (candidate) await this.assertPasswordComplexity(candidate); + + // ── ADR-0069 D1: password history (reject reuse) ──────────── + // change/reset only (sign-up has no prior history). Reuses + // better-auth's native password.verify — no bespoke crypto. Stashes + // the old hash so the after-hook appends it to the bounded ring on + // success. + if ( + candidate && + (ctx?.path === '/reset-password' || ctx?.path === '/change-password') + ) { + const userId = await this.resolvePasswordChangeUserId(ctx).catch(() => undefined); + if (userId) { + // Stash for the after-hook (password_changed_at stamp), regardless + // of whether history is enabled. + ctx.context.__osPwChangeUserId = userId; + const pw = ctx?.context?.password; + const verify = typeof pw?.verify === 'function' ? pw.verify.bind(pw) : undefined; + const oldHash = await this.assertPasswordNotReused(userId, candidate, verify); + if (oldHash !== undefined) ctx.context.__osPwHistory = { userId, oldHash }; + } + } + // fall through to the path's own handling below + } + + // ── #8019: capture the address a change-email would move AWAY from ── + // The notice goes out from the after-hook, but the OLD address only + // exists BEFORE the endpoint runs — and `ctx.context.session` is not + // it: better-auth's own `sensitiveSessionMiddleware` populates that, + // and endpoint middleware runs AFTER this global before-hook, so the + // field is still empty here. Resolve the session explicitly, the same + // hook-order-independent way `/oauth2/authorize` below has to. + // + // Stashed, never sent from here: at this point the request has not + // been accepted yet (it can still fail on `newEmail` equal to the + // current address, on rate limiting, or on a missing transport), and + // a notice for a request better-auth then refuses would be a false + // alarm about an identity move that never happened. + if (ctx?.path === '/change-email') { + try { + const { getSessionFromCtx } = await import('better-auth/api'); + const s: any = await getSessionFromCtx(ctx as any).catch(() => null); + const from = s?.user?.email; + if (typeof from === 'string' && from) { + ctx.context.__osChangeEmailFrom = { + email: from, + ...(typeof s?.user?.name === 'string' && s.user.name ? { name: s.user.name } : {}), + ...(typeof s?.user?.id === 'string' && s.user.id ? { id: s.user.id } : {}), + }; + } + } catch { + // Unresolvable session → no stash → no notice. better-auth's own + // session middleware still rejects the request with 401, so this + // never silently drops a notice for a change that proceeds. + } + } + + // ── #8289: remove-member answers its PERMISSION denial itself ── + // better-auth's `removeMember` orders "only an owner may remove an + // owner" AHEAD of its real permission check and reports it with the + // sole-owner invariant's code and a 400, so a caller who merely lacks + // permission is told they "cannot leave the organization as the only + // owner" — every clause false, and a 400 where every sibling denial is + // a 403. Answer the permission class here instead; the sole-owner + // invariant and every 200 path stay the vendor's, untouched. + // `remove-member-permission-guard.ts` carries the full reading, + // including why this MUST be a before-hook (an after-hook cannot + // change the status) and why the guard's refusal set is exactly the + // vendor's. + if (ctx?.path === '/organization/remove-member') { + await this.assertRemoveMemberPermitted(ctx); + // fall through — the vendor still re-decides everything it owns + } + + // ── #24948: organization delete refuses while records remain ── + // Before the vendor's handler, so a refused delete writes nothing: + // the vendor clears the caller's active organization and deletes the + // members before its own `beforeDeleteOrganization` hook runs. + // `organization-delete.ts` carries the contract. + if (ctx?.path === ORGANIZATION_DELETE_PATH) { + await this.assertOrganizationDeletable(ctx); + // fall through — the vendor still authorizes and deletes + } + + // ── #9714: a revoke that identifies NO record must not report ── + // success. better-auth 1.7.1's `revoke-session` handler skips the + // delete when the token matches zero rows (or another user's row) + // and still answers `200 { status: true }` — its success line is + // unconditional. Refuse those requests HERE, with the vendor's own + // admission predicate asked through the vendor's own adapter call, + // so the handler only ever runs on requests whose success answer is + // true. Before-hook on purpose: an after-hook cannot change the + // status. `revoke-session-match-guard.ts` carries the full reading, + // including the 404-over-`{status:false}` decision and the + // no-existence-oracle property. + if (ctx?.path === '/revoke-session') { + await this.assertRevokeSessionIdentifiesRecord(ctx); + // fall through — the vendor still performs the revoke itself + } + + // ── ADR-0069 D3: the admin plugin's role-scalar actions refuse a ── + // caller the authentication policy holds. better-auth serves these + // itself and authorizes them on a session plus the legacy `role` + // scalar, so no ObjectStack judge runs and nothing on the path read + // the posture: measured on a real boot, a held caller carrying that + // scalar was admitted to every one of them, writes included. The + // refusal is the session gate's, asked of the session gate's own + // posture carrier (`assertActorNotHeld`), and it runs FIRST — before + // the vendor's authorization and before the record guard below — the + // order the session gate applies. No resolvable session falls through + // to the vendor's own 401. `VENDOR_ROLE_SCALAR_ADMIN_PATHS` + // (`admin-impersonate-endpoint.ts`) classifies every other path the + // plugin registers. With no gate feature active the posture holds no + // one, so the session read is skipped, as the transport seams skip it + // (`isAuthGateActive`, the same caches the computation reads). + if (VENDOR_ROLE_SCALAR_ADMIN_PATHS.has(ctx?.path) && this.isAuthGateActive()) { + const actor = await this.resolveActor(ctx); + if (actor?.userId) await this.assertActorNotHeld(actor); + // fall through — the vendor still authorizes everything it owns + } + + // ── #10069: the ADMIN revoke that identifies NO record must not ── + // report success either. better-auth 1.7.1's `admin/revoke-user- + // session` handler calls `deleteSession(ctx.body.sessionToken)` + // blindly and answers `200 { success: true }` unconditionally — + // measured on this pipeline: zero-match AND already-revoked tokens + // both answered success. NOT the #9714 predicate: no ownership + // dimension here (the caller is an admin acting on arbitrary + // users) — the question is only "does any session carry this + // token", and it is asked AFTER the vendor's own permission + // question so non-admins keep the vendor's 401/403 and gain no + // existence oracle. Before-hook on purpose: an after-hook cannot + // change the status. `admin-revoke-user-session-match-guard.ts` + // carries the full reading. + if (ctx?.path === '/admin/revoke-user-session') { + await this.assertAdminRevokeUserSessionIdentifiesRecord(ctx); + // fall through — the vendor still performs the revoke itself + } + + // ── ADR-0069 D3: nor may a held caller rewrite or delete one ────── + // `@better-auth/sso` lets a provider's OWNER (who, past the + // registration gate below, is a platform admin) update its issuer, + // endpoints and secrets or delete it, on a session alone. Measured on + // a real boot: a held platform admin who owned a provider rewrote its + // issuer to another host (200, landed) and deleted another (200, row + // gone) — login routing, rewritten by a caller the policy holds. The + // same refusal as registration, asked the same way; the vendor still + // decides who may manage the provider. + if ( + (ctx?.path === '/sso/update-provider' || ctx?.path === '/sso/delete-provider') && + this.isAuthGateActive() + ) { + const actor = await this.resolveActor(ctx); + if (actor?.userId) await this.assertActorNotHeld(actor); + // fall through — the vendor's own provider-access check still runs + } + + // ── ADR-0135 D6 + ADR-0068 D4: registering an identity provider is ── + // a PLATFORM-OPERATOR action. + // + // `@better-auth/sso`'s POST /sso/register only checks org-admin when + // `body.organizationId` is present (index.mjs: `if (ctx.body + // .organizationId) { … hasOrgAdminRole … }`). A GLOBAL (org-less) + // provider therefore passes with nothing but a valid session — so any + // authenticated member can register an env-wide external IdP, a JIT- + // provisioning / login-routing vector. So ObjectStack decides the + // authorization here, regardless of whether `organizationId` is + // supplied. + // + // [#10009] The admit set is PLATFORM ADMIN ONLY — the same posture + // #9653 landed on the `/admin/sso/*` bridges. It previously ALSO + // admitted an owner/admin of the caller's active org, and that gap + // made the bridge tightening honest labelling rather than a boundary: + // one principal, refused 403 at `/admin/sso/register` and admitted + // here, for the same underlying registration. Maintainer ruling + // 2026-08-20 closed the wider door (ADR-0068 D4 — platform-operator + // actions gate on the platform admin, sole operator). An org-scoped + // self-serve channel for org-scoped providers is a deliberate FUTURE + // ruling if multi-org IdP self-serve becomes a product goal — it is + // not the vendor default, inherited by omission. + // + // Unauthenticated requests fall through to better-auth's + // `sessionMiddleware` (→ 401). Fail-CLOSED: an unverifiable actor is + // denied. (D5.1's `/oauth2/authorize` gate is a different surface — + // the OP issuing codes, not the env's RP config.) + // + // ADR-0069 D3: a caller the authentication policy holds is refused + // FIRST, with the session gate's refusal (`assertActorNotHeld`) — + // identity, then posture, then the platform-admin question, the order + // `judgePlatformAdmin` applies on the `/admin/sso/*` bridges. Measured + // on a real boot before this: a held platform admin's registration + // answered `200` and the provider row landed, in every held state. + if (ctx?.path === '/sso/register') { + const actor = await this.resolveActor(ctx); + if (actor?.userId) { + await this.assertActorNotHeld(actor); + const ok = await this.isPlatformAdminUserId(actor.userId); + if (!ok) { + const { APIError } = await import('better-auth/api'); + throw new APIError('FORBIDDEN', { + message: 'Only a platform admin can register an SSO provider.', + code: 'SSO_REGISTER_FORBIDDEN', + }); + } + } + return; + } + + // ── D5.1: cloud-as-IdP authorization gate ─────────────────── + // On the OIDC OP's /oauth2/authorize, when a host gate is set + // (cloud control plane), an AUTHENTICATED subject must be + // authorized for the requesting client (env) before a code is + // issued — this enforces org-membership (app-assignment). Unset + // (open editions / self-host) → no gate. Unauthenticated → fall + // through so the OP redirects to login; the gate runs on the + // return pass (or immediately for a bearer/cookie session). + // + // [#8102] The acting subject is resolved through the shared, + // hook-order-independent {@link resolveActor} — NOT a second inline + // copy of it. This branch used to carry its own token lookup, line + // for line the same logic, and the two diverged the moment one was + // fixed: #8101 taught `resolveActor` to strip the signature from a + // bearer credential (`session.token` stores the UNSIGNED value while + // `bearer()` hands clients the SIGNED form in `set-auth-token`), and + // the copy here kept looking the signed credential up verbatim. It + // resolved nothing — and because the unresolved case is deliberately + // fail-open, an AUTHENTICATED caller on the documented API lane was + // read as unauthenticated and this env-access check never evaluated + // at all. Two resolution sites are what let them diverge, so the fix + // is to delete one, not to correct it in place. + if (ctx?.path === '/oauth2/authorize' && this.config.oidcAuthorizeGate) { + const clientId = ctx?.query?.client_id; + if (clientId) { + const actor = await this.resolveActor(ctx); + // Unauthenticated → fall through, per the contract above. Only an + // AUTHENTICATED subject is gated here. `resolveActor` also returns + // `activeOrgId`; this gate deliberately does not consume it — the + // D5.1 host contract is `(userId, clientId)` and the control plane + // derives org membership / app assignment from the user itself. + if (actor?.userId) { + const allowed = await this.config.oidcAuthorizeGate({ + userId: actor.userId, + clientId: String(clientId), + }); + if (!allowed) { + const { APIError } = await import('better-auth/api'); + throw new APIError('FORBIDDEN', { + message: 'You are not authorized to sign in to this environment.', + code: 'ENV_ACCESS_DENIED', + }); + } + } + } + return; + } + + // ── Break-glass: never remove the LAST local-password login ────── + // Under enforced SSO the managed team holds no local credential; the + // env owner / a local admin keeps one as the break-glass escape hatch + // so an IdP outage can never lock the org out. Refuse to delete or + // ban the last user holding a `credential` account. Generic over the + // IdP. Managed (credential-less) users are unaffected. Fail-open on + // lookup hiccups (a transient query error must not block legit ops). + if ( + ctx?.path === '/delete-user' || + ctx?.path === '/admin/remove-user' || + ctx?.path === '/admin/ban-user' + ) { + // ⚠️ `/admin/ban-user` no longer reaches this hook — #9652 mounts an + // ObjectStack raw route on that path ahead of the catch-all (the + // ADR-0068 gate better-auth cannot express), so the request never + // enters better-auth's router. The path stays listed here because + // the mount is conditional on the admin plugin, and because the + // guard itself now lives in ONE module both call sites share — + // `last-local-credential.ts`, whose header records this trap. + // + // ⚠️ `/admin/remove-user` IS ALSO SHADED NOW (commit 6dd3e6968) — and it DOES + // still reach this hook, which is the opposite of the line above + // and is the point. Its mount only runs `gateAdmin` and then + // RE-DISPATCHES the request through `handleRequest`, so it re-enters + // better-auth's router and this hook fires exactly as before — + // just AFTER authorization instead of before it. That is the whole + // fix: an authenticated non-admin is now refused by the mount and + // never reaches the lookup below, while an admitted platform admin + // reaches the identical lookup and the identical `CONFLICT`. + // ⛔ Do not "reconcile" the two notes by deleting this path from + // the list — that would silently drop the guard on the one route + // that still depends on this hook to run it. + // ── [#10776] AUTHENTICATE FIRST ──────────────────────────── + // A `hooks.before` runs AHEAD of the endpoint's own + // `use: [adminMiddleware]`, and that middleware is the only layer + // establishing identity on this lane. So this guard used to read an + // unauthenticated request's body, ask the database a question about + // the named user, and answer it — a per-record answer to a caller + // nobody had authenticated, where every sibling `/admin/` route + // answers 401. Resolving the actor first makes the guard's decision + // (and its distinctive refusal) reachable only once the caller has + // an identity; an anonymous caller falls through to the vendor's own + // `sessionMiddleware`/`adminMiddleware` and hears the ordinary 401. + // + // Maintainer ruling 2026-08-22 (decision-inbox digest, accepted + // verbatim 「接受所有」): option A, authentication before the guard. + // + // ⚠️ This changes WHEN the guard decides, never WHAT it decides: + // an authenticated caller reaches exactly the same lookup and the + // same `CONFLICT`. `break-glass-guard-authentication-order.test.ts` + // pins both halves — the disclosure closing AND the invariant + // surviving — because closing the first by deleting the guard would + // satisfy a disclosure-only suite. + // + // Same shape as the `/oauth2/authorize` gate above: unauthenticated + // → fall through, never a refusal invented here. + const breakGlassActor = await this.resolveActor(ctx); + + let isLastLocalCredential = false; + if (breakGlassActor?.userId) { + try { + // `/delete-user` names no target in the vendor's own contract: + // the subject IS the authenticated caller. That must hold + // UNCONDITIONALLY — a body-supplied `userId` on this + // self-service route is never the target, present or not. + // (A prior `if (!targetId && …)` form let a body value win + // whenever one was supplied, turning the guard's refusal into + // a per-record oracle over the break-glass holder for any + // authenticated caller, not just the caller acting on + // themselves. `/admin/remove-user` and `/admin/ban-user` + // legitimately name a target in their own contract, so only + // this route's resolution is keyed on the actor instead of + // the body.) + const targetId: string | undefined = + ctx.path === '/delete-user' + ? breakGlassActor.userId + : (ctx?.body?.userId ?? ctx?.body?.user_id); + if (targetId) { + isLastLocalCredential = await isLastLocalCredentialHolder( + ctx.context.adapter, + targetId, + ); + } + } catch { + // Fail-open — never block a legitimate op on a lookup error. + } + } + if (isLastLocalCredential) { + const { APIError } = await import('better-auth/api'); + throw new APIError('CONFLICT', { + message: LAST_LOCAL_CREDENTIAL_MESSAGE, + code: LAST_LOCAL_CREDENTIAL_CODE, + }); + } + // fall through to better-auth's own handler + } + + // ── [#11739] Audience posture on the PASSWORD sign-up route ── + // The DECISION is validateAudienceAdmission — the same single owner + // the user.validateUserInfo gate calls for every creation path. + // This route alone needs the refusal raised HERE, before the + // handler: better-auth's sign-up route wraps createUser in an + // anti-enumeration shield (dist/api/routes/sign-up.mjs:163,235 — + // `shouldReturnGenericDuplicateResponse`, on whenever + // requireEmailVerification is on) that converts ANY 403 from the + // creation seam into a synthetic 200 "success". Correct for + // account-existence oracles; wrong for an audience refusal, which + // is deterministic per (posture, domain, invitation) and reveals + // nothing about any account — swallowed, it would turn the domain + // gate's refusal into a silent black hole (fake success, no + // verification mail ever). Measured: with posture `open`/ + // `email_domain` (verification forced on) the validateUserInfo + // refusal came back 200 `{token:null,user:{synthetic}}`. The + // validateUserInfo gate stays wired for this route too — the + // decision is idempotent, so the second ask cannot disagree. + if (ctx?.path === '/sign-up/email') { + const refusal = await this.validateAudienceAdmission( + { + user: { + ...(typeof ctx?.body?.email === 'string' + ? { email: ctx.body.email.toLowerCase() } + : {}), + }, + source: { action: 'create-user', method: 'email-password' }, + }, + ctx, + ); + if (refusal) { + const { APIError } = await import('better-auth/api'); + throw new APIError('FORBIDDEN', { + code: refusal.error, + message: refusal.errorDescription ?? refusal.error, + }); + } + + // ── [#15587] The UNIQUENESS refusal, raised here for the same + // reason the audience refusal above is — and STRICTLY after it. + // + // Same shield, its other arm. `shouldReturnGenericDuplicateResponse` + // (better-auth 1.7.12 sign-up.mjs:155) is on whenever + // `requireEmailVerification` is on OR `autoSignIn === false`, and it + // guards TWO sites: the 403 catch at :227 (what the audience block + // above steps around) and the duplicate pre-check at :192–204. On + // the second, the vendor finds the + // existing row, logs it, and returns `buildGenericDuplicateResponse()` + // — a 200 carrying a freshly `generateId()`-ed user that is never + // written — INSTEAD of throwing USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL. + // + // We turn that shield on ourselves: a posture that permits + // self-registration forces `requireEmailVerification` on by default + // (see `createAuthInstance` — always under `email_domain`, and under + // `open` unless the deployment declared it off, #20389), so + // `email_domain` and `open` sign-ups for an address that already + // exists answered 200 while `invite_only` answered 422 on the same + // population. Measured on a real ObjectQL + // engine with the posture held CONSTANT and only the verification flag + // moved, so the divergence is the flag's, not the posture's: zero + // inserts reach the engine, no `sys_account` appears, and the next + // sign-in is 401 with nothing anywhere explaining it. The silent + // success shape, on the recovery path a locked-out deployment walks. + // + // ORDER IS LOAD-BEARING: this runs only for a caller the posture + // ALREADY ADMITTED. Asking it first would hand an uninvited stranger + // an account-existence oracle under the `invite_only` DEFAULT (422 for + // a real address vs 403 for an unknown one) — inventing on the closed + // posture exactly what the vendor's shield exists to prevent. After + // the gate, `invite_only` is untouched: a stranger still gets + // SELF_REGISTRATION_CLOSED and learns nothing. + // + // [#25060, ruling 1B on #25040] ONE exception, decided here and + // nowhere else ({@link existingAddressSignUpAnswersSuccess}): under + // `open` and `email_domain`, where the vendor's shield is on and a + // mail transport is live, the request FALLS THROUGH. The vendor then + // finds the row, answers its generic success — the same shape a new + // sign-up gets there, and no row, credential or session is written — + // and its `onExistingUserSignUp` hook mails the address's holder + // ({@link sendExistingAccountSignUpNotice}). Every other case keeps + // the explicit refusal: `invite_only`, a shield that is off (a new + // sign-up's answer there carries a session, which an existing + // address cannot mirror without one for someone else's account), and + // a transport that is not live (a success with no mail is the black + // hole this refusal replaced). The refusal is the platform's at this + // one seam, so it never depends on the vendor also refusing; where + // the vendor would have answered, its code and message are the + // vendor's OWN constant, so the two lanes are byte-identical by + // construction rather than by copying. + const signUpAddress = typeof ctx?.body?.email === 'string' ? ctx.body.email : ''; + if (signUpAddress && (await this.hasExistingUserFor(signUpAddress))) { + if (!this.existingAddressSignUpAnswersSuccess(ctx?.context?.options?.emailAndPassword)) { + const { APIError, BASE_ERROR_CODES } = await import('@better-auth/core/error'); + throw APIError.from( + 'UNPROCESSABLE_ENTITY', + BASE_ERROR_CODES.USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL, + ); + } + // The answer will carry a user id that no row holds: tell the + // after-hook, so it stamps nothing on that id (it would report a + // lost write for a user that was never created). + if (ctx?.context) ctx.context.__osExistingAddressSignUp = true; + } + // fall through — the vendor still decides everything it owns + } + + // ── [#25083] cloud-managed: invite-member issues nothing ──── + // `beforeCreateInvitation` refuses a NEW invitation, but + // better-auth answers `resend: true` for an invitee who is + // already pending BEFORE it consults that hook: it extends the + // row's `expiresAt` and sends the invitation e-mail again + // (`organization/routes/crud-invites.mjs`, better-auth 1.7.12). + // The console reaches exactly that branch through + // `sys_invitation`'s `resend_invitation` action. On a + // cloud-managed environment no invitation leaves this door in + // either shape, so the route refuses ahead of the vendor's + // handler; the hook keeps its own arm as the second line. The + // fact is public (`features.cloudManaged` on `/auth/config`), so + // answering it ahead of the session check tells an anonymous + // caller nothing new. + if (ctx?.path === '/organization/invite-member' && isCloudManaged()) { + const { APIError } = await import('better-auth/api'); + throw new APIError('CONFLICT', { + message: CLOUD_MANAGED_REFUSAL_MESSAGES.invitation, + code: CLOUD_MANAGED_REFUSAL_CODE, + }); + } + + // ── ADR-0069 D2: account lockout (gate) ───────────────────── + // Reject a sign-in for a locked identity BEFORE better-auth checks + // the password — a lock must hold even against the correct password. + // The refusal is a refused sign-in, so it writes its `login_failed` + // row (`auth-session-audit.ts`, account events) before it throws. + if (ctx?.path === SIGN_IN_EMAIL_PATH) { + const email = typeof ctx?.body?.email === 'string' ? ctx.body.email : ''; + if (email) await this.assertAccountNotLocked(email, ctx); + return; + } + + if (ctx?.path !== '/sign-up/email') return; + const ep = ctx?.context?.options?.emailAndPassword; + if (!ep?.disableSignUp) return; + // [#11767] Same population question, same owner. This site used to + // ask `ctx.context.adapter.findOne({ model: 'user', where: [] })`, + // which the real ObjectQL engine REFUSES (#4419 — + // `requireFindOnePredicate`) rather than answers; the surrounding + // `catch` read that refusal as "users exist" and the declared + // bypass never fired on a real deployment. `isBootstrapCreation` + // owns the answerable form — see its doc. + // [#14157] …and the deployment's own boot command provisioning its + // admin is the OTHER way through this door. It is not a + // self-registration and must not depend on a self-registration + // carve-out: an app that seeds people makes the bootstrap probe + // answer "populated" before the seed ever runs. Cheap synchronous + // check first, so the ticket path costs no I/O. + // [#14373] The email alone no longer admits — see + // `stageOperatorProvisioning`'s doc. `ctx.body` is the vendor's own + // zod-validated body, which keeps any key outside the declared + // schema (`signUpEmailBodySchema.and(z.record(...))`), so the + // in-process seed call's ticket field survives here untouched. + const signUpEmail = typeof ctx?.body?.email === 'string' ? ctx.body.email : undefined; + const provisioningTicket = ctx?.body?.[AuthManager.OPERATOR_PROVISIONING_TICKET_FIELD]; + if ( + this.isOperatorProvisioning(signUpEmail, provisioningTicket) || + (await this.isBootstrapCreation()) + ) { + ctx.context.__osDisableSignUpOrig = ep.disableSignUp; + ep.disableSignUp = false; + } +} diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 101ff51792c..4017a18955e 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -73,9 +73,7 @@ import { type ExternalDirectoryDoorDeps } from './external-directory-door.js'; import { isExternalMembershipRole } from './external-membership.js'; import { permissionSetInEffect } from './catalog-set-in-effect.js'; import { recoverInternalFieldsForSystemRead } from './internal-field-readback.js'; -import { setAuthActorResolver } from './auth-actor-attribution.js'; import { - SIGN_IN_EMAIL_PATH, type AuthEventAuditSurface, } from './auth-session-audit.js'; import { @@ -83,10 +81,8 @@ import { } from './better-auth-route-ownership.js'; import { applyPlatformAdminImpersonation, - VENDOR_ROLE_SCALAR_ADMIN_PATHS, } from './admin-impersonate-endpoint.js'; import { - carriesPrincipalSession, mintPrincipalSession, type PrincipalSessionAuthContext, } from './principal-session-mint.js'; @@ -108,11 +104,6 @@ import { import type { TenancyService } from './tenancy-service.js'; import { OtpSendGuard, assertOtpCooldownSeconds } from './otp-send-guard.js'; import type { CounterStore } from './rate-limit-storage.js'; -import { - isLastLocalCredentialHolder, - LAST_LOCAL_CREDENTIAL_CODE, - LAST_LOCAL_CREDENTIAL_MESSAGE, -} from './last-local-credential.js'; import { applyUserBan, applyUserUnban, @@ -132,7 +123,6 @@ import { buildPhoneNumberPluginSchema, } from './auth-schema-config.js'; import { - ORGANIZATION_DELETE_PATH, isDefaultOrganizationTarget, removeOrganizationAuthStructure, } from './organization-delete.js'; @@ -252,6 +242,10 @@ import { resolvePasswordHasher } from './auth-manager-password-hasher.js'; // `hooks.after` arrow in `createAuthInstance` hands it the members it reads // (`AfterHookHost`). import { globalAfterHook, type AfterHookHost } from './auth-manager-after-hook.js'; +// [#25224 cut 13] The global before-hook of the better-auth instance; the +// `hooks.before` arrow in `createAuthInstance` hands it the members it reads +// (`BeforeHookHost`) and the one local it read. +import { globalBeforeHook, type BeforeHookHost } from './auth-manager-before-hook.js'; /** * Detect WebContainer (StackBlitz) environment. @@ -2242,589 +2236,33 @@ export class AuthManager { // request's context. Once the owner is created the next request // sees `userCount > 0` and the toggle is enforced again. hooks: { - before: createAuthMiddleware(async (ctx: any) => { - // ── #4586: hand the attribution scope its actor resolver ───── - // FIRST, and unconditionally: this global before-hook is the one - // seam every better-auth endpoint passes through (`matcher: () => - // true`, plugin routes included), and it is the only layer where - // the session actor exists at all. Registering here is O(1) — the - // resolver is STORED, not run; the session lookup happens only if - // a write later asks who to credit, and at most once per request. - // - // What travels is `attributedUserId`, which no security middleware - // reads. The adapter's writes stay `isSystem` (see - // `withSystemContext`): better-auth already authorized them under - // its own ACL, and re-authorizing as the human would open the - // second adjudication track ADR-0095 D3 closed. - setAuthActorResolver(async () => { - const actor = await this.resolveActor(ctx); - return actor?.userId ?? null; - }); - - // ── #23813: a minted principal session is never refreshed ─────── - // Also FIRST: the switch must be set before anything in this - // request reads the session (the actor resolver above may, later). - // A renewal of a minted session writes the row and re-sets its - // cookie, so `bearer()` answers `set-auth-token` on every request - // and the console re-resolves identity forever. Request-scoped - // (better-auth's own state), so it reaches the nested session reads - // of every endpoint on both lanes and in-process — see - // `principal-session-mint.ts` (3b). - if (carriesPrincipalSession(ctx)) await setShouldSkipSessionRefresh(true); - - // ── #2780: per-number OTP send guard (admission control) ───── - // MUST run BEFORE the phone-number endpoints: better-auth's - // send-otp handler stores a fresh code and only THEN invokes - // `sendOTP` — a guard that throws inside the callback would - // still rotate (invalidate) the previously delivered code, so a - // blocked resend (or an attacker spamming the endpoint) could - // keep voiding the user's valid OTP. Rejecting here leaves the - // stored code untouched. Applies uniformly to registered and - // unregistered numbers (no account-existence oracle). - if ( - ctx?.path === '/phone-number/send-otp' || - ctx?.path === '/phone-number/request-password-reset' - ) { - const phone = typeof ctx?.body?.phoneNumber === 'string' ? ctx.body.phoneNumber : ''; - await this.assertPhoneOtpSendAllowed(phone); - } - - // ── ADR-0069 D1: password complexity (validator) ──────────── - // better-auth enforces only min/max length; class-mix is custom. - // Runs on the password-mutating endpoints; reads the candidate from - // the path-appropriate body field (sign-up: `password`; reset / - // change: `newPassword`). - if ( - ctx?.path === '/sign-up/email' || - ctx?.path === '/reset-password' || - ctx?.path === '/change-password' - ) { - const candidate = - (typeof ctx?.body?.password === 'string' && ctx.body.password) || - (typeof ctx?.body?.newPassword === 'string' && ctx.body.newPassword) || - ''; - if (candidate) await this.assertPasswordComplexity(candidate); - - // ── ADR-0069 D1: password history (reject reuse) ──────────── - // change/reset only (sign-up has no prior history). Reuses - // better-auth's native password.verify — no bespoke crypto. Stashes - // the old hash so the after-hook appends it to the bounded ring on - // success. - if ( - candidate && - (ctx?.path === '/reset-password' || ctx?.path === '/change-password') - ) { - const userId = await this.resolvePasswordChangeUserId(ctx).catch(() => undefined); - if (userId) { - // Stash for the after-hook (password_changed_at stamp), regardless - // of whether history is enabled. - ctx.context.__osPwChangeUserId = userId; - const pw = ctx?.context?.password; - const verify = typeof pw?.verify === 'function' ? pw.verify.bind(pw) : undefined; - const oldHash = await this.assertPasswordNotReused(userId, candidate, verify); - if (oldHash !== undefined) ctx.context.__osPwHistory = { userId, oldHash }; - } - } - // fall through to the path's own handling below - } - - // ── #8019: capture the address a change-email would move AWAY from ── - // The notice goes out from the after-hook, but the OLD address only - // exists BEFORE the endpoint runs — and `ctx.context.session` is not - // it: better-auth's own `sensitiveSessionMiddleware` populates that, - // and endpoint middleware runs AFTER this global before-hook, so the - // field is still empty here. Resolve the session explicitly, the same - // hook-order-independent way `/oauth2/authorize` below has to. - // - // Stashed, never sent from here: at this point the request has not - // been accepted yet (it can still fail on `newEmail` equal to the - // current address, on rate limiting, or on a missing transport), and - // a notice for a request better-auth then refuses would be a false - // alarm about an identity move that never happened. - if (ctx?.path === '/change-email') { - try { - const { getSessionFromCtx } = await import('better-auth/api'); - const s: any = await getSessionFromCtx(ctx as any).catch(() => null); - const from = s?.user?.email; - if (typeof from === 'string' && from) { - ctx.context.__osChangeEmailFrom = { - email: from, - ...(typeof s?.user?.name === 'string' && s.user.name ? { name: s.user.name } : {}), - ...(typeof s?.user?.id === 'string' && s.user.id ? { id: s.user.id } : {}), - }; - } - } catch { - // Unresolvable session → no stash → no notice. better-auth's own - // session middleware still rejects the request with 401, so this - // never silently drops a notice for a change that proceeds. - } - } - - // ── #8289: remove-member answers its PERMISSION denial itself ── - // better-auth's `removeMember` orders "only an owner may remove an - // owner" AHEAD of its real permission check and reports it with the - // sole-owner invariant's code and a 400, so a caller who merely lacks - // permission is told they "cannot leave the organization as the only - // owner" — every clause false, and a 400 where every sibling denial is - // a 403. Answer the permission class here instead; the sole-owner - // invariant and every 200 path stay the vendor's, untouched. - // `remove-member-permission-guard.ts` carries the full reading, - // including why this MUST be a before-hook (an after-hook cannot - // change the status) and why the guard's refusal set is exactly the - // vendor's. - if (ctx?.path === '/organization/remove-member') { - await this.assertRemoveMemberPermitted(ctx); - // fall through — the vendor still re-decides everything it owns - } - - // ── #24948: organization delete refuses while records remain ── - // Before the vendor's handler, so a refused delete writes nothing: - // the vendor clears the caller's active organization and deletes the - // members before its own `beforeDeleteOrganization` hook runs. - // `organization-delete.ts` carries the contract. - if (ctx?.path === ORGANIZATION_DELETE_PATH) { - await this.assertOrganizationDeletable(ctx); - // fall through — the vendor still authorizes and deletes - } - - // ── #9714: a revoke that identifies NO record must not report ── - // success. better-auth 1.7.1's `revoke-session` handler skips the - // delete when the token matches zero rows (or another user's row) - // and still answers `200 { status: true }` — its success line is - // unconditional. Refuse those requests HERE, with the vendor's own - // admission predicate asked through the vendor's own adapter call, - // so the handler only ever runs on requests whose success answer is - // true. Before-hook on purpose: an after-hook cannot change the - // status. `revoke-session-match-guard.ts` carries the full reading, - // including the 404-over-`{status:false}` decision and the - // no-existence-oracle property. - if (ctx?.path === '/revoke-session') { - await this.assertRevokeSessionIdentifiesRecord(ctx); - // fall through — the vendor still performs the revoke itself - } - - // ── ADR-0069 D3: the admin plugin's role-scalar actions refuse a ── - // caller the authentication policy holds. better-auth serves these - // itself and authorizes them on a session plus the legacy `role` - // scalar, so no ObjectStack judge runs and nothing on the path read - // the posture: measured on a real boot, a held caller carrying that - // scalar was admitted to every one of them, writes included. The - // refusal is the session gate's, asked of the session gate's own - // posture carrier (`assertActorNotHeld`), and it runs FIRST — before - // the vendor's authorization and before the record guard below — the - // order the session gate applies. No resolvable session falls through - // to the vendor's own 401. `VENDOR_ROLE_SCALAR_ADMIN_PATHS` - // (`admin-impersonate-endpoint.ts`) classifies every other path the - // plugin registers. With no gate feature active the posture holds no - // one, so the session read is skipped, as the transport seams skip it - // (`isAuthGateActive`, the same caches the computation reads). - if (VENDOR_ROLE_SCALAR_ADMIN_PATHS.has(ctx?.path) && this.isAuthGateActive()) { - const actor = await this.resolveActor(ctx); - if (actor?.userId) await this.assertActorNotHeld(actor); - // fall through — the vendor still authorizes everything it owns - } - - // ── #10069: the ADMIN revoke that identifies NO record must not ── - // report success either. better-auth 1.7.1's `admin/revoke-user- - // session` handler calls `deleteSession(ctx.body.sessionToken)` - // blindly and answers `200 { success: true }` unconditionally — - // measured on this pipeline: zero-match AND already-revoked tokens - // both answered success. NOT the #9714 predicate: no ownership - // dimension here (the caller is an admin acting on arbitrary - // users) — the question is only "does any session carry this - // token", and it is asked AFTER the vendor's own permission - // question so non-admins keep the vendor's 401/403 and gain no - // existence oracle. Before-hook on purpose: an after-hook cannot - // change the status. `admin-revoke-user-session-match-guard.ts` - // carries the full reading. - if (ctx?.path === '/admin/revoke-user-session') { - await this.assertAdminRevokeUserSessionIdentifiesRecord(ctx); - // fall through — the vendor still performs the revoke itself - } - - // ── ADR-0069 D3: nor may a held caller rewrite or delete one ────── - // `@better-auth/sso` lets a provider's OWNER (who, past the - // registration gate below, is a platform admin) update its issuer, - // endpoints and secrets or delete it, on a session alone. Measured on - // a real boot: a held platform admin who owned a provider rewrote its - // issuer to another host (200, landed) and deleted another (200, row - // gone) — login routing, rewritten by a caller the policy holds. The - // same refusal as registration, asked the same way; the vendor still - // decides who may manage the provider. - if ( - (ctx?.path === '/sso/update-provider' || ctx?.path === '/sso/delete-provider') && - this.isAuthGateActive() - ) { - const actor = await this.resolveActor(ctx); - if (actor?.userId) await this.assertActorNotHeld(actor); - // fall through — the vendor's own provider-access check still runs - } - - // ── ADR-0135 D6 + ADR-0068 D4: registering an identity provider is ── - // a PLATFORM-OPERATOR action. - // - // `@better-auth/sso`'s POST /sso/register only checks org-admin when - // `body.organizationId` is present (index.mjs: `if (ctx.body - // .organizationId) { … hasOrgAdminRole … }`). A GLOBAL (org-less) - // provider therefore passes with nothing but a valid session — so any - // authenticated member can register an env-wide external IdP, a JIT- - // provisioning / login-routing vector. So ObjectStack decides the - // authorization here, regardless of whether `organizationId` is - // supplied. - // - // [#10009] The admit set is PLATFORM ADMIN ONLY — the same posture - // #9653 landed on the `/admin/sso/*` bridges. It previously ALSO - // admitted an owner/admin of the caller's active org, and that gap - // made the bridge tightening honest labelling rather than a boundary: - // one principal, refused 403 at `/admin/sso/register` and admitted - // here, for the same underlying registration. Maintainer ruling - // 2026-08-20 closed the wider door (ADR-0068 D4 — platform-operator - // actions gate on the platform admin, sole operator). An org-scoped - // self-serve channel for org-scoped providers is a deliberate FUTURE - // ruling if multi-org IdP self-serve becomes a product goal — it is - // not the vendor default, inherited by omission. - // - // Unauthenticated requests fall through to better-auth's - // `sessionMiddleware` (→ 401). Fail-CLOSED: an unverifiable actor is - // denied. (D5.1's `/oauth2/authorize` gate is a different surface — - // the OP issuing codes, not the env's RP config.) - // - // ADR-0069 D3: a caller the authentication policy holds is refused - // FIRST, with the session gate's refusal (`assertActorNotHeld`) — - // identity, then posture, then the platform-admin question, the order - // `judgePlatformAdmin` applies on the `/admin/sso/*` bridges. Measured - // on a real boot before this: a held platform admin's registration - // answered `200` and the provider row landed, in every held state. - if (ctx?.path === '/sso/register') { - const actor = await this.resolveActor(ctx); - if (actor?.userId) { - await this.assertActorNotHeld(actor); - const ok = await this.isPlatformAdminUserId(actor.userId); - if (!ok) { - const { APIError } = await import('better-auth/api'); - throw new APIError('FORBIDDEN', { - message: 'Only a platform admin can register an SSO provider.', - code: 'SSO_REGISTER_FORBIDDEN', - }); - } - } - return; - } - - // ── D5.1: cloud-as-IdP authorization gate ─────────────────── - // On the OIDC OP's /oauth2/authorize, when a host gate is set - // (cloud control plane), an AUTHENTICATED subject must be - // authorized for the requesting client (env) before a code is - // issued — this enforces org-membership (app-assignment). Unset - // (open editions / self-host) → no gate. Unauthenticated → fall - // through so the OP redirects to login; the gate runs on the - // return pass (or immediately for a bearer/cookie session). - // - // [#8102] The acting subject is resolved through the shared, - // hook-order-independent {@link resolveActor} — NOT a second inline - // copy of it. This branch used to carry its own token lookup, line - // for line the same logic, and the two diverged the moment one was - // fixed: #8101 taught `resolveActor` to strip the signature from a - // bearer credential (`session.token` stores the UNSIGNED value while - // `bearer()` hands clients the SIGNED form in `set-auth-token`), and - // the copy here kept looking the signed credential up verbatim. It - // resolved nothing — and because the unresolved case is deliberately - // fail-open, an AUTHENTICATED caller on the documented API lane was - // read as unauthenticated and this env-access check never evaluated - // at all. Two resolution sites are what let them diverge, so the fix - // is to delete one, not to correct it in place. - if (ctx?.path === '/oauth2/authorize' && this.config.oidcAuthorizeGate) { - const clientId = ctx?.query?.client_id; - if (clientId) { - const actor = await this.resolveActor(ctx); - // Unauthenticated → fall through, per the contract above. Only an - // AUTHENTICATED subject is gated here. `resolveActor` also returns - // `activeOrgId`; this gate deliberately does not consume it — the - // D5.1 host contract is `(userId, clientId)` and the control plane - // derives org membership / app assignment from the user itself. - if (actor?.userId) { - const allowed = await this.config.oidcAuthorizeGate({ - userId: actor.userId, - clientId: String(clientId), - }); - if (!allowed) { - const { APIError } = await import('better-auth/api'); - throw new APIError('FORBIDDEN', { - message: 'You are not authorized to sign in to this environment.', - code: 'ENV_ACCESS_DENIED', - }); - } - } - } - return; - } - - // ── Break-glass: never remove the LAST local-password login ────── - // Under enforced SSO the managed team holds no local credential; the - // env owner / a local admin keeps one as the break-glass escape hatch - // so an IdP outage can never lock the org out. Refuse to delete or - // ban the last user holding a `credential` account. Generic over the - // IdP. Managed (credential-less) users are unaffected. Fail-open on - // lookup hiccups (a transient query error must not block legit ops). - if ( - ctx?.path === '/delete-user' || - ctx?.path === '/admin/remove-user' || - ctx?.path === '/admin/ban-user' - ) { - // ⚠️ `/admin/ban-user` no longer reaches this hook — #9652 mounts an - // ObjectStack raw route on that path ahead of the catch-all (the - // ADR-0068 gate better-auth cannot express), so the request never - // enters better-auth's router. The path stays listed here because - // the mount is conditional on the admin plugin, and because the - // guard itself now lives in ONE module both call sites share — - // `last-local-credential.ts`, whose header records this trap. - // - // ⚠️ `/admin/remove-user` IS ALSO SHADED NOW (commit 6dd3e6968) — and it DOES - // still reach this hook, which is the opposite of the line above - // and is the point. Its mount only runs `gateAdmin` and then - // RE-DISPATCHES the request through `handleRequest`, so it re-enters - // better-auth's router and this hook fires exactly as before — - // just AFTER authorization instead of before it. That is the whole - // fix: an authenticated non-admin is now refused by the mount and - // never reaches the lookup below, while an admitted platform admin - // reaches the identical lookup and the identical `CONFLICT`. - // ⛔ Do not "reconcile" the two notes by deleting this path from - // the list — that would silently drop the guard on the one route - // that still depends on this hook to run it. - // ── [#10776] AUTHENTICATE FIRST ──────────────────────────── - // A `hooks.before` runs AHEAD of the endpoint's own - // `use: [adminMiddleware]`, and that middleware is the only layer - // establishing identity on this lane. So this guard used to read an - // unauthenticated request's body, ask the database a question about - // the named user, and answer it — a per-record answer to a caller - // nobody had authenticated, where every sibling `/admin/` route - // answers 401. Resolving the actor first makes the guard's decision - // (and its distinctive refusal) reachable only once the caller has - // an identity; an anonymous caller falls through to the vendor's own - // `sessionMiddleware`/`adminMiddleware` and hears the ordinary 401. - // - // Maintainer ruling 2026-08-22 (decision-inbox digest, accepted - // verbatim 「接受所有」): option A, authentication before the guard. - // - // ⚠️ This changes WHEN the guard decides, never WHAT it decides: - // an authenticated caller reaches exactly the same lookup and the - // same `CONFLICT`. `break-glass-guard-authentication-order.test.ts` - // pins both halves — the disclosure closing AND the invariant - // surviving — because closing the first by deleting the guard would - // satisfy a disclosure-only suite. - // - // Same shape as the `/oauth2/authorize` gate above: unauthenticated - // → fall through, never a refusal invented here. - const breakGlassActor = await this.resolveActor(ctx); - - let isLastLocalCredential = false; - if (breakGlassActor?.userId) { - try { - // `/delete-user` names no target in the vendor's own contract: - // the subject IS the authenticated caller. That must hold - // UNCONDITIONALLY — a body-supplied `userId` on this - // self-service route is never the target, present or not. - // (A prior `if (!targetId && …)` form let a body value win - // whenever one was supplied, turning the guard's refusal into - // a per-record oracle over the break-glass holder for any - // authenticated caller, not just the caller acting on - // themselves. `/admin/remove-user` and `/admin/ban-user` - // legitimately name a target in their own contract, so only - // this route's resolution is keyed on the actor instead of - // the body.) - const targetId: string | undefined = - ctx.path === '/delete-user' - ? breakGlassActor.userId - : (ctx?.body?.userId ?? ctx?.body?.user_id); - if (targetId) { - isLastLocalCredential = await isLastLocalCredentialHolder( - ctx.context.adapter, - targetId, - ); - } - } catch { - // Fail-open — never block a legitimate op on a lookup error. - } - } - if (isLastLocalCredential) { - const { APIError } = await import('better-auth/api'); - throw new APIError('CONFLICT', { - message: LAST_LOCAL_CREDENTIAL_MESSAGE, - code: LAST_LOCAL_CREDENTIAL_CODE, - }); - } - // fall through to better-auth's own handler - } - - // ── [#11739] Audience posture on the PASSWORD sign-up route ── - // The DECISION is validateAudienceAdmission — the same single owner - // the user.validateUserInfo gate calls for every creation path. - // This route alone needs the refusal raised HERE, before the - // handler: better-auth's sign-up route wraps createUser in an - // anti-enumeration shield (dist/api/routes/sign-up.mjs:163,235 — - // `shouldReturnGenericDuplicateResponse`, on whenever - // requireEmailVerification is on) that converts ANY 403 from the - // creation seam into a synthetic 200 "success". Correct for - // account-existence oracles; wrong for an audience refusal, which - // is deterministic per (posture, domain, invitation) and reveals - // nothing about any account — swallowed, it would turn the domain - // gate's refusal into a silent black hole (fake success, no - // verification mail ever). Measured: with posture `open`/ - // `email_domain` (verification forced on) the validateUserInfo - // refusal came back 200 `{token:null,user:{synthetic}}`. The - // validateUserInfo gate stays wired for this route too — the - // decision is idempotent, so the second ask cannot disagree. - if (ctx?.path === '/sign-up/email') { - const refusal = await this.validateAudienceAdmission( - { - user: { - ...(typeof ctx?.body?.email === 'string' - ? { email: ctx.body.email.toLowerCase() } - : {}), - }, - source: { action: 'create-user', method: 'email-password' }, - }, - ctx, - ); - if (refusal) { - const { APIError } = await import('better-auth/api'); - throw new APIError('FORBIDDEN', { - code: refusal.error, - message: refusal.errorDescription ?? refusal.error, - }); - } - - // ── [#15587] The UNIQUENESS refusal, raised here for the same - // reason the audience refusal above is — and STRICTLY after it. - // - // Same shield, its other arm. `shouldReturnGenericDuplicateResponse` - // (better-auth 1.7.12 sign-up.mjs:155) is on whenever - // `requireEmailVerification` is on OR `autoSignIn === false`, and it - // guards TWO sites: the 403 catch at :227 (what the audience block - // above steps around) and the duplicate pre-check at :192–204. On - // the second, the vendor finds the - // existing row, logs it, and returns `buildGenericDuplicateResponse()` - // — a 200 carrying a freshly `generateId()`-ed user that is never - // written — INSTEAD of throwing USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL. - // - // We turn that shield on ourselves: a posture that permits - // self-registration forces `requireEmailVerification` on by default - // (see `createAuthInstance` — always under `email_domain`, and under - // `open` unless the deployment declared it off, #20389), so - // `email_domain` and `open` sign-ups for an address that already - // exists answered 200 while `invite_only` answered 422 on the same - // population. Measured on a real ObjectQL - // engine with the posture held CONSTANT and only the verification flag - // moved, so the divergence is the flag's, not the posture's: zero - // inserts reach the engine, no `sys_account` appears, and the next - // sign-in is 401 with nothing anywhere explaining it. The silent - // success shape, on the recovery path a locked-out deployment walks. - // - // ORDER IS LOAD-BEARING: this runs only for a caller the posture - // ALREADY ADMITTED. Asking it first would hand an uninvited stranger - // an account-existence oracle under the `invite_only` DEFAULT (422 for - // a real address vs 403 for an unknown one) — inventing on the closed - // posture exactly what the vendor's shield exists to prevent. After - // the gate, `invite_only` is untouched: a stranger still gets - // SELF_REGISTRATION_CLOSED and learns nothing. - // - // [#25060, ruling 1B on #25040] ONE exception, decided here and - // nowhere else ({@link existingAddressSignUpAnswersSuccess}): under - // `open` and `email_domain`, where the vendor's shield is on and a - // mail transport is live, the request FALLS THROUGH. The vendor then - // finds the row, answers its generic success — the same shape a new - // sign-up gets there, and no row, credential or session is written — - // and its `onExistingUserSignUp` hook mails the address's holder - // ({@link sendExistingAccountSignUpNotice}). Every other case keeps - // the explicit refusal: `invite_only`, a shield that is off (a new - // sign-up's answer there carries a session, which an existing - // address cannot mirror without one for someone else's account), and - // a transport that is not live (a success with no mail is the black - // hole this refusal replaced). The refusal is the platform's at this - // one seam, so it never depends on the vendor also refusing; where - // the vendor would have answered, its code and message are the - // vendor's OWN constant, so the two lanes are byte-identical by - // construction rather than by copying. - const signUpAddress = typeof ctx?.body?.email === 'string' ? ctx.body.email : ''; - if (signUpAddress && (await this.hasExistingUserFor(signUpAddress))) { - if (!this.existingAddressSignUpAnswersSuccess(ctx?.context?.options?.emailAndPassword)) { - const { APIError, BASE_ERROR_CODES } = await import('@better-auth/core/error'); - throw APIError.from( - 'UNPROCESSABLE_ENTITY', - BASE_ERROR_CODES.USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL, - ); - } - // The answer will carry a user id that no row holds: tell the - // after-hook, so it stamps nothing on that id (it would report a - // lost write for a user that was never created). - if (ctx?.context) ctx.context.__osExistingAddressSignUp = true; - } - // fall through — the vendor still decides everything it owns - } - - // ── [#25083] cloud-managed: invite-member issues nothing ──── - // `beforeCreateInvitation` refuses a NEW invitation, but - // better-auth answers `resend: true` for an invitee who is - // already pending BEFORE it consults that hook: it extends the - // row's `expiresAt` and sends the invitation e-mail again - // (`organization/routes/crud-invites.mjs`, better-auth 1.7.12). - // The console reaches exactly that branch through - // `sys_invitation`'s `resend_invitation` action. On a - // cloud-managed environment no invitation leaves this door in - // either shape, so the route refuses ahead of the vendor's - // handler; the hook keeps its own arm as the second line. The - // fact is public (`features.cloudManaged` on `/auth/config`), so - // answering it ahead of the session check tells an anonymous - // caller nothing new. - if (ctx?.path === '/organization/invite-member' && isCloudManaged()) { - const { APIError } = await import('better-auth/api'); - throw new APIError('CONFLICT', { - message: CLOUD_MANAGED_REFUSAL_MESSAGES.invitation, - code: CLOUD_MANAGED_REFUSAL_CODE, - }); - } - - // ── ADR-0069 D2: account lockout (gate) ───────────────────── - // Reject a sign-in for a locked identity BEFORE better-auth checks - // the password — a lock must hold even against the correct password. - // The refusal is a refused sign-in, so it writes its `login_failed` - // row (`auth-session-audit.ts`, account events) before it throws. - if (ctx?.path === SIGN_IN_EMAIL_PATH) { - const email = typeof ctx?.body?.email === 'string' ? ctx.body.email : ''; - if (email) await this.assertAccountNotLocked(email, ctx); - return; - } - - if (ctx?.path !== '/sign-up/email') return; - const ep = ctx?.context?.options?.emailAndPassword; - if (!ep?.disableSignUp) return; - // [#11767] Same population question, same owner. This site used to - // ask `ctx.context.adapter.findOne({ model: 'user', where: [] })`, - // which the real ObjectQL engine REFUSES (#4419 — - // `requireFindOnePredicate`) rather than answers; the surrounding - // `catch` read that refusal as "users exist" and the declared - // bypass never fired on a real deployment. `isBootstrapCreation` - // owns the answerable form — see its doc. - // [#14157] …and the deployment's own boot command provisioning its - // admin is the OTHER way through this door. It is not a - // self-registration and must not depend on a self-registration - // carve-out: an app that seeds people makes the bootstrap probe - // answer "populated" before the seed ever runs. Cheap synchronous - // check first, so the ticket path costs no I/O. - // [#14373] The email alone no longer admits — see - // `stageOperatorProvisioning`'s doc. `ctx.body` is the vendor's own - // zod-validated body, which keeps any key outside the declared - // schema (`signUpEmailBodySchema.and(z.record(...))`), so the - // in-process seed call's ticket field survives here untouched. - const signUpEmail = typeof ctx?.body?.email === 'string' ? ctx.body.email : undefined; - const provisioningTicket = ctx?.body?.[AuthManager.OPERATOR_PROVISIONING_TICKET_FIELD]; - if ( - this.isOperatorProvisioning(signUpEmail, provisioningTicket) || - (await this.isBootstrapCreation()) - ) { - ctx.context.__osDisableSignUpOrig = ep.disableSignUp; - ep.disableSignUp = false; - } + // [#25224 cut 13] The body moved to `auth-manager-before-hook.ts`; this + // arrow hands it the members it reads (`BeforeHookHost`) and the one + // local it read, `setShouldSkipSessionRefresh`. + before: createAuthMiddleware((ctx: any) => { + const readConfig = () => this.config; + const host: BeforeHookHost = { + assertAccountNotLocked: (...a) => this.assertAccountNotLocked(...a), + assertActorNotHeld: (...a) => this.assertActorNotHeld(...a), + assertAdminRevokeUserSessionIdentifiesRecord: (...a) => this.assertAdminRevokeUserSessionIdentifiesRecord(...a), + assertOrganizationDeletable: (...a) => this.assertOrganizationDeletable(...a), + assertPasswordComplexity: (...a) => this.assertPasswordComplexity(...a), + assertPasswordNotReused: (...a) => this.assertPasswordNotReused(...a), + assertPhoneOtpSendAllowed: (...a) => this.assertPhoneOtpSendAllowed(...a), + assertRemoveMemberPermitted: (...a) => this.assertRemoveMemberPermitted(...a), + assertRevokeSessionIdentifiesRecord: (...a) => this.assertRevokeSessionIdentifiesRecord(...a), + get config() { return readConfig(); }, + existingAddressSignUpAnswersSuccess: (...a) => this.existingAddressSignUpAnswersSuccess(...a), + hasExistingUserFor: (...a) => this.hasExistingUserFor(...a), + isAuthGateActive: (...a) => this.isAuthGateActive(...a), + isBootstrapCreation: (...a) => this.isBootstrapCreation(...a), + isOperatorProvisioning: (...a) => this.isOperatorProvisioning(...a), + isPlatformAdminUserId: (...a) => this.isPlatformAdminUserId(...a), + resolveActor: (...a) => this.resolveActor(...a), + resolvePasswordChangeUserId: (...a) => this.resolvePasswordChangeUserId(...a), + validateAudienceAdmission: (...a) => this.validateAudienceAdmission(...a), + }; + return globalBeforeHook.call(host, ctx, setShouldSkipSessionRefresh); }), // [#25224 cut 12] The body moved to `auth-manager-after-hook.ts`; this // arrow hands it the members it reads (`AfterHookHost`). diff --git a/scripts/check-dispatcher-error-vocabulary.mjs b/scripts/check-dispatcher-error-vocabulary.mjs index 6184ca8c1ce..c3e52398d73 100644 --- a/scripts/check-dispatcher-error-vocabulary.mjs +++ b/scripts/check-dispatcher-error-vocabulary.mjs @@ -720,7 +720,7 @@ export const SHAPES = [ // `APIError.from` copies `record.code` onto the body it constructs, so the // record IS the stamp. When that record is a MEMBER EXPRESSION on a binding // this file imports — `BASE_ERROR_CODES.USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL`, - // better-auth's own documented idiom, live in `auth-manager.ts` — the member + // better-auth's own documented idiom, live in `auth-manager-before-hook.ts` — the member // NAME is the code: a vendor code table built by `defineErrorCodes` maps each // KEY to `{ code: , message }`, and the repo's own restated records // (`admin-impersonate-endpoint.ts`'s `USER_NOT_FOUND`) carry the same shape. @@ -1615,7 +1615,7 @@ export const INLINE_LITERAL_EXPRESSION_CENSUS = Object.freeze({ * * ⭐ Read `newSites: 0` with `siteWhenDeregistered: 1` beside it, or it reads as * "nothing here". The one live member expression on this tree — - * `BASE_ERROR_CODES.USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL` in `auth-manager.ts` + * `BASE_ERROR_CODES.USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL` in `auth-manager-before-hook.ts` * — IS registered, by #15587, which is why the widening derives no site today. * What was missing was never the registration; it was anything that would have * REQUIRED it. `siteWhenDeregistered` is the controlled measurement of exactly @@ -1657,7 +1657,9 @@ export const APIERROR_ARGUMENT_POSITION_CENSUS = Object.freeze({ memberOutsideTheValueSpace: 0, /** The live one, named — a count with no subject cannot be re-derived. */ liveMember: 'BASE_ERROR_CODES.USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL', - liveMemberFile: 'packages/plugins/plugin-auth/src/auth-manager.ts', + // [#25224 cut 13] The member moved, unedited, with `createAuthInstance`'s + // `hooks.before` body; the counts above stay the reading at `measuredOn`. + liveMemberFile: 'packages/plugins/plugin-auth/src/auth-manager-before-hook.ts', /** Sites, verdict rows and unresolved entries this widening adds TODAY. */ newSites: 0, newVerdictRows: 0, @@ -2731,7 +2733,7 @@ export function bindingNames(pattern, depth = 0) { * shapes. `import type` is excluded: a type binding cannot be the object * of a value member expression. * ② `const = await import('')` (and the `require` and - * non-awaited spellings) — the DYNAMIC form, which is how `auth-manager.ts` + * non-awaited spellings) — the DYNAMIC form, which is how `auth-manager-before-hook.ts` * binds `BASE_ERROR_CODES` and therefore the spelling the card is about. * ③ `const [] = await Promise.all([ import(…), … ])` — the batched * dynamic form, live in `admin-impersonate-endpoint.ts`. Confirmed by