From 81bf176e2c6baecc9bc67bd6f9564d02696018c9 Mon Sep 17 00:00:00 2001 From: "claude[bot]" Date: Thu, 3 Sep 2026 02:28:04 +0000 Subject: [PATCH 1/3] test(plugin-auth): pin the SCIM DELETE of the last administrator, and de-flake the timed-ban face MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a face driving `DELETE /Users/{id}` against the last platform administrator. On `@better-auth/scim` 1.7.2 that verb is not a row delete: the vendor tombstones the SCIM source, the aggregate turns inactive, and the deprovision arrives as the same ban write as `PATCH active: false` — so the break-glass guard refuses it through `guardBan` on `beforeUpdate`, and `guardDelete` never runs. The mechanism covered this from the day the reconcile callback landed; nothing drove it. Widens the timed-ban face's expiry from 1.5 s to 5 s and waits `expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is still in force, and on a loaded shard wall-clock time is not the test's progress. Nothing the face proves depends on the number. Adds the positive control the widened window makes necessary: same setup, no SCIM deactivation, same wait, sign-in accepted. Without it a green face is equally well explained by "the expiry never elapsed". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- .../scim-deactivation-reconcile-user.test.ts | 99 ++++++++++++++++++- 1 file changed, 94 insertions(+), 5 deletions(-) diff --git a/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts b/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts index 8486a7f954..c27ebf78ae 100644 --- a/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts +++ b/packages/plugins/plugin-auth/src/scim-deactivation-reconcile-user.test.ts @@ -14,7 +14,7 @@ * identity provider deactivating a user revoked sessions and wrote nothing: * `sys_user.banned` stayed false, and a user holding a local password signed * straight back in. `auth-manager.ts` now passes the callback and routes it - * to the platform's own ban write (`admin-ban-endpoints.ts`). + * to the platform's own ban write (`user-ban-write.ts`). * * ## Why every case drives the vendor and none simulates the write * @@ -43,10 +43,16 @@ * (c) the last administrator: refused THROUGH SCIM as a 403 SCIM error, the * account stays active — plus the positive control (a second * administrator makes the same request succeed) that proves the guard - * was the thing refusing. + * was the thing refusing. Both SCIM lifecycle verbs are driven: + * `PATCH active: false` and `DELETE /Users/{id}`, which on 1.7.2 + * reach the guard through the SAME `beforeUpdate` ban write. * (d) negative controls: a PATCH that does not change `active` touches no * ban column and revokes nothing; an administrator's ban survives an - * IdP attribute sync and an explicit `active: true`. + * IdP attribute sync and an explicit `active: true`; a deactivation + * makes an administrator's EXPIRING ban permanent — paired with the + * positive control that the SAME expiry, with no deactivation, DOES + * re-admit, so what holds the refusal is the `ban_expires` clearing + * and not an expiry that never elapsed. * (e) a host that declines the admin plugin beside SCIM is still refused at * construction (#13816 — unchanged by this card). * (f) `DELETE /Users/{id}` leaves the tombstoned account disabled (the @@ -477,6 +483,44 @@ describe('[#14360] deactivating the last administrator is refused through SCIM, expect(last.status).toBe(403); expect(isBanned(await userRow(h, deputy.email))).toBe(false); }, 60_000); + + it('(c) DELETE /Users/{id} of the last administrator is refused the same way — a 1.7.2 deprovision is a ban, not a row delete', async () => { + const h = await boot(); + const owner = await provision(h, 'owner'); + await attachPassword(h, owner); + await makePlatformAdmin(h, owner.userId); + await expectSignInAccepted(h, owner.email); + + // The other lifecycle verb an IdP deprovisions with, and the one the + // guard's own header used to file under "deleting the `sys_user` row". + // On 1.7.2 it is not a row delete: the vendor tombstones the SCIM source + // and leaves the better-auth user in place (face (f)), the aggregate + // turns inactive, and the deprovision arrives as the SAME ban write as + // `active: false` — so it is `guardBan` on `beforeUpdate` that refuses + // here, and `guardDelete` on `beforeDelete` never runs at all. The + // mechanism covered this from the day #14360 landed; nothing drove it. + const res = await h.send(scimRequest(h, 'DELETE', `/Users/${owner.scimId}`)); + const body = (await res.json()) as { schemas?: string[]; status?: string; detail?: string }; + expect(res.status, `expected the guard's 403, got ${res.status} ${JSON.stringify(body)}`).toBe( + 403, + ); + expect(body.schemas ?? []).toContain(SCIM_ERROR_SCHEMA); + expect(String(body.status)).toBe('403'); + expect(body.detail).toMatch(/last administrator/i); + expect(body.detail).toMatch(/ADR-0024 D5\.2/); + + // Nothing landed: the row is still there, still unbanned, and the last + // administrator still signs in — the lockout the guard exists to stop. + const row = await userRow(h, owner.email); + expect(row, 'the refusal must leave the sys_user row in place').toBeTruthy(); + expect(isBanned(row)).toBe(false); + expect(row?.ban_reason ?? null).toBeNull(); + await expectSignInAccepted(h, owner.email); + + // …and the vendor's own tombstone rolled back with the refusal (#14522), + // so the IdP still sees an active resource, not a half-deprovisioned one. + expect(await scimActive(h, owner.scimId)).toBe(true); + }, 60_000); }); // --------------------------------------------------------------------------- @@ -540,6 +584,21 @@ describe('[#14360] a SCIM update that does not change `active` touches no ban co expect(row?.ban_reason).toBe('Policy violation'); }, 60_000); + /** + * The administrator's timed ban in the face below and in its positive + * control. Wide enough that the pre-PATCH sign-in — which has to land while + * the ban is still in force — is not racing the clock: on a loaded CI shard + * wall-clock time is not the test's progress (this file's own module import + * alone measured 12.7 s on an uncontended box), and the 1.5 s this replaces + * was a flake margin rather than a defect. Nothing either face proves + * depends on the number: the proof is that the ban OUTLIVES the expiry + * because `ban_expires` was nulled, and the control's is that the same + * expiry, left alone, re-admits. + */ + const TIMED_BAN_MS = 5_000; + /** `expiry + 500 ms` — puts both faces past the expiry with margin to spare. */ + const TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500; + it("(d) a deactivation makes an administrator's EXPIRING ban permanent — the expiry cannot re-admit a deactivated principal", async () => { const h = await boot(); const hana = await provision(h, 'hana'); @@ -549,7 +608,7 @@ describe('[#14360] a SCIM update that does not change `active` touches no ban co // moment `banExpires` is in the past, and nothing re-invokes the SCIM // callback until the IdP mutates the user again — so an expiry left in // place would ADMIT a principal the IdP still holds deactivated. - const expiresAt = new Date(Date.now() + 1_500); + const expiresAt = new Date(Date.now() + TIMED_BAN_MS); await h.engine.update( 'sys_user', { id: hana.userId, banned: true, ban_reason: 'Policy violation', ban_expires: expiresAt }, @@ -570,13 +629,43 @@ describe('[#14360] a SCIM update that does not change `active` touches no ban co // Let the administrator's expiry pass, then prove the refusal still holds // (status AND code): without the clearing above the vendor would have // auto-unbanned here and answered 2xx. - await new Promise((resolve) => setTimeout(resolve, 2_000)); + await new Promise((resolve) => setTimeout(resolve, TIMED_BAN_WAIT_MS)); expect(Date.now()).toBeGreaterThan(expiresAt.getTime()); await expectSignInBanned(h, hana.email); row = await userRow(h, hana.email); expect(isBanned(row)).toBe(true); expect(row?.ban_reason).toBe('Policy violation'); }, 60_000); + + it('(d) positive control: with NO SCIM deactivation the same expiry DOES re-admit — so it is the `ban_expires` clearing that holds the refusal above', async () => { + const h = await boot(); + const hana = await provision(h, 'hana'); + await attachPassword(h, hana); + + // Identical setup to the face above — same timed administrator ban, same + // wait — with exactly one thing taken away: the SCIM deactivation. Without + // this line a green face above is equally well explained by "the expiry + // never elapsed", which is the ambiguity a WIDER window would otherwise + // deepen: the refusal would be pinned by a clock rather than by the write + // under test. + const expiresAt = new Date(Date.now() + TIMED_BAN_MS); + await h.engine.update( + 'sys_user', + { id: hana.userId, banned: true, ban_reason: 'Policy violation', ban_expires: expiresAt }, + SYSTEM, + ); + await expectSignInBanned(h, hana.email); + + await new Promise((resolve) => setTimeout(resolve, TIMED_BAN_WAIT_MS)); + expect(Date.now()).toBeGreaterThan(expiresAt.getTime()); + + // The vendor's `session.create` hook auto-lifts a ban whose `banExpires` + // is in the past and lets the sign-in through — clearing the row as it + // goes. That auto-lift is live here, which is exactly what the face above + // disables by nulling the expiry. + await expectSignInAccepted(h, hana.email); + expect(isBanned(await userRow(h, hana.email))).toBe(false); + }, 60_000); }); // --------------------------------------------------------------------------- From 58469f500403430351eb46a31c627933ca8efa53 Mon Sep 17 00:00:00 2001 From: "claude[bot]" Date: Thu, 3 Sep 2026 02:32:47 +0000 Subject: [PATCH 2/3] docs(plugin-auth): describe the 1.7.2 SCIM lifecycle where the prose still describes 1.6.x MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `last-admin-guard.ts`'s header listed two things that stopped being true when the vendor's own ban write was removed in 1.7.0. Shape (1) credited `@better-auth/scim` with mapping `active: false` onto the admin ban; that mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2) filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on 1.7.2 the vendor tombstones the SCIM source and the deprovision arrives as shape (1)'s ban write, so it is refused by `guardBan` on `beforeUpdate` and never reaches `guardDelete` — measured by ablation, both directions. `auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as the home of the platform ban write while the module's own import reads `./user-ban-write.js`. Both modules exist, so the pointer sent the reader to the wrong one of two live files rather than to a missing one. Its DELETE paragraph also left implicit what the PATCH bullet above it states: the deactivation branch clears `banExpires`, so a DELETE makes an administrator's timed ban permanent too. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- .../plugins/plugin-auth/src/auth-manager.ts | 12 ++++++--- .../plugin-auth/src/last-admin-guard.ts | 25 ++++++++++++++----- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 66ba6e5cd1..3a46671474 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -5135,7 +5135,7 @@ export class AuthManager { * generated docs and the #13816 refusal all asserted the ban. * * This method restores declared = enforced by routing the state to the - * platform's OWN ban write (`admin-ban-endpoints.ts`): + * platform's OWN ban write (`user-ban-write.ts`): * * - `active: false` on a row that is not banned ⇒ `applyUserBan` with * `SCIM_DEACTIVATION_BAN_REASON` and no expiry. The vendor's @@ -5164,8 +5164,14 @@ export class AuthManager { * A consequence worth stating: on 1.7.2 a SCIM `DELETE /Users/{id}` no * longer deletes the better-auth user (the vendor tombstones the source); * it leaves the user with no active source, so this callback disables the - * account. Re-provisioning through the tombstone re-links the same user, - * the state turns active, and the SCIM ban is lifted by the second bullet. + * account — by the SAME branch as `active: false`, including over an + * administrator's timed ban, whose `banExpires` a DELETE therefore clears + * too: a deprovision cannot be outlived by an expiry the administrator set. + * For the same reason a DELETE is judged by the `beforeUpdate` guard below + * and never by any `beforeDelete` — deleting the last administrator through + * SCIM is refused exactly as deactivating them is. Re-provisioning through + * the tombstone re-links the same user, the state turns active, and the + * SCIM ban is lifted by the second bullet. * * The break-glass last-administrator guard (ADR-0024 D5.2, #5892) is an * ENGINE `beforeUpdate` hook on `sys_user`, so it judges this write exactly diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.ts b/packages/plugins/plugin-auth/src/last-admin-guard.ts index e45a2c6a39..40daa74e67 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.ts @@ -8,12 +8,25 @@ * holds on all of them — they are one invariant, not five policies: * * 1. **`sys_user.banned = true`** (#5892) — how every *disable* 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). - * 2. **deleting the `sys_user` row** (#5941) — how every *remove* lands: SCIM - * `DELETE /Users/{id}`, better-auth's `/admin/remove-user` and - * `/delete-user`, an import, a script. + * better-auth admin plugin's ban endpoint writes it, and so does the SCIM + * lifecycle. `@better-auth/scim` 1.6.x mapped a SCIM `active: false` onto + * that same admin ban itself; 1.7.0 removed that write and replaced it + * with an OPTIONAL host callback, so since #14360 the ban is landed by + * `plugin-auth`'s own `identity.reconcileUser` (`auth-manager.ts` -> + * `user-ban-write.ts`) — the same column and the same hook, a different + * author. SCIM still forces the admin plugin on (ADR-0071), which is what + * supplies the column and the `BANNED_USER` sign-in refusal. + * 2. **deleting the `sys_user` row** (#5941) — how every *remove* lands: + * better-auth's `/admin/remove-user` and `/delete-user`, an import, a + * script. ⚠️ SCIM `DELETE /Users/{id}` is NOT one of them on 1.7.2: the + * vendor tombstones the SCIM source and leaves the better-auth user in + * place, so the user is simply left with no active source, the aggregate + * turns inactive, and the deprovision arrives as shape (1)'s ban write. + * It is therefore refused by `guardBan` on `beforeUpdate` and never + * reaches `guardDelete` — measured by ablation on #14555: removing the + * `beforeDelete` registration leaves the SCIM DELETE face green, removing + * the `beforeUpdate` one reddens it. The row-delete half below still + * holds for every other remove path named here. * 3. **revoking the STANDING, leaving the user row untouched** (#5978) — the * shape neither of the first two can see, because "who is an * administrator" is not a fact stored on `sys_user` at all. It lives in the From f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 03:06:23 +0000 Subject: [PATCH 3/3] docs(permissions): re-anchor the system-context census citation after the guard header grew MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last-admin-guard header rewrite in this branch added 13 net lines above the `applies to EVERY context, isSystem included` line, moving it from 286 to 299. `check-system-context-census` anchors that line from `content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row finds it by NEEDLE rather than by number — so the ledger row followed the code and the page's hard-coded number did not, reddening the gate in both directions at once (anchor-is-not-a-read-site + ledger-row-unused). Repaired by the gate's own `--fix`, which is the authority on the form: re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299` Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/system-context.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b26e78e827..74036c4b80 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -198,7 +198,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10007`–`10024` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | -| "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` | +| "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` | | "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1522`, `:1551`; `domains/actions.ts:404` | ---