diff --git a/.changeset/last-admin-ban-break-glass.md b/.changeset/last-admin-ban-break-glass.md new file mode 100644 index 0000000000..ebe285634e --- /dev/null +++ b/.changeset/last-admin-ban-break-glass.md @@ -0,0 +1,49 @@ +--- +"@objectstack/plugin-auth": minor +--- + +feat(plugin-auth): break-glass — a ban may never leave the environment with zero administrators (#5892) + +`sys_user.banned = true` is where every deprovision lands: better-auth's admin +plugin writes it, and `@better-auth/scim` maps a SCIM `active: false` onto that +same admin ban. Nothing checked what the write left behind — so **banning the +last administrator was allowed, reported success, and locked the organization +out of its own environment permanently.** SCIM makes that a realistic accident +rather than a hypothetical one: the write is driven by an external system, so +nobody reads the payload before it commits, and one mis-scoped IdP group is +enough. + +**New guard (`last-admin-ban-guard.ts`, cloud ADR-0024 D5.2).** A `beforeUpdate` +hook on `sys_user` refuses any write that turns `banned` on when it would leave +the environment with **no unbanned administrator**. It sits on the write, not on +an endpoint, so it holds for the admin ban endpoint, the SCIM adapter write, an +import, a script, and anything added later — by-id **and** predicate/`multi` +writes alike. + +Who counts as an administrator is exactly what the rest of the platform already +counts: a platform admin (an unscoped, in-window `admin_full_access` grant — +the same evidence `resolveAuthzContext` derives `platform_admin` from) or an +organization `owner`/`admin` membership. `delegated_admin` does not count +(ADR-0105 D8: it can reach an endpoint, it carries no authority), an expired +grant does not count, and the non-loginable `usr_system` account does not count. + +Three consequences worth knowing before you upgrade: + +- The refusal is a **403** carrying `PERMISSION_DENIED` and a message that names + the user, the invariant, and the fix (grant someone else `admin_full_access` + or an owner/admin membership first — and if an IdP drove the ban, the SCIM + deprovision is too broad). On the auth pipeline it now surfaces as a proper + `APIError` instead of an opaque 500. +- It **fails closed**: if the administrator population cannot be read, or is too + large to enumerate, the ban is refused rather than guessed at. The failure + mode being prevented is a permanent lockout. +- Writes that do not turn `banned` on — unbans, profile edits, re-banning an + already-banned admin — are untouched, and so is banning anyone who is not an + administrator. + +The other half of the same invariant (`enforced` SSO must never disable the last +local admin's **password** — the escape hatch for an IdP outage) was already +implemented and is now pinned by tests rather than reimplemented: +`emailAndPassword.enabled` stays `true` under enforced SSO while sign-up is +forced off, and the last local `credential` account still cannot be banned, +removed or deleted. diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 842f73a106..6723bda63d 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -41,6 +41,7 @@ import { registerManagedUpdateWhitelist, type SecondaryStorageLike, } from './identity-write-guard.js'; +import { registerLastAdminBanGuard } from './last-admin-ban-guard.js'; import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js'; import { MANAGED_EXTENSION_EDITABLE_FIELDS } from './managed-extension-fields.js'; import { runSetInitialPassword } from './set-initial-password.js'; @@ -986,6 +987,20 @@ export class AuthPlugin implements Plugin { getSecondaryStorage: () => this.effectiveSecondaryStorage as SecondaryStorageLike | undefined, }); + // [cloud ADR-0024 D5.2] Break-glass — the SAME `sys_user` write + // chokepoint, guarding a different question: not "may this caller + // write identity tables" (above, and system writes bypass it by + // design) but "may this VALUE be written at all". A `banned = true` + // that would leave the environment with no administrator able to sign + // in is refused for EVERY context, `isSystem` included — because the + // path that actually locks an org out is the system one (better-auth's + // admin ban, driven by a SCIM `active: false`). Registered at + // priority 20 so the ADR-0092 strip above (10) still answers first for + // user-context callers. See last-admin-ban-guard.ts. + registerLastAdminBanGuard(engine, { + packageId: 'com.objectstack.plugin-auth.last-admin-ban-guard', + logger: ctx.logger, + }); } catch { // Engine not available (mock mode) — permission-set defaults remain // the only gate, exactly the pre-guard status quo. diff --git a/packages/plugins/plugin-auth/src/break-glass-local-credential.test.ts b/packages/plugins/plugin-auth/src/break-glass-local-credential.test.ts new file mode 100644 index 0000000000..10b39c2eeb --- /dev/null +++ b/packages/plugins/plugin-auth/src/break-glass-local-credential.test.ts @@ -0,0 +1,232 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5892 / cloud ADR-0024 D5.2] The PASSWORD half of the break-glass + * invariant — pinned, not implemented. + * + * #5892 asked for two things. The ban half was missing and is built in + * `last-admin-ban-guard.ts`; this half — "`enforced` SSO must never disable the + * last local admin's password" — was **already implemented** on `origin/main` + * and had no test of its own, which is the state that lets a security + * behaviour be refactored away without anything going red. So this file adds + * the pins, over the shipped code, unchanged: + * + * 1. **The escape hatch stays wired under enforced SSO.** `resolveSsoOnly()` + * forces `disableSignUp` on and tells the console to hide the password + * form (`features.ssoEnforced`), but it must NEVER touch + * `emailAndPassword.enabled` — the endpoint has to remain callable, or the + * "use a password" link the login UI keeps for the env owner leads + * nowhere the day the IdP is down (`auth-manager.ts`, the + * `emailAndPassword` block and `getPublicConfig`). + * 2. **The last local password cannot be removed.** The global before-hook + * refuses `/admin/ban-user`, `/admin/remove-user` and `/delete-user` when + * the target is the only user holding a `credential` account + * (`LAST_LOCAL_CREDENTIAL`). Under enforced SSO the managed team has no + * local credential at all, so that one account IS the escape hatch. + * + * The middleware is driven directly with a synthetic `ctx` — the same shape + * better-auth passes it (`path`, `body`, `context.adapter`) — because the + * decision under test is entirely a function of those three, and standing up + * a real better-auth server would test better-auth's router instead. + * + * Fail-OPEN is deliberate here and is NOT a drift from the ban guard's + * fail-closed posture: this check's failure mode is a blocked legitimate + * removal, whereas the ban guard's is a permanently locked-out environment. + * The last case below pins that direction so a future "make it consistent" + * refactor has to argue with a test. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { isAPIError } from 'better-auth/api'; +import { AuthManager } from './auth-manager'; + +// Mock better-auth so building the instance neither needs a database nor +// starts a server; the config object is what these assertions read. +vi.mock('better-auth', () => ({ + betterAuth: vi.fn(() => ({ handler: vi.fn(), api: {} })), +})); + +import { betterAuth } from 'better-auth'; + +const SECRET = 'test-secret-at-least-32-chars-long'; +const BASE_URL = 'http://localhost:3000'; + +type CapturedConfig = { + emailAndPassword?: { enabled?: boolean; disableSignUp?: boolean }; + hooks?: { before?: (ctx: unknown) => Promise }; +}; + +async function buildConfig( + options: Record = {}, +): Promise<{ config: CapturedConfig; manager: AuthManager }> { + let captured: CapturedConfig = {}; + (betterAuth as unknown as { mockImplementation: (f: (c: CapturedConfig) => unknown) => void }) + .mockImplementation((config: CapturedConfig) => { + captured = config; + return { handler: vi.fn(), api: {} }; + }); + const manager = new AuthManager({ secret: SECRET, baseUrl: BASE_URL, ...options } as never); + await manager.getAuthInstance(); + return { config: captured, manager }; +} + +/** + * The `account` rows a deployment holds. `findOne` answers the target's own + * credential lookup; `findMany` answers "who else holds one". + */ +function adapterWithCredentials(holders: string[]) { + const rows = holders.map((userId) => ({ userId, providerId: 'credential' })); + return { + findOne: vi.fn(async ({ where }: { where: Array<{ field: string; value: unknown }> }) => { + const userId = where.find((w) => w.field === 'userId')?.value; + return rows.find((r) => r.userId === userId) ?? null; + }), + findMany: vi.fn(async () => rows), + }; +} + +let consoleSpy: ReturnType; +let warnSpy: ReturnType; +const prevMcp = process.env.OS_MCP_SERVER_ENABLED; +const prevSsoOnly = process.env.OS_AUTH_SSO_ONLY; + +beforeEach(() => { + vi.clearAllMocks(); + consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // Keep the plugin list to the default surface — the MCP pair has its own + // coverage and only lengthens these boots. + process.env.OS_MCP_SERVER_ENABLED = 'false'; + delete process.env.OS_AUTH_SSO_ONLY; +}); + +afterEach(() => { + consoleSpy.mockRestore(); + warnSpy.mockRestore(); + if (prevMcp === undefined) delete process.env.OS_MCP_SERVER_ENABLED; + else process.env.OS_MCP_SERVER_ENABLED = prevMcp; + if (prevSsoOnly === undefined) delete process.env.OS_AUTH_SSO_ONLY; + else process.env.OS_AUTH_SSO_ONLY = prevSsoOnly; +}); + +// --------------------------------------------------------------------------- +// 1. Enforced SSO keeps the password endpoint alive +// --------------------------------------------------------------------------- + +describe('[#5892] enforced SSO hides the password form — it never disables it', () => { + it('config knob: `emailAndPassword.enabled` stays true while sign-up is forced off', async () => { + const { config, manager } = await buildConfig({ ssoOnlyMode: true }); + + expect(config.emailAndPassword?.enabled).toBe(true); + expect(config.emailAndPassword?.disableSignUp).toBe(true); + + const publicConfig = manager.getPublicConfig() as { + emailPassword: { enabled: boolean; disableSignUp: boolean }; + features: { ssoEnforced: boolean }; + }; + // The console is told to HIDE the form (ssoEnforced) while the capability + // it hides is still advertised as enabled — that gap is the break-glass + // link, not an inconsistency. + expect(publicConfig.features.ssoEnforced).toBe(true); + expect(publicConfig.emailPassword.enabled).toBe(true); + expect(publicConfig.emailPassword.disableSignUp).toBe(true); + }); + + it('env knob: `OS_AUTH_SSO_ONLY` reaches the same place', async () => { + process.env.OS_AUTH_SSO_ONLY = 'true'; + const { config, manager } = await buildConfig(); + + expect(config.emailAndPassword?.enabled).toBe(true); + expect(config.emailAndPassword?.disableSignUp).toBe(true); + expect( + (manager.getPublicConfig() as { features: { ssoEnforced: boolean } }).features.ssoEnforced, + ).toBe(true); + }); + + it('a deployment that really wants passwords off can still say so explicitly', async () => { + // The invariant is "enforced SSO does not disable it", not "it can never be + // disabled" — otherwise the assertion above would pass against code that + // ignores the option entirely. + const { config } = await buildConfig({ emailAndPassword: { enabled: false } }); + expect(config.emailAndPassword?.enabled).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// 2. The last local credential cannot be banned / removed / deleted +// --------------------------------------------------------------------------- + +describe('[#5892] the last local password login survives ban / remove / delete', () => { + const BAN_PATHS = ['/admin/ban-user', '/admin/remove-user', '/delete-user']; + + it('refuses the removal when the target holds the ONLY credential account', async () => { + const { config } = await buildConfig({ ssoOnlyMode: true }); + const before = config.hooks?.before; + expect(typeof before).toBe('function'); + + for (const path of BAN_PATHS) { + const adapter = adapterWithCredentials(['usr_owner']); + let caught: unknown; + try { + await before!({ path, body: { userId: 'usr_owner' }, context: { adapter } }); + } catch (e) { + caught = e; + } + expect(isAPIError(caught)).toBe(true); + const api = caught as { statusCode: number; body: { code?: string; message?: string } }; + expect(api.body.code).toBe('LAST_LOCAL_CREDENTIAL'); + expect(api.body.message).toMatch(/identity-\s*provider outage|provider outage/); + } + }); + + it('allows it when another user still holds a local password', async () => { + const { config } = await buildConfig({ ssoOnlyMode: true }); + const adapter = adapterWithCredentials(['usr_owner', 'usr_second_admin']); + + await expect( + config.hooks!.before!({ + path: '/admin/ban-user', + body: { userId: 'usr_owner' }, + context: { adapter }, + }), + ).resolves.toBeUndefined(); + }); + + it('never fires for a credential-less (IdP-managed) target', async () => { + // The managed population signs in through the IdP and holds no local + // password, so removing one of them cannot cost anyone the escape hatch. + const { config } = await buildConfig({ ssoOnlyMode: true }); + const adapter = adapterWithCredentials(['usr_owner']); + + await expect( + config.hooks!.before!({ + path: '/admin/ban-user', + body: { userId: 'usr_managed' }, + context: { adapter }, + }), + ).resolves.toBeUndefined(); + // Only the target's own lookup ran — the whole-table scan is skipped. + expect(adapter.findMany).not.toHaveBeenCalled(); + }); + + it('fails OPEN on a lookup error — the opposite direction from the ban guard, on purpose', async () => { + const { config } = await buildConfig({ ssoOnlyMode: true }); + const adapter = { + findOne: vi.fn(async () => { + throw new Error('account table unreadable'); + }), + findMany: vi.fn(async () => []), + }; + + // A blocked legitimate removal is the cost here; a locked-out environment + // is the cost in `last-admin-ban-guard.ts`. Different failure modes, + // different directions — see that file's header. + await expect( + config.hooks!.before!({ + path: '/admin/ban-user', + body: { userId: 'usr_owner' }, + context: { adapter }, + }), + ).resolves.toBeUndefined(); + }); +}); diff --git a/packages/plugins/plugin-auth/src/index.ts b/packages/plugins/plugin-auth/src/index.ts index 965023ba6c..ec8612e1e4 100644 --- a/packages/plugins/plugin-auth/src/index.ts +++ b/packages/plugins/plugin-auth/src/index.ts @@ -20,6 +20,12 @@ export * from './admin-user-endpoints.js'; export * from './placeholder-email.js'; export * from './admin-import-users.js'; export * from './identity-write-guard.js'; +// [cloud ADR-0024 D5.2 / #5892] The break-glass ban guard. Exported for the +// same reason its ADR-0092 neighbour above is: a host that stands up its own +// ObjectQL engine (the cloud control plane, an embedding that skips this +// plugin's `kernel:ready` wiring) has to be able to register the invariant +// itself rather than ship an environment that can ban its last administrator. +export * from './last-admin-ban-guard.js'; export * from './sys-user-writable-fields.js'; export * from './otp-send-guard.js'; // ADR-0069 D2 / #4772 — the cross-node rate-limit counter store (kernel cache, diff --git a/packages/plugins/plugin-auth/src/invitation-role-cap.ts b/packages/plugins/plugin-auth/src/invitation-role-cap.ts index 46233ae367..7a07c3c087 100644 --- a/packages/plugins/plugin-auth/src/invitation-role-cap.ts +++ b/packages/plugins/plugin-auth/src/invitation-role-cap.ts @@ -89,6 +89,27 @@ export function orgRoleGrade(raw: unknown): number { return grade; } +/** + * Does this `sys_member.role` value carry an ADMINISTRATIVE grade — i.e. is + * its holder one of the people who administer the organization? + * + * The grade ladder above is the one place that answers it, so every consumer + * asks here rather than re-spelling `role === 'owner' || role === 'admin'`: + * a hand-written copy drops the comma-joined (`'owner,member'`) and array + * spellings `parseOrgRoles` handles, and on a security path that difference is + * silent. Second consumer, and the reason this is exported: the break-glass + * ban guard (`last-admin-ban-guard.ts`, ADR-0024 D5.2), which counts the + * administrators an environment would have left after a ban — a guard that + * mistook the only owner for an ordinary member would wave the lockout + * through. + * + * `delegated_admin` is NOT an administrative grade (ADR-0105 D8: it can reach + * an endpoint, it carries no authority), and neither is an unresolvable value. + */ +export function isOrgAdminGrade(raw: unknown): boolean { + return orgRoleGrade(raw) >= GRADE_ADMIN; +} + /** * Is this invitation exactly a plain `member`? Such an invitation can never * trip the cap, so the hook skips resolving the issuer's membership row — the diff --git a/packages/plugins/plugin-auth/src/last-admin-ban-guard.test.ts b/packages/plugins/plugin-auth/src/last-admin-ban-guard.test.ts new file mode 100644 index 0000000000..b32dd62cc7 --- /dev/null +++ b/packages/plugins/plugin-auth/src/last-admin-ban-guard.test.ts @@ -0,0 +1,511 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5892 / cloud ADR-0024 D5.2] The break-glass ban guard. + * + * ## Why there is no fake engine here + * + * The guard's whole job is to read the identity tables and decide whether an + * administrator survives the write. A fake engine would mean a hand-written + * `where` matcher deciding which rows the guard sees — i.e. the fixture, not + * the product, answering the question under test (the hazard #5785 names in as + * many words). So every case below runs on a REAL {@link ObjectQL} engine over + * a real better-sqlite3 `:memory:` database: the engine dispatches the hook, + * the SQL builder compiles `$in` / `$ne`, and sqlite stores the booleans as + * 0/1 — which is also how the guard's numeric-flag handling gets exercised for + * free, since better-auth's adapter is configured `supportsBooleans: false` and + * hands ObjectQL a `1` for `banned: true`. + * + * The object fixtures declare only the columns this guard reads (plus enough + * identity to be a table). `sys_user.banned` keeps its production + * `readonly: true` so the system-context exemption is the real one. + * + * ## The two faces + * + * 1. **The engine write** — `engine.update('sys_user', …)`, by-id and + * predicate/multi, which is every path that reaches the column. + * 2. **The SCIM / admin-ban path** — the same refusal driven through + * `createObjectQLAdapterFactory`, the adapter `@better-auth/scim`'s + * `active: false` → admin ban actually writes through, asserting it + * surfaces as a 403 `APIError` and not an opaque 500. + * + * Reverse verification (recorded because the direction is not obvious): with + * `registerLastAdminBanGuard` NOT called, the "last administrator" cases below + * are GREEN-as-in-the-ban-succeeds — `engine.update` resolves and the row comes + * back `banned = 1`. That is the pre-#5892 behaviour, and it is what every + * `rejects.toThrow` here is measured against. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { isAPIError } from 'better-auth/api'; +import { ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; +import { SystemUserId } from '@objectstack/spec/system'; +import { registerLastAdminBanGuard, type LastAdminBanGuardEngine } from './last-admin-ban-guard.js'; +import { registerIdentityWriteGuard, registerManagedUpdateWhitelist } from './identity-write-guard.js'; +import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js'; +import { createObjectQLAdapterFactory } from './objectql-adapter.js'; +import { buildAdminPluginSchema } from './auth-schema-config.js'; +import { admin } from 'better-auth/plugins/admin'; + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +const sysUser = { + name: 'sys_user', + label: 'User', + // The ADR-0092 guard keys off this; the ban guard deliberately does not. + managedBy: 'better-auth', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const }, + email: { name: 'email', type: 'text' as const }, + // Production spelling: writable only by an `isSystem` caller (#2948). + banned: { name: 'banned', type: 'boolean' as const, readonly: true }, + }, +}; + +const sysMember = { + name: 'sys_member', + label: 'Member', + managedBy: 'better-auth', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + user_id: { name: 'user_id', type: 'text' as const }, + organization_id: { name: 'organization_id', type: 'text' as const }, + role: { name: 'role', type: 'text' as const }, + }, +}; + +const sysPermissionSet = { + name: 'sys_permission_set', + label: 'Permission Set', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const }, + }, +}; + +const sysUserPermissionSet = { + name: 'sys_user_permission_set', + label: 'User Permission Set', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + user_id: { name: 'user_id', type: 'text' as const }, + permission_set_id: { name: 'permission_set_id', type: 'text' as const }, + organization_id: { name: 'organization_id', type: 'text' as const }, + valid_from: { name: 'valid_from', type: 'datetime' as const }, + valid_until: { name: 'valid_until', type: 'datetime' as const }, + }, +}; + +const SYSTEM = { context: { isSystem: true } } as const; +const ORG = 'org_1'; +const PS_ADMIN = 'ps_admin_full_access'; + +/** Every ban a real deprovision performs is a system-context write. */ +async function ban(engine: ObjectQL, id: string): Promise { + return engine.update('sys_user', { id, banned: true }, SYSTEM); +} + +async function bannedFlag(engine: ObjectQL, id: string): Promise { + const row = await engine.findOne('sys_user', { where: { id }, fields: ['id', 'banned'] }, SYSTEM); + return row?.banned; +} + +interface BootOptions { + /** Overrides the engine the GUARD reads through (hook stays on the real one). */ + readThrough?: (engine: ObjectQL) => LastAdminBanGuardEngine; + maxScan?: number; + /** Register the ADR-0092 identity write guard alongside, at its own priority. */ + withIdentityWriteGuard?: boolean; + /** Skip registration entirely — the pre-#5892 engine, for reverse verification. */ + unguarded?: boolean; +} + +let engines: ObjectQL[] = []; + +afterEach(async () => { + // `:memory:` dies with its connection; closing keeps one live database per + // test from piling up in a file this size. + const open = engines; + engines = []; + for (const e of open) { + try { await e.destroy(); } catch { /* noop */ } + } +}); + +async function boot(opts: BootOptions = {}): Promise { + const engine = new ObjectQL(); + engines.push(engine); + engine.registerDriver( + new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), + true, + ); + await engine.init(); + for (const o of [sysUser, sysMember, sysPermissionSet, sysUserPermissionSet]) { + engine.registry.registerObject(o as never); + } + await engine.syncSchemas(); + + if (opts.withIdentityWriteGuard) { + registerManagedUpdateWhitelist('sys_user', SYS_USER_PROFILE_EDIT_FIELDS); + registerIdentityWriteGuard(engine, { packageId: 'test.identity-write-guard' }); + } + if (!opts.unguarded) { + registerLastAdminBanGuard(opts.readThrough?.(engine) ?? (engine as unknown as LastAdminBanGuardEngine), { + packageId: 'test.last-admin-ban-guard', + ...(opts.maxScan !== undefined ? { maxScan: opts.maxScan } : {}), + }); + } + return engine; +} + +/** A user row, plus whatever standing the case needs. */ +async function seedUser( + engine: ObjectQL, + id: string, + extra: { role?: string; platformAdmin?: boolean; banned?: boolean; grant?: Record } = {}, +): Promise { + await engine.insert( + 'sys_user', + { id, name: id, email: `${id}@example.com`, banned: extra.banned ?? false }, + SYSTEM, + ); + if (extra.role) { + await engine.insert( + 'sys_member', + { id: `mem_${id}`, user_id: id, organization_id: ORG, role: extra.role }, + SYSTEM, + ); + } + if (extra.platformAdmin || extra.grant) { + await engine.insert( + 'sys_user_permission_set', + { id: `ups_${id}`, user_id: id, permission_set_id: PS_ADMIN, ...(extra.grant ?? {}) }, + SYSTEM, + ); + } +} + +async function seedAdminPermissionSet(engine: ObjectQL): Promise { + await engine.insert('sys_permission_set', { id: PS_ADMIN, name: ADMIN_FULL_ACCESS }, SYSTEM); + await engine.insert('sys_permission_set', { id: 'ps_member', name: 'member_default' }, SYSTEM); +} + +// --------------------------------------------------------------------------- +// Face 1 — the invariant, on the engine write +// --------------------------------------------------------------------------- + +describe('[#5892] break-glass: the last unbanned administrator cannot be banned', () => { + let engine: ObjectQL; + + beforeEach(async () => { + engine = await boot(); + await seedAdminPermissionSet(engine); + }); + + it('two org admins: banning the first is allowed, banning the last is refused', async () => { + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, 'usr_admin', { role: 'admin' }); + await seedUser(engine, 'usr_member', { role: 'member' }); + + // One of two — the environment keeps an administrator, so this proceeds. + await expect(ban(engine, 'usr_admin')).resolves.toBeTruthy(); + expect(await bannedFlag(engine, 'usr_admin')).toBeTruthy(); + + // The last one — refused, and nothing is written. + await expect(ban(engine, 'usr_owner')).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + object: 'sys_user', + }); + expect(await bannedFlag(engine, 'usr_owner')).toBeFalsy(); + }); + + it('the refusal explains itself: which user, why, and what to do about it', async () => { + await seedUser(engine, 'usr_owner', { role: 'owner' }); + + await expect(ban(engine, 'usr_owner')).rejects.toThrow(/usr_owner/); + await expect(ban(engine, 'usr_owner')).rejects.toThrow(/last administrator/i); + await expect(ban(engine, 'usr_owner')).rejects.toThrow(/ADR-0024 D5\.2/); + // Information as operating instructions: how to make the ban legal, and + // where to look when an IdP drove it. + await expect(ban(engine, 'usr_owner')).rejects.toThrow(new RegExp(ADMIN_FULL_ACCESS)); + await expect(ban(engine, 'usr_owner')).rejects.toThrow(/SCIM deprovision is too broad/); + }); + + it('banning a non-administrator is untouched, even when exactly one admin exists', async () => { + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, 'usr_member', { role: 'member' }); + await seedUser(engine, 'usr_nobody'); + + await expect(ban(engine, 'usr_member')).resolves.toBeTruthy(); + await expect(ban(engine, 'usr_nobody')).resolves.toBeTruthy(); + expect(await bannedFlag(engine, 'usr_member')).toBeTruthy(); + }); + + it('a platform admin (unscoped admin_full_access) counts, a SCOPED grant does not', async () => { + // `usr_platform` holds the org-less grant → platform admin. + await seedUser(engine, 'usr_platform', { platformAdmin: true }); + // `usr_scoped` holds the SAME permission set scoped to an org → a tenant + // admin, not the environment's break-glass account (ADR-0068 D2 reads the + // unscoped grant only). + await seedUser(engine, 'usr_scoped', { grant: { organization_id: ORG } }); + + await expect(ban(engine, 'usr_scoped')).resolves.toBeTruthy(); + await expect(ban(engine, 'usr_platform')).rejects.toThrow(/last administrator/i); + }); + + it('an EXPIRED admin grant is not an administrator — neither as survivor nor as target', async () => { + const past = new Date(Date.now() - 86_400_000).toISOString(); + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, 'usr_expired', { grant: { valid_until: past } }); + + // The expired holder is bannable: they were never an administrator. + await expect(ban(engine, 'usr_expired')).resolves.toBeTruthy(); + // …and cannot be counted as the survivor that lets the real one go. + await expect(ban(engine, 'usr_owner')).rejects.toThrow(/last administrator/i); + }); + + it('`delegated_admin` does not count as an administrator (ADR-0105 D8: reach, not authority)', async () => { + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, 'usr_delegate', { role: 'delegated_admin' }); + + await expect(ban(engine, 'usr_owner')).rejects.toThrow(/last administrator/i); + }); + + it('a comma-joined membership role is still an administrator', async () => { + await seedUser(engine, 'usr_multi', { role: 'owner,member' }); + + await expect(ban(engine, 'usr_multi')).rejects.toThrow(/last administrator/i); + }); + + it('the non-loginable `usr_system` account is never counted as the survivor', async () => { + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, SystemUserId.SYSTEM, { platformAdmin: true }); + + await expect(ban(engine, 'usr_owner')).rejects.toThrow(/last administrator/i); + }); + + it('an already-banned administrator can be re-banned (nothing is being taken away)', async () => { + await seedUser(engine, 'usr_owner', { role: 'owner', banned: true }); + + await expect(ban(engine, 'usr_owner')).resolves.toBeTruthy(); + }); + + it('unbanning, and any write that does not turn `banned` on, is never guarded', async () => { + await seedUser(engine, 'usr_owner', { role: 'owner', banned: true }); + + await expect( + engine.update('sys_user', { id: 'usr_owner', banned: false }, SYSTEM), + ).resolves.toBeTruthy(); + await expect( + engine.update('sys_user', { id: 'usr_owner', name: 'Renamed' }, SYSTEM), + ).resolves.toBeTruthy(); + expect(await bannedFlag(engine, 'usr_owner')).toBeFalsy(); + }); +}); + +// --------------------------------------------------------------------------- +// Predicate / bulk writes — the shape a by-id guard would miss +// --------------------------------------------------------------------------- + +describe('[#5892] the guard holds on predicate (multi) bans, not only by-id', () => { + let engine: ObjectQL; + + beforeEach(async () => { + engine = await boot(); + await seedAdminPermissionSet(engine); + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, 'usr_admin', { role: 'admin' }); + await seedUser(engine, 'usr_member', { role: 'member' }); + }); + + it('a predicate that would sweep every administrator is refused', async () => { + await expect( + engine.update('sys_user', { banned: true }, { multi: true, where: { banned: false }, ...SYSTEM }), + ).rejects.toThrow(/last administrator/i); + expect(await bannedFlag(engine, 'usr_owner')).toBeFalsy(); + expect(await bannedFlag(engine, 'usr_admin')).toBeFalsy(); + }); + + it('an `$in` predicate naming both admins is refused — a scalar-id read would have missed it', async () => { + await expect( + engine.update( + 'sys_user', + { banned: true }, + { multi: true, where: { id: { $in: ['usr_owner', 'usr_admin'] } }, ...SYSTEM }, + ), + ).rejects.toThrow(/last administrator/i); + }); + + it('a predicate that spares one administrator proceeds', async () => { + await expect( + engine.update( + 'sys_user', + { banned: true }, + { multi: true, where: { id: { $in: ['usr_admin', 'usr_member'] } }, ...SYSTEM }, + ), + ).resolves.toBeDefined(); + expect(await bannedFlag(engine, 'usr_admin')).toBeTruthy(); + expect(await bannedFlag(engine, 'usr_owner')).toBeFalsy(); + }); +}); + +// --------------------------------------------------------------------------- +// Fail-closed +// --------------------------------------------------------------------------- + +describe('[#5892] the guard fails CLOSED — an unverifiable population refuses the ban', () => { + it('a failing identity read refuses the ban and names the reason', async () => { + const engine = await boot({ + readThrough: (real) => ({ + registerHook: (event, handler, options) => real.registerHook(event, handler, options), + find: async () => { + throw new Error('sys_member is unreadable'); + }, + }), + }); + await seedAdminPermissionSet(engine); + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, 'usr_admin', { role: 'admin' }); + + // Two admins exist — this ban WOULD be legal. It is refused anyway, + // because the guard could not prove it. + await expect(ban(engine, 'usr_admin')).rejects.toThrow(/could not be verified/i); + await expect(ban(engine, 'usr_admin')).rejects.toThrow(/sys_member is unreadable/); + await expect(ban(engine, 'usr_admin')).rejects.toMatchObject({ code: 'PERMISSION_DENIED' }); + expect(await bannedFlag(engine, 'usr_admin')).toBeFalsy(); + }); + + it('a population larger than the guard can enumerate refuses the ban', async () => { + const engine = await boot({ maxScan: 1 }); + await seedAdminPermissionSet(engine); + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, 'usr_admin', { role: 'admin' }); + + await expect(ban(engine, 'usr_admin')).rejects.toThrow(/more than 1 rows/); + }); + + it('an environment with no administrator at all is not blocked (nothing to protect)', async () => { + // Pre-bootstrap shape: users exist, nobody administers anything yet. + const engine = await boot(); + await seedUser(engine, 'usr_a'); + await seedUser(engine, 'usr_b'); + + await expect(ban(engine, 'usr_a')).resolves.toBeTruthy(); + }); +}); + +// --------------------------------------------------------------------------- +// Ordering against the ADR-0092 identity write guard +// --------------------------------------------------------------------------- + +describe('[#5892] a USER-CONTEXT ban still gets the ADR-0092 answer, not this one', () => { + it('the identity write guard (priority 10) answers first for a data-API caller', async () => { + const engine = await boot({ withIdentityWriteGuard: true }); + await seedAdminPermissionSet(engine); + await seedUser(engine, 'usr_owner', { role: 'owner' }); + + // `banned` is not on the sys_user profile whitelist, so the ADR-0092 guard + // rejects the payload before the break-glass guard is reached — the caller + // is told the column is not editable through the data API, which is the + // accurate answer for that surface. + await expect( + engine.update( + 'sys_user', + { id: 'usr_owner', banned: true }, + { context: { isSystem: false, userId: 'usr_caller', positions: [], permissions: [] } }, + ), + ).rejects.toThrow(/Editable fields/); + }); +}); + +// --------------------------------------------------------------------------- +// Face 2 — the SCIM / admin-ban path, through better-auth's adapter +// --------------------------------------------------------------------------- + +describe('[#5892] the SCIM / admin-ban path: refused as a 403, not an opaque 500', () => { + let engine: ObjectQL; + let adapter: { + update: (args: { model: string; where: unknown[]; update: Record }) => Promise; + }; + + beforeEach(async () => { + engine = await boot(); + await seedAdminPermissionSet(engine); + await seedUser(engine, 'usr_owner', { role: 'owner' }); + await seedUser(engine, 'usr_admin', { role: 'admin' }); + // The production factory, exactly as `AuthManager` builds it: writes run + // through `withSystemContext`, which is why the ADR-0092 guard is not the + // thing standing between an IdP and a locked-out environment. + // + // The admin plugin has to be in the options, not because this test drives + // its endpoints, but because `banned` is ITS column: better-auth's adapter + // transforms a payload against the declared table, so a `banned` no plugin + // declared is dropped before the engine ever sees it — the write would + // "succeed" and change nothing. That is also the production wiring: SCIM + // forces the admin plugin on precisely because `active: false` lands as its + // ban (ADR-0071), and `buildAdminPluginSchema()` is the same mapping + // `AuthManager` passes. + adapter = (createObjectQLAdapterFactory(engine) as unknown as (o: unknown) => typeof adapter)({ + plugins: [admin({ schema: buildAdminPluginSchema() })], + }); + }); + + /** What `@better-auth/scim`'s `active: false` → admin ban ultimately writes. */ + const deprovision = (userId: string) => + adapter.update({ + model: 'user', + where: [{ field: 'id', value: userId, operator: 'eq', connector: 'AND' }], + update: { banned: true }, + }); + + it('deprovisioning the second-to-last administrator succeeds', async () => { + await expect(deprovision('usr_admin')).resolves.toBeTruthy(); + expect(await bannedFlag(engine, 'usr_admin')).toBeTruthy(); + }); + + it('deprovisioning the LAST administrator is refused with a 403 APIError', async () => { + await deprovision('usr_admin'); + + let caught: unknown; + try { + await deprovision('usr_owner'); + } catch (e) { + caught = e; + } + + expect(caught).toBeDefined(); + // A raw engine error would reach the IdP as a 500 with no explanation — + // the one outcome a guard whose product is an explanation must not have. + expect(isAPIError(caught)).toBe(true); + const api = caught as { statusCode: number; body: { code?: string; message?: string } }; + expect(api.statusCode).toBe(403); + expect(api.body.code).toBe('PERMISSION_DENIED'); + expect(api.body.message).toMatch(/last administrator/i); + expect(await bannedFlag(engine, 'usr_owner')).toBeFalsy(); + }); +}); + +// --------------------------------------------------------------------------- +// Reverse verification — the same fixtures with the guard NOT registered +// --------------------------------------------------------------------------- + +describe('[#5892] reverse verification: without the guard, the lockout goes through', () => { + it('the pre-#5892 engine bans the last administrator and reports success', async () => { + const engine = await boot({ unguarded: true }); + await seedAdminPermissionSet(engine); + await seedUser(engine, 'usr_owner', { role: 'owner' }); + + await expect(ban(engine, 'usr_owner')).resolves.toBeTruthy(); + expect(await bannedFlag(engine, 'usr_owner')).toBeTruthy(); + }); +}); diff --git a/packages/plugins/plugin-auth/src/last-admin-ban-guard.ts b/packages/plugins/plugin-auth/src/last-admin-ban-guard.ts new file mode 100644 index 0000000000..0121692777 --- /dev/null +++ b/packages/plugins/plugin-auth/src/last-admin-ban-guard.ts @@ -0,0 +1,363 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [cloud ADR-0024 D5.2] Break-glass — a write may never leave this environment + * with ZERO administrators able to sign in. + * + * `sys_user.banned = true` is how EVERY deprovision lands: the better-auth + * admin plugin's ban endpoint writes it, and `@better-auth/scim` maps a SCIM + * `active: false` onto that same admin ban (which is why SCIM forces the admin + * plugin on — ADR-0071). SCIM writes are driven by an EXTERNAL system: nobody + * reads the payload before it commits, so one mis-scoped IdP group or one + * over-broad deprovision run is enough for an organization to ban its own last + * administrator and lock itself out of its environment permanently. There is no + * recovery path from inside the product once that happens. + * + * So the invariant is enforced at the WRITE, on the one chokepoint every path + * goes through — `beforeUpdate` on `sys_user` — rather than at any individual + * endpoint. HTTP-level guards protect only the endpoint they are attached to; + * this one holds for the admin ban endpoint, the SCIM adapter write, an import, + * a script, and anything added later. + * + * ## What counts as an administrator + * + * Exactly what `AuthManager.isOrgOrPlatformAdmin` (the repo's existing + * admin-gate answer) counts, enumerated in the opposite direction: + * + * 1. **platform admin** — an UNSCOPED (`organization_id = null`), in-window + * (ADR-0091) `sys_user_permission_set` grant of `admin_full_access`. This + * is the same evidence `resolveAuthzContext` derives `platform_admin` from + * (ADR-0068 D2 / ADR-0095 D3) — never a stored `sys_user.role` string. + * 2. **organization owner / admin** — a `sys_member` row whose role carries + * the `owner` or `admin` grade (ADR-0108's closed vocabulary). Grade, not + * capability: it is read here only as "who administers this org", which is + * the standing ADR-0057 D4 leaves on that column. + * + * `delegated_admin` deliberately does NOT count. ADR-0105 D8 defines it as a + * grade that can REACH an endpoint, carrying no authority of its own — counting + * it would let the guard believe an administrator remains when nobody can + * actually restore access. `usr_system` does not count either: the legacy + * service account is not loginable, so it can never be the escape hatch (the + * same exclusion the first-admin bootstrap makes). + * + * ## Fail-closed + * + * Every lookup this guard makes is part of a SAFETY proof: a ban is permitted + * only when at least one other unbanned administrator is provably left. A + * lookup that fails, or a population too large to enumerate, proves nothing — + * so the ban is REFUSED, loudly, with the reason. That is the opposite of the + * fail-OPEN posture the neighbouring last-local-credential guard takes in + * `auth-manager.ts` (an HTTP-level convenience check whose failure mode is a + * blocked legitimate op); here the failure mode is a permanent lockout, so the + * two directions are chosen deliberately and are not a drift. + * + * ## Scope: the ENVIRONMENT, not each organization + * + * The invariant protects the deployment's ability to be administered at all — + * "≥ 1 unbanned administrator remains anywhere in this environment". A stricter + * per-organization rule ("every org keeps an owner") is a different, larger + * policy with its own product decisions (what happens to an org whose only + * owner leaves the company); it is deliberately not invented here. + * + * ## Relationship to the ADR-0092 identity write guard + * + * `identity-write-guard.ts` answers "may this CALLER write identity tables + * through the generic data path" and bypasses system-context writes by design — + * better-auth's own adapter is exactly what it must let through. This guard + * answers a different question, "may this VALUE be written at all", and + * therefore applies to EVERY context, `isSystem` included: the ban path that + * actually causes lockouts is the system one. The two are registered together + * (`auth-plugin.ts`, `kernel:ready`) and ordered so the ADR-0092 strip runs + * first (priority 10 → 20): a user-context caller keeps getting the ADR-0092 + * message ("`banned` is not editable via the data API"), and only the writes + * that legitimately carry `banned` reach this guard. + */ + +import type { BaseEngineOptions, EngineQueryOptions } from '@objectstack/spec/data'; +import { + ADMIN_FULL_ACCESS, + MEMBERSHIP_ROLE_ADMIN, + MEMBERSHIP_ROLE_MEMBER, + MEMBERSHIP_ROLE_OWNER, +} from '@objectstack/spec/identity'; +import { SystemObjectName, SystemUserId } from '@objectstack/spec/system'; +import { isGrantActive } from '@objectstack/core'; + +import { isOrgAdminGrade } from './invitation-role-cap.js'; + +/** `sys_user_permission_set` has no `SystemObjectName` member; it is spelled once, here. */ +const USER_PERMISSION_SET = 'sys_user_permission_set'; + +type LoggerLike = { + info(msg: string): void; + warn(msg: string): void; + debug?(msg: string): void; +}; + +/** + * The engine surface this guard needs — hook registration plus system-context + * reads. Structural rather than `IObjectQLEngine` so the guard can be driven + * directly in tests without standing up an engine. + */ +export interface LastAdminBanGuardEngine { + registerHook( + event: string, + handler: (ctx: unknown) => Promise, + options?: { object?: string | string[]; priority?: number; packageId?: string }, + ): void; + find( + objectName: string, + query?: EngineQueryOptions, + options?: BaseEngineOptions, + ): Promise>>; +} + +export interface LastAdminBanGuardOptions { + packageId: string; + logger?: LoggerLike; + /** + * Largest row count any one enumeration read may return before the guard + * gives up and refuses (fail-closed). The administrator population of an + * environment is tiny; this exists so a pathological predicate ban — or a + * `sys_member` table with tens of thousands of non-plain-member rows — + * cannot be silently under-counted into a lockout. Default 1000. + */ + maxScan?: number; +} + +const DEFAULT_MAX_SCAN = 1000; + +/** Reads run as system: this is a safety proof, never RLS-scoped to a caller. */ +const SYSTEM_READ: BaseEngineOptions = { context: { isSystem: true } }; + +/** + * Boolean columns arrive spelled by whichever driver / transport wrote them: + * better-auth's adapter is configured `supportsBooleans: false` (so it hands + * ObjectQL 1/0), sqlite stores 1/0, the memory driver keeps real booleans, and + * a REST body can carry the string. Anything that is not one of these is NOT a + * ban — an `undefined` / absent `banned` must never be read as "true". + */ +function isTrueFlag(value: unknown): boolean { + return value === true || value === 1 || value === '1' || value === 'true'; +} + +/** The refusal. `PERMISSION_DENIED` + 403 is what `mapDataError` already maps. */ +function refuse(message: string): Error { + const err = new Error(`PERMISSION_DENIED: ${message}`) as Error & { + code?: string; + status?: number; + object?: string; + }; + err.code = 'PERMISSION_DENIED'; + err.status = 403; + err.object = SystemObjectName.USER; + return err; +} + +/** Marker so the fail-closed wrapper re-throws a deliberate refusal unchanged. */ +function isRefusal(err: unknown): boolean { + return (err as { code?: string } | null)?.code === 'PERMISSION_DENIED'; +} + +const BREAK_GLASS_CITATION = + 'break-glass invariant, ADR-0024 D5.2 — an environment must always keep at least one ' + + 'administrator who can sign in'; + +function toId(value: unknown): string | undefined { + if (typeof value === 'string' && value.length > 0) return value; + if (typeof value === 'number') return String(value); + return undefined; +} + +/** + * Register the last-administrator ban guard on an ObjectQL engine. + * + * Idempotent per package the same way the identity write guard is: a caller + * re-binding after a hot reload runs `engine.unregisterHooksByPackage(packageId)` + * first. + */ +export function registerLastAdminBanGuard( + engine: LastAdminBanGuardEngine, + opts: LastAdminBanGuardOptions, +): void { + const { packageId, logger } = opts; + const maxScan = opts.maxScan ?? DEFAULT_MAX_SCAN; + + /** Enumerate `object` under a hard ceiling; overflow proves nothing → refuse. */ + const scan = async ( + object: string, + query: EngineQueryOptions, + ): Promise>> => { + const rows = await engine.find(object, { ...query, limit: maxScan + 1 }, SYSTEM_READ); + const list = Array.isArray(rows) ? rows : []; + if (list.length > maxScan) { + throw refuse( + `Refusing this ban: '${object}' returned more than ${maxScan} rows, so the remaining ` + + `administrators could not be verified (${BREAK_GLASS_CITATION}). Ban a narrower set of ` + + 'users, or raise the guard\'s maxScan if this environment really is that large.', + ); + } + return list; + }; + + /** Every user this environment currently recognises as an administrator. */ + const resolveAdminUserIds = async (): Promise> => { + const ids = new Set(); + const now = Date.now(); + + // 1) Platform admins — unscoped, in-window `admin_full_access` grants. + const sets = await scan(SystemObjectName.PERMISSION_SET, { + where: { name: ADMIN_FULL_ACCESS }, + fields: ['id', 'name'], + }); + const adminSetIds = sets.map((r) => toId(r.id)).filter((v): v is string => Boolean(v)); + if (adminSetIds.length > 0) { + const links = await scan(USER_PERMISSION_SET, { + where: { permission_set_id: { $in: adminSetIds } }, + }); + for (const link of links) { + // An org-SCOPED grant makes a tenant admin, not the environment's + // break-glass admin — the same distinction `resolveAuthzContext` draws + // when it derives `platform_admin` from the unscoped grant only. + if (link.organization_id ?? link.organizationId) continue; + if (!isGrantActive(link, now)) continue; + const uid = toId(link.user_id ?? link.userId); + if (uid) ids.add(uid); + } + } + + // 2) Organization owners / admins, graded by the ONE ladder that answers + // "does this role administer the org" (`invitation-role-cap.ts`) — a + // re-spelled `role === 'owner'` here would drop the comma-joined and + // array spellings that ladder handles, and mistake an environment's + // only owner for an ordinary member. Narrowed to non-plain-member rows + // so a large membership table is not read wholesale; the grade test + // itself still runs in memory, over every row that narrowing kept. + const members = await scan(SystemObjectName.MEMBER, { + where: { role: { $ne: MEMBERSHIP_ROLE_MEMBER } }, + }); + for (const m of members) { + if (!isOrgAdminGrade(m.role)) continue; + const uid = toId(m.user_id ?? m.userId); + if (uid) ids.add(uid); + } + + // The legacy service account is not loginable — it can never be the escape + // hatch, so it must not be counted as one. + ids.delete(SystemUserId.SYSTEM); + return ids; + }; + + /** Of `adminIds`, those whose `sys_user` row is present and not banned. */ + const resolveUnbannedAdmins = async (adminIds: Set): Promise> => { + const rows = await scan(SystemObjectName.USER, { + where: { id: { $in: [...adminIds] } }, + fields: ['id', 'banned'], + }); + const out = new Set(); + for (const row of rows) { + const id = toId(row.id); + // A row already carrying `banned` cannot sign in, so it is not one of the + // administrators this write could be taking away. + if (id && adminIds.has(id) && !isTrueFlag(row.banned)) out.add(id); + } + return out; + }; + + /** Which `sys_user` rows this one update writes to. */ + const resolveTargetIds = async ( + id: unknown, + data: Record, + options: { where?: unknown } | undefined, + ): Promise> => { + const single = toId(id) ?? toId(data.id); + if (single) return new Set([single]); + // Predicate / multi update: `input.id` is unbound and the row-scoping + // predicate rides on `input.options.where` (#5273 pinned that shape). + const where = options?.where as EngineQueryOptions['where']; + const rows = await scan(SystemObjectName.USER, { + ...(where !== undefined ? { where } : {}), + fields: ['id'], + }); + const out = new Set(); + for (const row of rows) { + const rid = toId(row.id); + if (rid) out.add(rid); + } + return out; + }; + + const guardBan = async (rawCtx: unknown): Promise => { + const ctx = (rawCtx ?? {}) as { + object?: string; + input?: { id?: unknown; data?: Record; options?: { where?: unknown } }; + }; + if (ctx.object !== SystemObjectName.USER) return; + + const data = (ctx.input?.data ?? {}) as Record; + // Only a write that TURNS the ban on is interesting. An unban, an + // unrelated profile write, or a payload the ADR-0092 strip already emptied + // of `banned` can never reduce the administrator population. + if (!('banned' in data) || !isTrueFlag(data.banned)) return; + + try { + const admins = await resolveAdminUserIds(); + // Nothing recognised as an administrator: there is no break-glass account + // to protect and refusing every ban would be a guard inventing a policy + // out of an empty measurement. (A deployment reaches this only before the + // first admin is bootstrapped.) + if (admins.size === 0) return; + + const unbanned = await resolveUnbannedAdmins(admins); + const targets = await resolveTargetIds(ctx.input?.id, data, ctx.input?.options); + + const losing = [...unbanned].filter((id) => targets.has(id)); + // No administrator that could still sign in is affected → not our case. + // This is also what makes re-banning an already-banned admin a no-op + // rather than a refusal: nothing is being taken away. + if (losing.length === 0) return; + + const remaining = [...unbanned].filter((id) => !targets.has(id)); + if (remaining.length > 0) return; + + logger?.warn( + `[LastAdminBanGuard] refused a ban that would have left this environment with no ` + + `unbanned administrator (target: ${losing.join(', ')})`, + ); + const many = losing.length > 1; + throw refuse( + `Refusing to ban ${losing.map((id) => `'${id}'`).join(', ')}: ` + + `${many ? 'those are the last administrators' : 'that is the last administrator'} this ` + + `environment has that ${many ? 'are' : 'is'} not already banned, and banning ` + + `${many ? 'them' : 'that account'} would leave nobody able to administer the ` + + `environment or restore anyone's access (${BREAK_GLASS_CITATION}). Grant another user ` + + `the '${ADMIN_FULL_ACCESS}' permission set or an organization ` + + `'${MEMBERSHIP_ROLE_OWNER}'/'${MEMBERSHIP_ROLE_ADMIN}' membership first, then retry. ` + + 'If the ban came from an identity provider, the SCIM deprovision is too broad — fix the ' + + 'IdP group, not this guard.', + ); + } catch (err) { + if (isRefusal(err)) throw err; + // Fail CLOSED: the guard could not prove another administrator survives, + // and the cost of guessing wrong is a permanently locked-out environment. + const reason = (err as Error)?.message ?? String(err); + logger?.warn(`[LastAdminBanGuard] administrator lookup failed — ban refused: ${reason}`); + throw refuse( + 'Refusing this ban: the remaining administrators could not be verified ' + + `(${reason}). This guard fails closed — a ban is only permitted when at least one other ` + + `unbanned administrator is provably left (${BREAK_GLASS_CITATION}). Retry once the ` + + 'identity tables are readable again.', + ); + } + }; + + // Priority 20: AFTER the ADR-0092 identity write guard's strip (10), before + // default-priority hooks (100) spend work on a write this may refuse. + engine.registerHook('beforeUpdate', guardBan, { + object: SystemObjectName.USER, + priority: 20, + packageId, + }); + + logger?.info('[LastAdminBanGuard] last-administrator ban guard registered (ADR-0024 D5.2)'); +} diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.test.ts b/packages/plugins/plugin-auth/src/objectql-adapter.test.ts index 70f2500af1..8f0f36d1c7 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.test.ts +++ b/packages/plugins/plugin-auth/src/objectql-adapter.test.ts @@ -445,7 +445,11 @@ describe('withValidationErrorMapping – ObjectQL ValidationError → better-aut ]); }); - it('re-throws non-validation errors verbatim (not remapped to an APIError)', async () => { + // "Unclassified" — an error carrying neither the validation envelope nor the + // `PERMISSION_DENIED` policy-refusal code the break-glass guards throw + // (ADR-0024 D5.2; the 403 arm is pinned in `last-admin-ban-guard.test.ts`). + // A driver fault is exactly that, and it must NOT be dressed up as a 4xx. + it('re-throws unclassified errors verbatim (not remapped to an APIError)', async () => { const boom = new Error('driver exploded'); const adapter = withValidationErrorMapping({ update: async () => { diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.ts b/packages/plugins/plugin-auth/src/objectql-adapter.ts index 48cb0e348c..d3546dd549 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.ts +++ b/packages/plugins/plugin-auth/src/objectql-adapter.ts @@ -305,10 +305,28 @@ function isObjectQLValidationError( return e.code === 'VALIDATION_FAILED' || e.name === 'ValidationError'; } +/** + * An engine-level POLICY refusal — a `beforeUpdate`/`beforeInsert` guard that + * refused the write itself, rather than a payload the validator disliked. + * + * Detected by the same duck-typing as above (`code`), and for the same reason: + * the guards live in this package but throw a plain engine-shaped error so that + * BOTH transports can map it — `mapDataError` gives the REST data routes a 403, + * this gives the auth pipeline one. The concrete case is the break-glass + * last-administrator ban guard (ADR-0024 D5.2, `last-admin-ban-guard.ts`): + * without this arm, an over-broad SCIM deprovision would be refused correctly + * and then reported to the IdP as an opaque 500, which is the one thing a guard + * whose whole product is an explanation must not do. + */ +function isEnginePolicyRefusal(err: unknown): err is { code?: string; message?: string } { + if (!err || typeof err !== 'object') return false; + return (err as { code?: unknown }).code === 'PERMISSION_DENIED'; +} + /** * Re-throw `err` as a better-auth `APIError` when it is an ObjectQL validation - * failure; otherwise re-throw it verbatim. Always throws — the return type is - * `never`. + * failure or an engine policy refusal; otherwise re-throw it verbatim. Always + * throws — the return type is `never`. */ async function rethrowAsBetterAuthError(err: unknown): Promise { if (isObjectQLValidationError(err)) { @@ -323,14 +341,25 @@ async function rethrowAsBetterAuthError(err: unknown): Promise { ...(Array.isArray(fields) ? { fields } : {}), }); } + if (isEnginePolicyRefusal(err)) { + const { APIError } = await import('better-auth/api'); + throw new APIError('FORBIDDEN', { + message: + typeof err.message === 'string' && err.message.trim() + ? err.message + : 'Permission denied', + code: 'PERMISSION_DENIED', + }); + } throw err; } /** * Wrap every function-valued method of a better-auth adapter so an ObjectQL - * `ValidationError` thrown from the underlying engine surfaces as a 4xx - * `APIError` instead of an opaque 500. Non-function properties pass through - * untouched, and every non-validation error is re-thrown verbatim. + * `ValidationError` (400) or an engine policy refusal (403) thrown from the + * underlying engine surfaces as a 4xx `APIError` instead of an opaque 500. + * Non-function properties pass through untouched, and every error that carries + * neither signature is re-thrown verbatim. */ export function withValidationErrorMapping>(adapter: A): A { const out: Record = {};