Skip to content

Commit 61dc08e

Browse files
baozhoutaoclaude
andauthored
feat(plugin-auth): break-glass 守卫 —— ban 不得停用最后一个管理员(ADR-0024 D5.2) (#5939)
* feat(plugin-auth): break-glass guard — a ban may never leave zero administrators (#5892) `sys_user.banned = true` is where every deprovision lands (better-auth's admin ban; `@better-auth/scim` maps `active: false` onto it), and nothing checked what the write left behind: banning the last administrator succeeded and locked the organization out of its own environment for good. SCIM makes that an accident waiting to happen — the write is driven by an external system, so nobody reads the payload before it commits. `last-admin-ban-guard.ts` registers a `beforeUpdate` hook on `sys_user` that refuses any write turning `banned` on when it would leave the environment with no unbanned administrator. It guards the WRITE, not an endpoint, so the admin ban route, the SCIM adapter write, an import and a script are all covered, by-id and predicate/multi alike. Administrator = the platform's own answer: an unscoped in-window `admin_full_access` grant, or an `owner`/`admin` membership graded by the single ladder in `invitation-role-cap.ts` (now exported as `isOrgAdminGrade`). `delegated_admin`, expired grants and `usr_system` do not count. Fail-closed: an unverifiable population refuses the ban. The refusal carries `PERMISSION_DENIED` + 403; `withValidationErrorMapping` gains a 403 arm so the auth pipeline reports it as an `APIError` instead of an opaque 500. The password half of the same invariant (enforced SSO never disables the last local admin's password) was already implemented and is pinned, not rewritten. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwwiU9bjhwy2SWj13ho8uv * docs(plugin-auth): 拒绝信息的复数措辞 —— 批量 ban 时不再说「the last administrator ... they are」 守卫的产出就是那段解释,所以它得读得通。单数/复数分别成句,`'that account'` 替掉指代不明的 `'it'`。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwwiU9bjhwy2SWj13ho8uv --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1216dcc commit 61dc08e

9 files changed

Lines changed: 1236 additions & 6 deletions
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
"@objectstack/plugin-auth": minor
3+
---
4+
5+
feat(plugin-auth): break-glass — a ban may never leave the environment with zero administrators (#5892)
6+
7+
`sys_user.banned = true` is where every deprovision lands: better-auth's admin
8+
plugin writes it, and `@better-auth/scim` maps a SCIM `active: false` onto that
9+
same admin ban. Nothing checked what the write left behind — so **banning the
10+
last administrator was allowed, reported success, and locked the organization
11+
out of its own environment permanently.** SCIM makes that a realistic accident
12+
rather than a hypothetical one: the write is driven by an external system, so
13+
nobody reads the payload before it commits, and one mis-scoped IdP group is
14+
enough.
15+
16+
**New guard (`last-admin-ban-guard.ts`, cloud ADR-0024 D5.2).** A `beforeUpdate`
17+
hook on `sys_user` refuses any write that turns `banned` on when it would leave
18+
the environment with **no unbanned administrator**. It sits on the write, not on
19+
an endpoint, so it holds for the admin ban endpoint, the SCIM adapter write, an
20+
import, a script, and anything added later — by-id **and** predicate/`multi`
21+
writes alike.
22+
23+
Who counts as an administrator is exactly what the rest of the platform already
24+
counts: a platform admin (an unscoped, in-window `admin_full_access` grant —
25+
the same evidence `resolveAuthzContext` derives `platform_admin` from) or an
26+
organization `owner`/`admin` membership. `delegated_admin` does not count
27+
(ADR-0105 D8: it can reach an endpoint, it carries no authority), an expired
28+
grant does not count, and the non-loginable `usr_system` account does not count.
29+
30+
Three consequences worth knowing before you upgrade:
31+
32+
- The refusal is a **403** carrying `PERMISSION_DENIED` and a message that names
33+
the user, the invariant, and the fix (grant someone else `admin_full_access`
34+
or an owner/admin membership first — and if an IdP drove the ban, the SCIM
35+
deprovision is too broad). On the auth pipeline it now surfaces as a proper
36+
`APIError` instead of an opaque 500.
37+
- It **fails closed**: if the administrator population cannot be read, or is too
38+
large to enumerate, the ban is refused rather than guessed at. The failure
39+
mode being prevented is a permanent lockout.
40+
- Writes that do not turn `banned` on — unbans, profile edits, re-banning an
41+
already-banned admin — are untouched, and so is banning anyone who is not an
42+
administrator.
43+
44+
The other half of the same invariant (`enforced` SSO must never disable the last
45+
local admin's **password** — the escape hatch for an IdP outage) was already
46+
implemented and is now pinned by tests rather than reimplemented:
47+
`emailAndPassword.enabled` stays `true` under enforced SSO while sign-up is
48+
forced off, and the last local `credential` account still cannot be banned,
49+
removed or deleted.

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
registerManagedUpdateWhitelist,
4242
type SecondaryStorageLike,
4343
} from './identity-write-guard.js';
44+
import { registerLastAdminBanGuard } from './last-admin-ban-guard.js';
4445
import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js';
4546
import { MANAGED_EXTENSION_EDITABLE_FIELDS } from './managed-extension-fields.js';
4647
import { runSetInitialPassword } from './set-initial-password.js';
@@ -986,6 +987,20 @@ export class AuthPlugin implements Plugin {
986987
getSecondaryStorage: () =>
987988
this.effectiveSecondaryStorage as SecondaryStorageLike | undefined,
988989
});
990+
// [cloud ADR-0024 D5.2] Break-glass — the SAME `sys_user` write
991+
// chokepoint, guarding a different question: not "may this caller
992+
// write identity tables" (above, and system writes bypass it by
993+
// design) but "may this VALUE be written at all". A `banned = true`
994+
// that would leave the environment with no administrator able to sign
995+
// in is refused for EVERY context, `isSystem` included — because the
996+
// path that actually locks an org out is the system one (better-auth's
997+
// admin ban, driven by a SCIM `active: false`). Registered at
998+
// priority 20 so the ADR-0092 strip above (10) still answers first for
999+
// user-context callers. See last-admin-ban-guard.ts.
1000+
registerLastAdminBanGuard(engine, {
1001+
packageId: 'com.objectstack.plugin-auth.last-admin-ban-guard',
1002+
logger: ctx.logger,
1003+
});
9891004
} catch {
9901005
// Engine not available (mock mode) — permission-set defaults remain
9911006
// the only gate, exactly the pre-guard status quo.
Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5892 / cloud ADR-0024 D5.2] The PASSWORD half of the break-glass
5+
* invariant — pinned, not implemented.
6+
*
7+
* #5892 asked for two things. The ban half was missing and is built in
8+
* `last-admin-ban-guard.ts`; this half — "`enforced` SSO must never disable the
9+
* last local admin's password" — was **already implemented** on `origin/main`
10+
* and had no test of its own, which is the state that lets a security
11+
* behaviour be refactored away without anything going red. So this file adds
12+
* the pins, over the shipped code, unchanged:
13+
*
14+
* 1. **The escape hatch stays wired under enforced SSO.** `resolveSsoOnly()`
15+
* forces `disableSignUp` on and tells the console to hide the password
16+
* form (`features.ssoEnforced`), but it must NEVER touch
17+
* `emailAndPassword.enabled` — the endpoint has to remain callable, or the
18+
* "use a password" link the login UI keeps for the env owner leads
19+
* nowhere the day the IdP is down (`auth-manager.ts`, the
20+
* `emailAndPassword` block and `getPublicConfig`).
21+
* 2. **The last local password cannot be removed.** The global before-hook
22+
* refuses `/admin/ban-user`, `/admin/remove-user` and `/delete-user` when
23+
* the target is the only user holding a `credential` account
24+
* (`LAST_LOCAL_CREDENTIAL`). Under enforced SSO the managed team has no
25+
* local credential at all, so that one account IS the escape hatch.
26+
*
27+
* The middleware is driven directly with a synthetic `ctx` — the same shape
28+
* better-auth passes it (`path`, `body`, `context.adapter`) — because the
29+
* decision under test is entirely a function of those three, and standing up
30+
* a real better-auth server would test better-auth's router instead.
31+
*
32+
* Fail-OPEN is deliberate here and is NOT a drift from the ban guard's
33+
* fail-closed posture: this check's failure mode is a blocked legitimate
34+
* removal, whereas the ban guard's is a permanently locked-out environment.
35+
* The last case below pins that direction so a future "make it consistent"
36+
* refactor has to argue with a test.
37+
*/
38+
39+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
40+
import { isAPIError } from 'better-auth/api';
41+
import { AuthManager } from './auth-manager';
42+
43+
// Mock better-auth so building the instance neither needs a database nor
44+
// starts a server; the config object is what these assertions read.
45+
vi.mock('better-auth', () => ({
46+
betterAuth: vi.fn(() => ({ handler: vi.fn(), api: {} })),
47+
}));
48+
49+
import { betterAuth } from 'better-auth';
50+
51+
const SECRET = 'test-secret-at-least-32-chars-long';
52+
const BASE_URL = 'http://localhost:3000';
53+
54+
type CapturedConfig = {
55+
emailAndPassword?: { enabled?: boolean; disableSignUp?: boolean };
56+
hooks?: { before?: (ctx: unknown) => Promise<unknown> };
57+
};
58+
59+
async function buildConfig(
60+
options: Record<string, unknown> = {},
61+
): Promise<{ config: CapturedConfig; manager: AuthManager }> {
62+
let captured: CapturedConfig = {};
63+
(betterAuth as unknown as { mockImplementation: (f: (c: CapturedConfig) => unknown) => void })
64+
.mockImplementation((config: CapturedConfig) => {
65+
captured = config;
66+
return { handler: vi.fn(), api: {} };
67+
});
68+
const manager = new AuthManager({ secret: SECRET, baseUrl: BASE_URL, ...options } as never);
69+
await manager.getAuthInstance();
70+
return { config: captured, manager };
71+
}
72+
73+
/**
74+
* The `account` rows a deployment holds. `findOne` answers the target's own
75+
* credential lookup; `findMany` answers "who else holds one".
76+
*/
77+
function adapterWithCredentials(holders: string[]) {
78+
const rows = holders.map((userId) => ({ userId, providerId: 'credential' }));
79+
return {
80+
findOne: vi.fn(async ({ where }: { where: Array<{ field: string; value: unknown }> }) => {
81+
const userId = where.find((w) => w.field === 'userId')?.value;
82+
return rows.find((r) => r.userId === userId) ?? null;
83+
}),
84+
findMany: vi.fn(async () => rows),
85+
};
86+
}
87+
88+
let consoleSpy: ReturnType<typeof vi.spyOn>;
89+
let warnSpy: ReturnType<typeof vi.spyOn>;
90+
const prevMcp = process.env.OS_MCP_SERVER_ENABLED;
91+
const prevSsoOnly = process.env.OS_AUTH_SSO_ONLY;
92+
93+
beforeEach(() => {
94+
vi.clearAllMocks();
95+
consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
96+
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
97+
// Keep the plugin list to the default surface — the MCP pair has its own
98+
// coverage and only lengthens these boots.
99+
process.env.OS_MCP_SERVER_ENABLED = 'false';
100+
delete process.env.OS_AUTH_SSO_ONLY;
101+
});
102+
103+
afterEach(() => {
104+
consoleSpy.mockRestore();
105+
warnSpy.mockRestore();
106+
if (prevMcp === undefined) delete process.env.OS_MCP_SERVER_ENABLED;
107+
else process.env.OS_MCP_SERVER_ENABLED = prevMcp;
108+
if (prevSsoOnly === undefined) delete process.env.OS_AUTH_SSO_ONLY;
109+
else process.env.OS_AUTH_SSO_ONLY = prevSsoOnly;
110+
});
111+
112+
// ---------------------------------------------------------------------------
113+
// 1. Enforced SSO keeps the password endpoint alive
114+
// ---------------------------------------------------------------------------
115+
116+
describe('[#5892] enforced SSO hides the password form — it never disables it', () => {
117+
it('config knob: `emailAndPassword.enabled` stays true while sign-up is forced off', async () => {
118+
const { config, manager } = await buildConfig({ ssoOnlyMode: true });
119+
120+
expect(config.emailAndPassword?.enabled).toBe(true);
121+
expect(config.emailAndPassword?.disableSignUp).toBe(true);
122+
123+
const publicConfig = manager.getPublicConfig() as {
124+
emailPassword: { enabled: boolean; disableSignUp: boolean };
125+
features: { ssoEnforced: boolean };
126+
};
127+
// The console is told to HIDE the form (ssoEnforced) while the capability
128+
// it hides is still advertised as enabled — that gap is the break-glass
129+
// link, not an inconsistency.
130+
expect(publicConfig.features.ssoEnforced).toBe(true);
131+
expect(publicConfig.emailPassword.enabled).toBe(true);
132+
expect(publicConfig.emailPassword.disableSignUp).toBe(true);
133+
});
134+
135+
it('env knob: `OS_AUTH_SSO_ONLY` reaches the same place', async () => {
136+
process.env.OS_AUTH_SSO_ONLY = 'true';
137+
const { config, manager } = await buildConfig();
138+
139+
expect(config.emailAndPassword?.enabled).toBe(true);
140+
expect(config.emailAndPassword?.disableSignUp).toBe(true);
141+
expect(
142+
(manager.getPublicConfig() as { features: { ssoEnforced: boolean } }).features.ssoEnforced,
143+
).toBe(true);
144+
});
145+
146+
it('a deployment that really wants passwords off can still say so explicitly', async () => {
147+
// The invariant is "enforced SSO does not disable it", not "it can never be
148+
// disabled" — otherwise the assertion above would pass against code that
149+
// ignores the option entirely.
150+
const { config } = await buildConfig({ emailAndPassword: { enabled: false } });
151+
expect(config.emailAndPassword?.enabled).toBe(false);
152+
});
153+
});
154+
155+
// ---------------------------------------------------------------------------
156+
// 2. The last local credential cannot be banned / removed / deleted
157+
// ---------------------------------------------------------------------------
158+
159+
describe('[#5892] the last local password login survives ban / remove / delete', () => {
160+
const BAN_PATHS = ['/admin/ban-user', '/admin/remove-user', '/delete-user'];
161+
162+
it('refuses the removal when the target holds the ONLY credential account', async () => {
163+
const { config } = await buildConfig({ ssoOnlyMode: true });
164+
const before = config.hooks?.before;
165+
expect(typeof before).toBe('function');
166+
167+
for (const path of BAN_PATHS) {
168+
const adapter = adapterWithCredentials(['usr_owner']);
169+
let caught: unknown;
170+
try {
171+
await before!({ path, body: { userId: 'usr_owner' }, context: { adapter } });
172+
} catch (e) {
173+
caught = e;
174+
}
175+
expect(isAPIError(caught)).toBe(true);
176+
const api = caught as { statusCode: number; body: { code?: string; message?: string } };
177+
expect(api.body.code).toBe('LAST_LOCAL_CREDENTIAL');
178+
expect(api.body.message).toMatch(/identity-\s*provider outage|provider outage/);
179+
}
180+
});
181+
182+
it('allows it when another user still holds a local password', async () => {
183+
const { config } = await buildConfig({ ssoOnlyMode: true });
184+
const adapter = adapterWithCredentials(['usr_owner', 'usr_second_admin']);
185+
186+
await expect(
187+
config.hooks!.before!({
188+
path: '/admin/ban-user',
189+
body: { userId: 'usr_owner' },
190+
context: { adapter },
191+
}),
192+
).resolves.toBeUndefined();
193+
});
194+
195+
it('never fires for a credential-less (IdP-managed) target', async () => {
196+
// The managed population signs in through the IdP and holds no local
197+
// password, so removing one of them cannot cost anyone the escape hatch.
198+
const { config } = await buildConfig({ ssoOnlyMode: true });
199+
const adapter = adapterWithCredentials(['usr_owner']);
200+
201+
await expect(
202+
config.hooks!.before!({
203+
path: '/admin/ban-user',
204+
body: { userId: 'usr_managed' },
205+
context: { adapter },
206+
}),
207+
).resolves.toBeUndefined();
208+
// Only the target's own lookup ran — the whole-table scan is skipped.
209+
expect(adapter.findMany).not.toHaveBeenCalled();
210+
});
211+
212+
it('fails OPEN on a lookup error — the opposite direction from the ban guard, on purpose', async () => {
213+
const { config } = await buildConfig({ ssoOnlyMode: true });
214+
const adapter = {
215+
findOne: vi.fn(async () => {
216+
throw new Error('account table unreadable');
217+
}),
218+
findMany: vi.fn(async () => []),
219+
};
220+
221+
// A blocked legitimate removal is the cost here; a locked-out environment
222+
// is the cost in `last-admin-ban-guard.ts`. Different failure modes,
223+
// different directions — see that file's header.
224+
await expect(
225+
config.hooks!.before!({
226+
path: '/admin/ban-user',
227+
body: { userId: 'usr_owner' },
228+
context: { adapter },
229+
}),
230+
).resolves.toBeUndefined();
231+
});
232+
});

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ export * from './admin-user-endpoints.js';
2020
export * from './placeholder-email.js';
2121
export * from './admin-import-users.js';
2222
export * from './identity-write-guard.js';
23+
// [cloud ADR-0024 D5.2 / #5892] The break-glass ban guard. Exported for the
24+
// same reason its ADR-0092 neighbour above is: a host that stands up its own
25+
// ObjectQL engine (the cloud control plane, an embedding that skips this
26+
// plugin's `kernel:ready` wiring) has to be able to register the invariant
27+
// itself rather than ship an environment that can ban its last administrator.
28+
export * from './last-admin-ban-guard.js';
2329
export * from './sys-user-writable-fields.js';
2430
export * from './otp-send-guard.js';
2531
// ADR-0069 D2 / #4772 — the cross-node rate-limit counter store (kernel cache,

packages/plugins/plugin-auth/src/invitation-role-cap.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,27 @@ export function orgRoleGrade(raw: unknown): number {
8989
return grade;
9090
}
9191

92+
/**
93+
* Does this `sys_member.role` value carry an ADMINISTRATIVE grade — i.e. is
94+
* its holder one of the people who administer the organization?
95+
*
96+
* The grade ladder above is the one place that answers it, so every consumer
97+
* asks here rather than re-spelling `role === 'owner' || role === 'admin'`:
98+
* a hand-written copy drops the comma-joined (`'owner,member'`) and array
99+
* spellings `parseOrgRoles` handles, and on a security path that difference is
100+
* silent. Second consumer, and the reason this is exported: the break-glass
101+
* ban guard (`last-admin-ban-guard.ts`, ADR-0024 D5.2), which counts the
102+
* administrators an environment would have left after a ban — a guard that
103+
* mistook the only owner for an ordinary member would wave the lockout
104+
* through.
105+
*
106+
* `delegated_admin` is NOT an administrative grade (ADR-0105 D8: it can reach
107+
* an endpoint, it carries no authority), and neither is an unresolvable value.
108+
*/
109+
export function isOrgAdminGrade(raw: unknown): boolean {
110+
return orgRoleGrade(raw) >= GRADE_ADMIN;
111+
}
112+
92113
/**
93114
* Is this invitation exactly a plain `member`? Such an invitation can never
94115
* trip the cap, so the hook skips resolving the issuer's membership row — the

0 commit comments

Comments
 (0)