Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` |

---
Expand Down
12 changes: 9 additions & 3 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
25 changes: 19 additions & 6 deletions packages/plugins/plugin-auth/src/last-admin-guard.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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);
});

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -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');
Expand All@@ -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 },
Expand All@@ -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);
});

// ---------------------------------------------------------------------------
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` |

---
Expand Down
12 changes: 9 additions & 3 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
25 changes: 19 additions & 6 deletions packages/plugins/plugin-auth/src/last-admin-guard.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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);
});

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -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');
Expand All@@ -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 },
Expand All@@ -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);
});

// ---------------------------------------------------------------------------
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` |

---
Expand Down
12 changes: 9 additions & 3 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
25 changes: 19 additions & 6 deletions packages/plugins/plugin-auth/src/last-admin-guard.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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);
});

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -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');
Expand All@@ -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 },
Expand All@@ -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);
});

// ---------------------------------------------------------------------------
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` |

---
Expand Down
12 changes: 9 additions & 3 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
25 changes: 19 additions & 6 deletions packages/plugins/plugin-auth/src/last-admin-guard.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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);
});

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -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');
Expand All@@ -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 },
Expand All@@ -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);
});

// ---------------------------------------------------------------------------
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` |

---
Expand Down
12 changes: 9 additions & 3 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
25 changes: 19 additions & 6 deletions packages/plugins/plugin-auth/src/last-admin-guard.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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);
});

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -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');
Expand All@@ -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 },
Expand All@@ -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);
});

// ---------------------------------------------------------------------------
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` |

---
Expand Down
12 changes: 9 additions & 3 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
25 changes: 19 additions & 6 deletions packages/plugins/plugin-auth/src/last-admin-guard.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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);
});

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -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');
Expand All@@ -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 },
Expand All@@ -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);
});

// ---------------------------------------------------------------------------
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` |

---
Expand Down
12 changes: 9 additions & 3 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
25 changes: 19 additions & 6 deletions packages/plugins/plugin-auth/src/last-admin-guard.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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);
});

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -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');
Expand All@@ -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 },
Expand All@@ -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);
});

// ---------------------------------------------------------------------------
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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` |

---
Expand Down
12 changes: 9 additions & 3 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
Expand Down
25 changes: 19 additions & 6 deletions packages/plugins/plugin-auth/src/last-admin-guard.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
*
Expand DownExpand Up@@ -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
Expand DownExpand Up@@ -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);
});

// ---------------------------------------------------------------------------
Expand DownExpand Up@@ -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');
Expand All@@ -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 },
Expand All@@ -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);
});

// ---------------------------------------------------------------------------
Expand Down
Loading