feat(plugin-auth): break-glass 守卫 —— ban 不得停用最后一个管理员(ADR-0024 D5.2) - #5939
Merged
Conversation
…inistrators (#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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 10 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This was referenced Aug 6, 2026
Contributor
Author
|
范围外发现,已另行开单(Prime Directive #10),均未在本 PR 修:
两条都与本 PR 的落点相邻但不同:一条换事件(delete vs update),一条换调用方( Generated by Claude Code |
…. they are」 守卫的产出就是那段解释,所以它得读得通。单数/复数分别成句,`'that account'` 替掉指代不明的 `'it'`。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JwwiU9bjhwy2SWj13ho8uv
baozhoutao
marked this pull request as ready for review
August 6, 2026 13:03
This was referenced Aug 6, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5892
前提逐条复核(动手前做的,结论改变了范围)
派发令要求先核对正文三项在
origin/main上的现状。核对结果:sys_user.beforeUpdate:更新会导致「剩余 0 个未被 ban 的管理员」时拒绝git grep对「最后一个管理员 / 计数后拒绝」这一形状零命中(last admin/lastAdmin/data.banned判定均无命中)enforcedSSO 不得停用最后一个本地管理员的密码auth-manager.ts的emailAndPassword段(resolveSsoOnly()只强制disableSignUp,从不动enabled)、getPublicConfig()的features.ssoEnforced、以及/admin/ban-user等三条路径上的LAST_LOCAL_CREDENTIAL守卫(auth-manager.ts:1239-1300)。但没有任何测试所以本 PR 的范围比正文小:第 ② 项一行实现代码都没改。
① 新守卫:
last-admin-ban-guard.tssys_user.banned = true是所有停用路径的落点 —— better-auth admin 插件的 ban 端点写它,@better-auth/scim把 SCIMactive: false映射成同一次 admin ban(这正是 SCIM 会强制打开 admin 插件的原因,ADR-0071)。此前没有任何一层检查这次写入之后还剩下谁:把最后一个管理员 ban 掉是允许的,而且返回成功,组织就此被永久锁在自己的环境外面。SCIM 让它从假设变成现实风险 —— 写入由外部系统驱动,提交前没有人看一眼 payload,一个圈错的 IdP 组就够了。守卫因此长在写入的那一个咽喉上,而不是某个端点上:
sys_user的beforeUpdate。admin ban 端点、SCIM 的 adapter 写入、导入、脚本、以及以后新增的任何路径都被同一条不变量覆盖;by-id 与谓词/multi两种写法都守(谓词那一支是「只读input.id」的守卫必然漏掉的形状)。谁算管理员 —— 用平台自己已有的答案,不另发明:
organization_id = null)且在有效期内(ADR-0091)的admin_full_access授权 —— 与resolveAuthzContext推导platform_admin所用的是同一份证据(ADR-0068 D2 / ADR-0095 D3),绝不读sys_user.role字符串;sys_member.role的等级 —— 判定走invitation-role-cap.ts里唯一那把等级尺(本 PR 把它导出为isOrgAdminGrade),而不是在守卫里重新拼一个role === 'owner':手抄版会漏掉'owner,member'这种逗号拼写,把环境里唯一的 owner 误判成普通成员。delegated_admin不算(ADR-0105 D8:它只让你够得着端点,本身不带权限)、过期授权不算、不可登录的usr_system不算 —— 后两者若被算成「还剩一个管理员」,守卫就会替一次真实的锁死放行。fail-closed:管理员群体读不出来、或大到无法枚举时,拒绝这次 ban 而不是猜。这与邻近的
LAST_LOCAL_CREDENTIAL守卫(fail-open)方向相反,是刻意的:那边失败的代价是挡住一次合法操作,这边失败的代价是环境被永久锁死。两个方向都写进了各自文件的头注释,并各有一条测试钉住,免得日后有人「统一一下」。拒绝信息即操作指引:错误里点名是哪个用户、这条不变量是什么(ADR-0024 D5.2)、怎样让这次 ban 变合法(先给别人
admin_full_access或 owner/admin 成员身份),以及 —— 如果这次 ban 来自 IdP —— 要改的是 IdP 组而不是这个守卫。与 ADR-0092 身份写守卫的关系:那个守卫回答「这个调用方能否经通用数据面写身份表」,并且按设计放行 system 上下文(better-auth 自己的 adapter 正是它必须放行的);本守卫回答的是另一个问题「这个值能不能被写」,因此对所有上下文生效,
isSystem也不例外 —— 真正会造成锁死的恰恰是 system 这条路。两者在auth-plugin.ts的同一个kernel:ready里一起注册,优先级 10 → 20:用户上下文的调用方仍然先拿到 ADR-0092 的答复(「banned不是数据面可编辑列」),只有合法携带banned的写入才会走到本守卫。这条顺序有测试钉住。顺带修的一处(同一条路径上的必要半边):守卫抛的是引擎形状的错误(
PERMISSION_DENIED+ 403),REST 数据面的mapDataError本来就认;但 auth 管线不认,会变成一个空白 500。withValidationErrorMapping因此增加了一条 403 分支,把它映射成 better-auth 的APIError('FORBIDDEN')。一个「产出就是解释」的守卫,不能以「无解释的 500」抵达 IdP。② 密码逃生口:只补钉
break-glass-local-credential.test.ts覆盖既有实现,不改一行:ssoOnlyMode与OS_AUTH_SSO_ONLY两个开关)下emailAndPassword.enabled保持true、disableSignUp被强制true,getPublicConfig()同时报features.ssoEnforced: true与emailPassword.enabled: true—— 这个「叫 UI 藏起来、能力本身仍开着」的落差就是逃生口本身;emailAndPassword.enabled: false仍然生效 —— 否则上面那条断言对着一个「根本不读这个选项」的实现也会绿;/admin/ban-user、/admin/remove-user、/delete-user三条路径上,目标是唯一credential账户持有人时抛LAST_LOCAL_CREDENTIAL;有第二个密码持有人时放行;目标是 IdP 托管(无本地凭据)时连全表扫描都不做;查询报错时 fail-open。测试
last-admin-ban-guard.test.ts不用假引擎:守卫的全部工作就是读身份表再判断,假引擎意味着由 fixture 里手写的where匹配器来决定守卫看见哪些行 —— 也就是让 fixture 而不是产品来回答被测的问题(#5785 点名的那种危险)。所以每条用例都跑在真的ObjectQL引擎 + 真的 better-sqlite3:memory:上:引擎派发钩子,SQL builder 编译$in/$ne,布尔按 0/1 落库 —— 顺带把 better-authsupportsBooleans: false交给引擎的数字1这条真实形状也覆盖了。反向验证(方向先定后跑):文件里有一条
unguarded: true的用例 —— 不注册守卫时,ban 最后一个管理员成功且行确实变成banned = 1。这就是 #5892 之前的行为,也是其余每条rejects.toThrow的对照系。20 条守卫用例 + 7 条密码逃生口用例,覆盖:两个管理员先后 ban(第一个放行、最后一个拒绝)、非管理员不受影响、平台管理员 vs 组织内限定授权、过期授权、
delegated_admin、逗号拼写、usr_system、重复 ban 已封禁者、unban 与无关写入不触发、谓词与$in批量、fail-closed 的两种(读失败 / 超过枚举上限)、无管理员环境不被卡死、ADR-0092 顺序,以及经 better-auth adapter 的 SCIM/admin-ban 路径(403APIError而非 500)。闸门:
check:nul-bytes、check:engine-double-contract(64 pinned,无新条目)、check:query-options-erasure(non-test 84 未新增,test 面 267 未移动)、check:error-code-casing、check:role-word、check:adr-anchors、check:authz-resolver全绿;改动文件eslint --no-inline-config无告警。边界
@better-auth/scim的模型面 —— 本守卫是本仓自己的sys_user钩子,与 SCIM: 停在 @better-auth/scim rc.1,等正式版再整体迁移 —— rc.2 换掉了整套模型 #3653 的版本挂起无关;packages/spec/packages/objectql:拒绝时用的是标准目录里已有的PERMISSION_DENIED,而不是新造一个服务码(ADR-0112「通用条件走标准目录」,rest-server.ts对RECORD_NOT_ACCESSIBLE的处理就是这个先例),因此不需要往 error-code ledger 里加条目;Generated by Claude Code