fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360) - #14540

Merged
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user
Sep 2, 2026
Merged

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360)#14540
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#14360

What this does

Stable @better-auth/scim (1.7.0 and later; the repo pins and installs 1.7.2) no longer writes the admin plugin's banned column when an identity provider sends active: false — it hands the user's aggregate lifecycle state to an OPTIONAL host callback, identity.reconcileUser, and then only revokes sessions. plugin-auth passed no identity member, so a deactivation revoked sessions and wrote nothing: sys_user.banned stayed false and a user holding a local password signed straight back in, while ADR-0071, the generated docs and the #13816 construction-time refusal all asserted the ban.

Limb A of the triage ruling, and only limb A:

  • auth-manager.ts — the scim({...}) options gain identity.reconcileUser, routed to a new private method reconcileScimUserLifecycle that reads the user through the vendor's transaction-bound adapter and reconciles: active: false on a row that is not banned ⇒ the platform ban write (reason Deactivated via SCIM, the exact string the vendor wrote itself through 1.6.x; no expiry), and on a row already banned WITH an expiry (an administrator's timed ban) ⇒ the ban is made permanent (banExpires cleared, banned and the administrator's reason untouched — the vendor's session hook auto-lifts an expired ban and would otherwise admit a principal the IdP still holds deactivated); active: true on a row banned WITH that reason ⇒ unban; anything else is a no-op (the callback is contractually idempotent and the vendor calls it on EVERY user mutation).
  • user-ban-write.ts (new, PACKAGE-INTERNAL — not re-exported from src/index.ts) holds the existing /admin/ban-user / /admin/unban-user write as applyUserBan / applyUserUnban over a minimal UserBanWriter surface; admin-ban-endpoints.ts and the SCIM hook both call that ONE write (field for field the vendor's own banUser shape). No new HTTP endpoint, no new public symbol. The admin mount's behaviour is unchanged (its own suite: 88/88 across the six ban-adjacent files).
  • New end-to-end suite scim-deactivation-reconcile-user.test.ts driving @better-auth/scim itself through a real AuthManager over better-sqlite3 (the simulated deprovision() helper in last-admin-guard.test.ts is NOT extended, per triage).
  • One changeset for @objectstack/plugin-auth, level patch (see Contract review).

Admin forcing untouched (admin: pluginConfig.admin ?? scimEffective at three sites) — that is the #13816 shape-1 follow-up, not this card. The five stale assertion sites triage listed are untouched and are true again under this change.

Premise (re-verified on origin/main00ff228fe with the package installed)

  1. grep -ro "ban" node_modules/@better-auth/scim/dist | wc -l = 0; positive control over node_modules/better-auth/dist/plugins/admin = 117. dist/index.d.mts:267 declares reconcileUser?(input: SCIMIdentityState, context: SCIMTransactionContext); SCIMIdentityState carries userId, the aggregate active (true while ANY participating SCIM source is active), profileSourceId and sources; SCIMTransactionContext carries only database (the DBTransactionAdapter). A throw from the hook: runSCIMApplicationCallback rethrows a better-auth APIError unchanged and wraps anything else as a SCIM 500 with detail SCIM identity reconciliation failed and the original as cause — never swallowed.
  2. auth-manager.ts:3265 handed scim() exactly connections: [] and authentication.verifyBearerToken; repo-wide reconcileUser = 0 hits (control verifyBearerToken = 15).
  3. admin-ban-endpoints.ts owns the platform ban write and re-runs the last-LOCAL-credential guard; the last-ADMINISTRATOR guard (ADR-0024 D5.2, plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) is an ENGINE beforeUpdate hook in last-admin-guard.ts, registered by auth-plugin.ts at kernel:ready, so it holds on every banned write regardless of caller. Both writes were endpoint-shaped (they parsed a Request); the write itself is now a plain callable.
  4. The forced admin plugin supplies the banned column and the sign-in refusal: better-auth/dist/plugins/admin/admin.mjs:36-47 reads user?.banned in the session.create hook and throws FORBIDDEN / BANNED_USER.

PM hypotheses

One consequence stated rather than left implicit: on 1.7.2 a SCIM DELETE /Users/{id} tombstones the source instead of deleting the better-auth user, so the aggregate state turns inactive and the hook disables that account (face (f)); re-provisioning through the tombstone re-links the same user and the SCIM ban is lifted.

Tests (head 21c7dbe76)

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/scim-deactivation-reconcile-user.test.ts + the five ban-adjacent siblings (admin-ban-endpoints, admin-remove-user-gate-ordering, break-glass-guard-self-service-target, break-glass-local-credential, session-tombstone) — Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0.
  • The six SCIM-adjacent siblings (auth-manager, better-auth-schema-parity, credential-at-rest-posture, last-admin-guard, managed-extension-fields, scim-case-insensitive-identifier) — Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0.
  • tsc --noEmit (src program) exit 0; pnpm --filter @objectstack/plugin-auth check:test-typecheckOK — the test layer compiles under tsconfig.test.json (the new test file is in that program; the src program excludes tests by design). The tsconfig.examples.json half of the typecheck script resolves the package's own dist types, which this worktree has not built — NOT MEASURED locally; CI builds it.
  • Reverse verification on the committed tree (round-1 head 51000d482; the wiring line it removes is unchanged on 21c7dbe76, so the reading was not re-run) (scratchpad/issue-14360/ablation.sh, trap-restored): removed the three-line identity member from the scim() options — proven on disk (marker count 1 → 0, git diff --stat = 3 deletions) — and ran the suite: 6 red / 3 green, exactly (a), (b), the idempotency case, (c) [the request answers 200 instead of the guard's 403], the (c) positive control and (f) red; (d) attribute-only, (d) administrator-ban and (e) green. Restore via git checkout HEAD -- path: git diff HEAD empty, disk blob = HEAD blob e9d5c5eb…, marker count back to 1. No dist rebuild is involved: the suite imports ./auth-manager.js from src, and no workspace dependency was mutated.

Gates (head 21c7dbe76)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 36 commands from the changeset (re-derived on 21c7dbe76 after the changed-file set grew by user-ban-write.ts: identical list); each run at 21c7dbe76 with its exit captured before any pipe. 33 green, including check-system-context-census (OK — 109 elevation read sites … all anchored; 0 anchors on the two edited files, no census fix needed), check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:logger-receiver-detach, check-adr-0087-registration (non-breaking changeset), check-changeset-no-major. Three answered exit 3 = PREREQUISITE NOT MET by their own verdict text, recorded as NOT MEASURED locally (CI owns them): check-test-completeness (needs a saved turbo test log), check:dual-build-cjs-loads (needs pnpm build), check:type-check-debt (needs the full built closure). Control-character self-scan over the four changed files: clean.

Contract review (Clause ②)

  • Behaviour change, as accept/reject: a principal an identity provider has deactivated over SCIM (active: false, or deleted) is now REJECTED at sign-in with the vendor's 403 BANNED_USER (before: sessions revoked, sign-in ACCEPTED); an IdP active: true re-ACCEPTS a principal whose ban carries the SCIM reason (an administrator's ban stays a rejection). Deactivating the last administrator over SCIM is REJECTED with a 403 SCIM error (before: accepted, disabling nothing).
  • Exported-symbol delta (git diff -U0 00ff228fe...HEAD | grep -E '^[+-].*\bexport\b'): five export lines, ALL in the new package-internal module packages/plugins/plugin-auth/src/user-ban-write.ts (UserBanWriter, UserBanFields, SCIM_DEACTIVATION_BAN_REASON, applyUserBan, applyUserUnban). NO new export is reachable through src/index.ts: the barrel is untouched (84 lines before and after, zero references to user-ban-write), and the same grep over admin-ban-endpoints.ts, auth-manager.ts and index.ts prints nothing. No removals, no signature changes to existing exports. The scim() options gain the vendor's identity member (not an ObjectStack-authored config key).
  • Changeset level:patch — a bug fix restoring the declared behaviour with no new public symbol. Clause-② stays yes: sign-in accept/reject changes for a class of principals; the needs:contract-review carriers stay on the card and the PR.
  • Log levels: no new log site of any level; the last-admin refusal is an error RESPONSE through the vendor's SCIM error mapping.

Out of scope, filed

Patch round 2 (head 21c7dbe76) — the in-seat contract review's round-1 verdict, item by item

  • A (blocking finding 1 — implemented exactly as written).reconcileScimUserLifecycle reads banExpires too; in the !state.active branch a banned row with a non-null expiry gets { banExpires: null, updatedAt } and returns, otherwise returns — banned and the administrator's banReason untouched. Docblock first bullet and the changeset sentence say a deactivation also makes an existing expiring ban permanent. New face under (d): administrator ban with ban_expires 1.5s ahead, PATCH active: false → 200, expiry cleared and reason kept, then after the expiry has passed sign-in still answers 403 BANNED_USER (status AND code).
  • B (note 1, required). Shared write moved to the non-barrelled user-ban-write.ts; admin-ban-endpoints.ts, auth-manager.ts and the suite import it. Barrel export surface unchanged; changeset patch; the "New exports" paragraph removed.
  • C (notes 2, 3, 4, 6, 7). (2) changeset: the last-LOCAL-credential guard is deliberately not applied on the SCIM path — an IdP deprovision can disable the last password-holding account while non-administrator SSO users remain. (3) marker collision documented in the method docblock and the changeset (an administrator who types Deactivated via SCIM produces a ban the IdP can lift); the string is NOT reserved on the admin mount. (4) post-refusal behaviour stated above and on SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522. (6) new face (g): POST /Users with active: false provisions the account disabled — banned with the SCIM reason, sign-in 403 BANNED_USER. (7) SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522 carries the line-487 pin citation.
  • D (verification at 21c7dbe76, verify-lock sessions each under the cap, exits before any pipe). New suite + five ban-adjacent siblings: Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0. Six SCIM-adjacent siblings: Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0. tsc --noEmit exit 0 (0 errors); check:test-typecheckOK — the test layer compiles under tsconfig.test.json. Gates re-derived on 21c7dbe76 (identical 36) and all run there: 33 green, the same three prerequisite-not-met (check-test-completeness, check:dual-build-cjs-loads, check:type-check-debt). git merge-tree --write-tree --name-only origin/main HEAD after git fetch origin main (origin/main = fed4fa409): clean, no conflicted paths. GitHub mergeable_state read after the push: clean.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

… the platform ban write
SCIM active:false revoked sessions and wrote nothing on stable
@better-auth/scim (the vendor's ban write left the package in 1.7.0);
sys_user.banned was never set and a local-password user signed straight
back in. Wire identity.reconcileUser into the scim() options and route it
to the shared ban/unban write in admin-ban-endpoints.ts, inside the SCIM
transaction; the engine-level last-administrator guard judges it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…M transaction scope leaves (#14522)
The (c) face keeps the account enabled and the SCIM 403 shape; the vendor's
own scimUser.active write surviving the refusal is pinned as observed and
attributed to the adapter's SCIM transaction scoping never engaging on
1.7.2, so the fix for that seam flips the pin deliberately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 15 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 32800235fc78c29dffbfc4bdfd601e0028eb72ca — the merge of head 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f into base fed4fa40924bd1366354ac9b8a55db9d94a7e404, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32800235fc78c29dffbfc4bdfd601e0028eb72ca && git checkout 32800235fc78c29dffbfc4bdfd601e0028eb72ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fed4fa40924bd1366354ac9b8a55db9d94a7e404 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f && git checkout -B drift-repro fed4fa40924bd1366354ac9b8a55db9d94a7e404 && git merge --no-ff 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f
node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fed4fa40924bd1366354ac9b8a55db9d94a7e404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-salesClaude

Copy link
Copy Markdown
Collaborator

Contract review round 1 at 51000d482: FAIL — patch round 2 in flight

The in-seat isolated Clause-② review (fable tier, transcript-verified 78/78) is posted verbatim on #14360. One blocking finding: a SCIM active: false over a row that already carries an expiring administrator ban leaves the expiry in place, so the vendor's session.create hook auto-unbans and admits the principal once the expiry passes while the identity provider still holds them deactivated. Fix (a pure tightening): on deactivation, make an existing expiring ban permanent (banExpires: null) without touching banned or the administrator's reason; one new suite face pins it.

Also taken up in the same round: the five new symbols move into a non-barrelled module so the package surface does not grow (changeset → patch; Clause-② stays yes for the sign-in accept-set change), plus one-sentence documentation of the last-LOCAL-credential guard exemption on the SCIM path, the marker-collision case, the post-refusal 403 behaviour (also noted on #14522), and a POST /Usersactive: false pin.

needs:contract-review stays on the PR and the card until a delta review passes on the patched head; the PR stays draft.


Generated by Claude Code

…nt; move the shared ban write off the barrel
Contract review round 1 (#14360): a SCIM active:false over an
administrator's TIMED ban left the expiry in place, and the vendor's
session hook auto-lifts an expired ban - so the principal was re-admitted
while the IdP still held them deactivated. The hook now clears banExpires
on that row (reason and banned untouched). The shared write moves to the
package-internal user-ban-write.ts (not re-exported from index.ts), so no
new public symbol ships and the changeset drops to patch. Two faces added:
the expiring-ban overlap and POST /Users with active:false.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@os-salesClaude

Copy link
Copy Markdown
Collaborator

Landing provenance — ready + auto-merge at head 21c7dbe76


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sales@claude
, '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

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360) - #14540

Merged
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user
Sep 2, 2026
Merged

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360)#14540
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#14360

What this does

Stable @better-auth/scim (1.7.0 and later; the repo pins and installs 1.7.2) no longer writes the admin plugin's banned column when an identity provider sends active: false — it hands the user's aggregate lifecycle state to an OPTIONAL host callback, identity.reconcileUser, and then only revokes sessions. plugin-auth passed no identity member, so a deactivation revoked sessions and wrote nothing: sys_user.banned stayed false and a user holding a local password signed straight back in, while ADR-0071, the generated docs and the #13816 construction-time refusal all asserted the ban.

Limb A of the triage ruling, and only limb A:

  • auth-manager.ts — the scim({...}) options gain identity.reconcileUser, routed to a new private method reconcileScimUserLifecycle that reads the user through the vendor's transaction-bound adapter and reconciles: active: false on a row that is not banned ⇒ the platform ban write (reason Deactivated via SCIM, the exact string the vendor wrote itself through 1.6.x; no expiry), and on a row already banned WITH an expiry (an administrator's timed ban) ⇒ the ban is made permanent (banExpires cleared, banned and the administrator's reason untouched — the vendor's session hook auto-lifts an expired ban and would otherwise admit a principal the IdP still holds deactivated); active: true on a row banned WITH that reason ⇒ unban; anything else is a no-op (the callback is contractually idempotent and the vendor calls it on EVERY user mutation).
  • user-ban-write.ts (new, PACKAGE-INTERNAL — not re-exported from src/index.ts) holds the existing /admin/ban-user / /admin/unban-user write as applyUserBan / applyUserUnban over a minimal UserBanWriter surface; admin-ban-endpoints.ts and the SCIM hook both call that ONE write (field for field the vendor's own banUser shape). No new HTTP endpoint, no new public symbol. The admin mount's behaviour is unchanged (its own suite: 88/88 across the six ban-adjacent files).
  • New end-to-end suite scim-deactivation-reconcile-user.test.ts driving @better-auth/scim itself through a real AuthManager over better-sqlite3 (the simulated deprovision() helper in last-admin-guard.test.ts is NOT extended, per triage).
  • One changeset for @objectstack/plugin-auth, level patch (see Contract review).

Admin forcing untouched (admin: pluginConfig.admin ?? scimEffective at three sites) — that is the #13816 shape-1 follow-up, not this card. The five stale assertion sites triage listed are untouched and are true again under this change.

Premise (re-verified on origin/main00ff228fe with the package installed)

  1. grep -ro "ban" node_modules/@better-auth/scim/dist | wc -l = 0; positive control over node_modules/better-auth/dist/plugins/admin = 117. dist/index.d.mts:267 declares reconcileUser?(input: SCIMIdentityState, context: SCIMTransactionContext); SCIMIdentityState carries userId, the aggregate active (true while ANY participating SCIM source is active), profileSourceId and sources; SCIMTransactionContext carries only database (the DBTransactionAdapter). A throw from the hook: runSCIMApplicationCallback rethrows a better-auth APIError unchanged and wraps anything else as a SCIM 500 with detail SCIM identity reconciliation failed and the original as cause — never swallowed.
  2. auth-manager.ts:3265 handed scim() exactly connections: [] and authentication.verifyBearerToken; repo-wide reconcileUser = 0 hits (control verifyBearerToken = 15).
  3. admin-ban-endpoints.ts owns the platform ban write and re-runs the last-LOCAL-credential guard; the last-ADMINISTRATOR guard (ADR-0024 D5.2, plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) is an ENGINE beforeUpdate hook in last-admin-guard.ts, registered by auth-plugin.ts at kernel:ready, so it holds on every banned write regardless of caller. Both writes were endpoint-shaped (they parsed a Request); the write itself is now a plain callable.
  4. The forced admin plugin supplies the banned column and the sign-in refusal: better-auth/dist/plugins/admin/admin.mjs:36-47 reads user?.banned in the session.create hook and throws FORBIDDEN / BANNED_USER.

PM hypotheses

One consequence stated rather than left implicit: on 1.7.2 a SCIM DELETE /Users/{id} tombstones the source instead of deleting the better-auth user, so the aggregate state turns inactive and the hook disables that account (face (f)); re-provisioning through the tombstone re-links the same user and the SCIM ban is lifted.

Tests (head 21c7dbe76)

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/scim-deactivation-reconcile-user.test.ts + the five ban-adjacent siblings (admin-ban-endpoints, admin-remove-user-gate-ordering, break-glass-guard-self-service-target, break-glass-local-credential, session-tombstone) — Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0.
  • The six SCIM-adjacent siblings (auth-manager, better-auth-schema-parity, credential-at-rest-posture, last-admin-guard, managed-extension-fields, scim-case-insensitive-identifier) — Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0.
  • tsc --noEmit (src program) exit 0; pnpm --filter @objectstack/plugin-auth check:test-typecheckOK — the test layer compiles under tsconfig.test.json (the new test file is in that program; the src program excludes tests by design). The tsconfig.examples.json half of the typecheck script resolves the package's own dist types, which this worktree has not built — NOT MEASURED locally; CI builds it.
  • Reverse verification on the committed tree (round-1 head 51000d482; the wiring line it removes is unchanged on 21c7dbe76, so the reading was not re-run) (scratchpad/issue-14360/ablation.sh, trap-restored): removed the three-line identity member from the scim() options — proven on disk (marker count 1 → 0, git diff --stat = 3 deletions) — and ran the suite: 6 red / 3 green, exactly (a), (b), the idempotency case, (c) [the request answers 200 instead of the guard's 403], the (c) positive control and (f) red; (d) attribute-only, (d) administrator-ban and (e) green. Restore via git checkout HEAD -- path: git diff HEAD empty, disk blob = HEAD blob e9d5c5eb…, marker count back to 1. No dist rebuild is involved: the suite imports ./auth-manager.js from src, and no workspace dependency was mutated.

Gates (head 21c7dbe76)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 36 commands from the changeset (re-derived on 21c7dbe76 after the changed-file set grew by user-ban-write.ts: identical list); each run at 21c7dbe76 with its exit captured before any pipe. 33 green, including check-system-context-census (OK — 109 elevation read sites … all anchored; 0 anchors on the two edited files, no census fix needed), check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:logger-receiver-detach, check-adr-0087-registration (non-breaking changeset), check-changeset-no-major. Three answered exit 3 = PREREQUISITE NOT MET by their own verdict text, recorded as NOT MEASURED locally (CI owns them): check-test-completeness (needs a saved turbo test log), check:dual-build-cjs-loads (needs pnpm build), check:type-check-debt (needs the full built closure). Control-character self-scan over the four changed files: clean.

Contract review (Clause ②)

  • Behaviour change, as accept/reject: a principal an identity provider has deactivated over SCIM (active: false, or deleted) is now REJECTED at sign-in with the vendor's 403 BANNED_USER (before: sessions revoked, sign-in ACCEPTED); an IdP active: true re-ACCEPTS a principal whose ban carries the SCIM reason (an administrator's ban stays a rejection). Deactivating the last administrator over SCIM is REJECTED with a 403 SCIM error (before: accepted, disabling nothing).
  • Exported-symbol delta (git diff -U0 00ff228fe...HEAD | grep -E '^[+-].*\bexport\b'): five export lines, ALL in the new package-internal module packages/plugins/plugin-auth/src/user-ban-write.ts (UserBanWriter, UserBanFields, SCIM_DEACTIVATION_BAN_REASON, applyUserBan, applyUserUnban). NO new export is reachable through src/index.ts: the barrel is untouched (84 lines before and after, zero references to user-ban-write), and the same grep over admin-ban-endpoints.ts, auth-manager.ts and index.ts prints nothing. No removals, no signature changes to existing exports. The scim() options gain the vendor's identity member (not an ObjectStack-authored config key).
  • Changeset level:patch — a bug fix restoring the declared behaviour with no new public symbol. Clause-② stays yes: sign-in accept/reject changes for a class of principals; the needs:contract-review carriers stay on the card and the PR.
  • Log levels: no new log site of any level; the last-admin refusal is an error RESPONSE through the vendor's SCIM error mapping.

Out of scope, filed

Patch round 2 (head 21c7dbe76) — the in-seat contract review's round-1 verdict, item by item

  • A (blocking finding 1 — implemented exactly as written).reconcileScimUserLifecycle reads banExpires too; in the !state.active branch a banned row with a non-null expiry gets { banExpires: null, updatedAt } and returns, otherwise returns — banned and the administrator's banReason untouched. Docblock first bullet and the changeset sentence say a deactivation also makes an existing expiring ban permanent. New face under (d): administrator ban with ban_expires 1.5s ahead, PATCH active: false → 200, expiry cleared and reason kept, then after the expiry has passed sign-in still answers 403 BANNED_USER (status AND code).
  • B (note 1, required). Shared write moved to the non-barrelled user-ban-write.ts; admin-ban-endpoints.ts, auth-manager.ts and the suite import it. Barrel export surface unchanged; changeset patch; the "New exports" paragraph removed.
  • C (notes 2, 3, 4, 6, 7). (2) changeset: the last-LOCAL-credential guard is deliberately not applied on the SCIM path — an IdP deprovision can disable the last password-holding account while non-administrator SSO users remain. (3) marker collision documented in the method docblock and the changeset (an administrator who types Deactivated via SCIM produces a ban the IdP can lift); the string is NOT reserved on the admin mount. (4) post-refusal behaviour stated above and on SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522. (6) new face (g): POST /Users with active: false provisions the account disabled — banned with the SCIM reason, sign-in 403 BANNED_USER. (7) SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522 carries the line-487 pin citation.
  • D (verification at 21c7dbe76, verify-lock sessions each under the cap, exits before any pipe). New suite + five ban-adjacent siblings: Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0. Six SCIM-adjacent siblings: Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0. tsc --noEmit exit 0 (0 errors); check:test-typecheckOK — the test layer compiles under tsconfig.test.json. Gates re-derived on 21c7dbe76 (identical 36) and all run there: 33 green, the same three prerequisite-not-met (check-test-completeness, check:dual-build-cjs-loads, check:type-check-debt). git merge-tree --write-tree --name-only origin/main HEAD after git fetch origin main (origin/main = fed4fa409): clean, no conflicted paths. GitHub mergeable_state read after the push: clean.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

… the platform ban write
SCIM active:false revoked sessions and wrote nothing on stable
@better-auth/scim (the vendor's ban write left the package in 1.7.0);
sys_user.banned was never set and a local-password user signed straight
back in. Wire identity.reconcileUser into the scim() options and route it
to the shared ban/unban write in admin-ban-endpoints.ts, inside the SCIM
transaction; the engine-level last-administrator guard judges it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…M transaction scope leaves (#14522)
The (c) face keeps the account enabled and the SCIM 403 shape; the vendor's
own scimUser.active write surviving the refusal is pinned as observed and
attributed to the adapter's SCIM transaction scoping never engaging on
1.7.2, so the fix for that seam flips the pin deliberately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 15 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 32800235fc78c29dffbfc4bdfd601e0028eb72ca — the merge of head 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f into base fed4fa40924bd1366354ac9b8a55db9d94a7e404, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32800235fc78c29dffbfc4bdfd601e0028eb72ca && git checkout 32800235fc78c29dffbfc4bdfd601e0028eb72ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fed4fa40924bd1366354ac9b8a55db9d94a7e404 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f && git checkout -B drift-repro fed4fa40924bd1366354ac9b8a55db9d94a7e404 && git merge --no-ff 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f
node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fed4fa40924bd1366354ac9b8a55db9d94a7e404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-salesClaude

Copy link
Copy Markdown
Collaborator

Contract review round 1 at 51000d482: FAIL — patch round 2 in flight

The in-seat isolated Clause-② review (fable tier, transcript-verified 78/78) is posted verbatim on #14360. One blocking finding: a SCIM active: false over a row that already carries an expiring administrator ban leaves the expiry in place, so the vendor's session.create hook auto-unbans and admits the principal once the expiry passes while the identity provider still holds them deactivated. Fix (a pure tightening): on deactivation, make an existing expiring ban permanent (banExpires: null) without touching banned or the administrator's reason; one new suite face pins it.

Also taken up in the same round: the five new symbols move into a non-barrelled module so the package surface does not grow (changeset → patch; Clause-② stays yes for the sign-in accept-set change), plus one-sentence documentation of the last-LOCAL-credential guard exemption on the SCIM path, the marker-collision case, the post-refusal 403 behaviour (also noted on #14522), and a POST /Usersactive: false pin.

needs:contract-review stays on the PR and the card until a delta review passes on the patched head; the PR stays draft.


Generated by Claude Code

…nt; move the shared ban write off the barrel
Contract review round 1 (#14360): a SCIM active:false over an
administrator's TIMED ban left the expiry in place, and the vendor's
session hook auto-lifts an expired ban - so the principal was re-admitted
while the IdP still held them deactivated. The hook now clears banExpires
on that row (reason and banned untouched). The shared write moves to the
package-internal user-ban-write.ts (not re-exported from index.ts), so no
new public symbol ships and the changeset drops to patch. Two faces added:
the expiring-ban overlap and POST /Users with active:false.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@os-salesClaude

Copy link
Copy Markdown
Collaborator

Landing provenance — ready + auto-merge at head 21c7dbe76


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sales@claude
, '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

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360) - #14540

Merged
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user
Sep 2, 2026
Merged

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360)#14540
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#14360

What this does

Stable @better-auth/scim (1.7.0 and later; the repo pins and installs 1.7.2) no longer writes the admin plugin's banned column when an identity provider sends active: false — it hands the user's aggregate lifecycle state to an OPTIONAL host callback, identity.reconcileUser, and then only revokes sessions. plugin-auth passed no identity member, so a deactivation revoked sessions and wrote nothing: sys_user.banned stayed false and a user holding a local password signed straight back in, while ADR-0071, the generated docs and the #13816 construction-time refusal all asserted the ban.

Limb A of the triage ruling, and only limb A:

  • auth-manager.ts — the scim({...}) options gain identity.reconcileUser, routed to a new private method reconcileScimUserLifecycle that reads the user through the vendor's transaction-bound adapter and reconciles: active: false on a row that is not banned ⇒ the platform ban write (reason Deactivated via SCIM, the exact string the vendor wrote itself through 1.6.x; no expiry), and on a row already banned WITH an expiry (an administrator's timed ban) ⇒ the ban is made permanent (banExpires cleared, banned and the administrator's reason untouched — the vendor's session hook auto-lifts an expired ban and would otherwise admit a principal the IdP still holds deactivated); active: true on a row banned WITH that reason ⇒ unban; anything else is a no-op (the callback is contractually idempotent and the vendor calls it on EVERY user mutation).
  • user-ban-write.ts (new, PACKAGE-INTERNAL — not re-exported from src/index.ts) holds the existing /admin/ban-user / /admin/unban-user write as applyUserBan / applyUserUnban over a minimal UserBanWriter surface; admin-ban-endpoints.ts and the SCIM hook both call that ONE write (field for field the vendor's own banUser shape). No new HTTP endpoint, no new public symbol. The admin mount's behaviour is unchanged (its own suite: 88/88 across the six ban-adjacent files).
  • New end-to-end suite scim-deactivation-reconcile-user.test.ts driving @better-auth/scim itself through a real AuthManager over better-sqlite3 (the simulated deprovision() helper in last-admin-guard.test.ts is NOT extended, per triage).
  • One changeset for @objectstack/plugin-auth, level patch (see Contract review).

Admin forcing untouched (admin: pluginConfig.admin ?? scimEffective at three sites) — that is the #13816 shape-1 follow-up, not this card. The five stale assertion sites triage listed are untouched and are true again under this change.

Premise (re-verified on origin/main00ff228fe with the package installed)

  1. grep -ro "ban" node_modules/@better-auth/scim/dist | wc -l = 0; positive control over node_modules/better-auth/dist/plugins/admin = 117. dist/index.d.mts:267 declares reconcileUser?(input: SCIMIdentityState, context: SCIMTransactionContext); SCIMIdentityState carries userId, the aggregate active (true while ANY participating SCIM source is active), profileSourceId and sources; SCIMTransactionContext carries only database (the DBTransactionAdapter). A throw from the hook: runSCIMApplicationCallback rethrows a better-auth APIError unchanged and wraps anything else as a SCIM 500 with detail SCIM identity reconciliation failed and the original as cause — never swallowed.
  2. auth-manager.ts:3265 handed scim() exactly connections: [] and authentication.verifyBearerToken; repo-wide reconcileUser = 0 hits (control verifyBearerToken = 15).
  3. admin-ban-endpoints.ts owns the platform ban write and re-runs the last-LOCAL-credential guard; the last-ADMINISTRATOR guard (ADR-0024 D5.2, plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) is an ENGINE beforeUpdate hook in last-admin-guard.ts, registered by auth-plugin.ts at kernel:ready, so it holds on every banned write regardless of caller. Both writes were endpoint-shaped (they parsed a Request); the write itself is now a plain callable.
  4. The forced admin plugin supplies the banned column and the sign-in refusal: better-auth/dist/plugins/admin/admin.mjs:36-47 reads user?.banned in the session.create hook and throws FORBIDDEN / BANNED_USER.

PM hypotheses

One consequence stated rather than left implicit: on 1.7.2 a SCIM DELETE /Users/{id} tombstones the source instead of deleting the better-auth user, so the aggregate state turns inactive and the hook disables that account (face (f)); re-provisioning through the tombstone re-links the same user and the SCIM ban is lifted.

Tests (head 21c7dbe76)

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/scim-deactivation-reconcile-user.test.ts + the five ban-adjacent siblings (admin-ban-endpoints, admin-remove-user-gate-ordering, break-glass-guard-self-service-target, break-glass-local-credential, session-tombstone) — Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0.
  • The six SCIM-adjacent siblings (auth-manager, better-auth-schema-parity, credential-at-rest-posture, last-admin-guard, managed-extension-fields, scim-case-insensitive-identifier) — Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0.
  • tsc --noEmit (src program) exit 0; pnpm --filter @objectstack/plugin-auth check:test-typecheckOK — the test layer compiles under tsconfig.test.json (the new test file is in that program; the src program excludes tests by design). The tsconfig.examples.json half of the typecheck script resolves the package's own dist types, which this worktree has not built — NOT MEASURED locally; CI builds it.
  • Reverse verification on the committed tree (round-1 head 51000d482; the wiring line it removes is unchanged on 21c7dbe76, so the reading was not re-run) (scratchpad/issue-14360/ablation.sh, trap-restored): removed the three-line identity member from the scim() options — proven on disk (marker count 1 → 0, git diff --stat = 3 deletions) — and ran the suite: 6 red / 3 green, exactly (a), (b), the idempotency case, (c) [the request answers 200 instead of the guard's 403], the (c) positive control and (f) red; (d) attribute-only, (d) administrator-ban and (e) green. Restore via git checkout HEAD -- path: git diff HEAD empty, disk blob = HEAD blob e9d5c5eb…, marker count back to 1. No dist rebuild is involved: the suite imports ./auth-manager.js from src, and no workspace dependency was mutated.

Gates (head 21c7dbe76)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 36 commands from the changeset (re-derived on 21c7dbe76 after the changed-file set grew by user-ban-write.ts: identical list); each run at 21c7dbe76 with its exit captured before any pipe. 33 green, including check-system-context-census (OK — 109 elevation read sites … all anchored; 0 anchors on the two edited files, no census fix needed), check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:logger-receiver-detach, check-adr-0087-registration (non-breaking changeset), check-changeset-no-major. Three answered exit 3 = PREREQUISITE NOT MET by their own verdict text, recorded as NOT MEASURED locally (CI owns them): check-test-completeness (needs a saved turbo test log), check:dual-build-cjs-loads (needs pnpm build), check:type-check-debt (needs the full built closure). Control-character self-scan over the four changed files: clean.

Contract review (Clause ②)

  • Behaviour change, as accept/reject: a principal an identity provider has deactivated over SCIM (active: false, or deleted) is now REJECTED at sign-in with the vendor's 403 BANNED_USER (before: sessions revoked, sign-in ACCEPTED); an IdP active: true re-ACCEPTS a principal whose ban carries the SCIM reason (an administrator's ban stays a rejection). Deactivating the last administrator over SCIM is REJECTED with a 403 SCIM error (before: accepted, disabling nothing).
  • Exported-symbol delta (git diff -U0 00ff228fe...HEAD | grep -E '^[+-].*\bexport\b'): five export lines, ALL in the new package-internal module packages/plugins/plugin-auth/src/user-ban-write.ts (UserBanWriter, UserBanFields, SCIM_DEACTIVATION_BAN_REASON, applyUserBan, applyUserUnban). NO new export is reachable through src/index.ts: the barrel is untouched (84 lines before and after, zero references to user-ban-write), and the same grep over admin-ban-endpoints.ts, auth-manager.ts and index.ts prints nothing. No removals, no signature changes to existing exports. The scim() options gain the vendor's identity member (not an ObjectStack-authored config key).
  • Changeset level:patch — a bug fix restoring the declared behaviour with no new public symbol. Clause-② stays yes: sign-in accept/reject changes for a class of principals; the needs:contract-review carriers stay on the card and the PR.
  • Log levels: no new log site of any level; the last-admin refusal is an error RESPONSE through the vendor's SCIM error mapping.

Out of scope, filed

Patch round 2 (head 21c7dbe76) — the in-seat contract review's round-1 verdict, item by item

  • A (blocking finding 1 — implemented exactly as written).reconcileScimUserLifecycle reads banExpires too; in the !state.active branch a banned row with a non-null expiry gets { banExpires: null, updatedAt } and returns, otherwise returns — banned and the administrator's banReason untouched. Docblock first bullet and the changeset sentence say a deactivation also makes an existing expiring ban permanent. New face under (d): administrator ban with ban_expires 1.5s ahead, PATCH active: false → 200, expiry cleared and reason kept, then after the expiry has passed sign-in still answers 403 BANNED_USER (status AND code).
  • B (note 1, required). Shared write moved to the non-barrelled user-ban-write.ts; admin-ban-endpoints.ts, auth-manager.ts and the suite import it. Barrel export surface unchanged; changeset patch; the "New exports" paragraph removed.
  • C (notes 2, 3, 4, 6, 7). (2) changeset: the last-LOCAL-credential guard is deliberately not applied on the SCIM path — an IdP deprovision can disable the last password-holding account while non-administrator SSO users remain. (3) marker collision documented in the method docblock and the changeset (an administrator who types Deactivated via SCIM produces a ban the IdP can lift); the string is NOT reserved on the admin mount. (4) post-refusal behaviour stated above and on SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522. (6) new face (g): POST /Users with active: false provisions the account disabled — banned with the SCIM reason, sign-in 403 BANNED_USER. (7) SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522 carries the line-487 pin citation.
  • D (verification at 21c7dbe76, verify-lock sessions each under the cap, exits before any pipe). New suite + five ban-adjacent siblings: Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0. Six SCIM-adjacent siblings: Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0. tsc --noEmit exit 0 (0 errors); check:test-typecheckOK — the test layer compiles under tsconfig.test.json. Gates re-derived on 21c7dbe76 (identical 36) and all run there: 33 green, the same three prerequisite-not-met (check-test-completeness, check:dual-build-cjs-loads, check:type-check-debt). git merge-tree --write-tree --name-only origin/main HEAD after git fetch origin main (origin/main = fed4fa409): clean, no conflicted paths. GitHub mergeable_state read after the push: clean.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

… the platform ban write
SCIM active:false revoked sessions and wrote nothing on stable
@better-auth/scim (the vendor's ban write left the package in 1.7.0);
sys_user.banned was never set and a local-password user signed straight
back in. Wire identity.reconcileUser into the scim() options and route it
to the shared ban/unban write in admin-ban-endpoints.ts, inside the SCIM
transaction; the engine-level last-administrator guard judges it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…M transaction scope leaves (#14522)
The (c) face keeps the account enabled and the SCIM 403 shape; the vendor's
own scimUser.active write surviving the refusal is pinned as observed and
attributed to the adapter's SCIM transaction scoping never engaging on
1.7.2, so the fix for that seam flips the pin deliberately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 15 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 32800235fc78c29dffbfc4bdfd601e0028eb72ca — the merge of head 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f into base fed4fa40924bd1366354ac9b8a55db9d94a7e404, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32800235fc78c29dffbfc4bdfd601e0028eb72ca && git checkout 32800235fc78c29dffbfc4bdfd601e0028eb72ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fed4fa40924bd1366354ac9b8a55db9d94a7e404 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f && git checkout -B drift-repro fed4fa40924bd1366354ac9b8a55db9d94a7e404 && git merge --no-ff 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f
node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fed4fa40924bd1366354ac9b8a55db9d94a7e404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-salesClaude

Copy link
Copy Markdown
Collaborator

Contract review round 1 at 51000d482: FAIL — patch round 2 in flight

The in-seat isolated Clause-② review (fable tier, transcript-verified 78/78) is posted verbatim on #14360. One blocking finding: a SCIM active: false over a row that already carries an expiring administrator ban leaves the expiry in place, so the vendor's session.create hook auto-unbans and admits the principal once the expiry passes while the identity provider still holds them deactivated. Fix (a pure tightening): on deactivation, make an existing expiring ban permanent (banExpires: null) without touching banned or the administrator's reason; one new suite face pins it.

Also taken up in the same round: the five new symbols move into a non-barrelled module so the package surface does not grow (changeset → patch; Clause-② stays yes for the sign-in accept-set change), plus one-sentence documentation of the last-LOCAL-credential guard exemption on the SCIM path, the marker-collision case, the post-refusal 403 behaviour (also noted on #14522), and a POST /Usersactive: false pin.

needs:contract-review stays on the PR and the card until a delta review passes on the patched head; the PR stays draft.


Generated by Claude Code

…nt; move the shared ban write off the barrel
Contract review round 1 (#14360): a SCIM active:false over an
administrator's TIMED ban left the expiry in place, and the vendor's
session hook auto-lifts an expired ban - so the principal was re-admitted
while the IdP still held them deactivated. The hook now clears banExpires
on that row (reason and banned untouched). The shared write moves to the
package-internal user-ban-write.ts (not re-exported from index.ts), so no
new public symbol ships and the changeset drops to patch. Two faces added:
the expiring-ban overlap and POST /Users with active:false.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@os-salesClaude

Copy link
Copy Markdown
Collaborator

Landing provenance — ready + auto-merge at head 21c7dbe76


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sales@claude
, '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

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360) - #14540

Merged
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user
Sep 2, 2026
Merged

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360)#14540
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#14360

What this does

Stable @better-auth/scim (1.7.0 and later; the repo pins and installs 1.7.2) no longer writes the admin plugin's banned column when an identity provider sends active: false — it hands the user's aggregate lifecycle state to an OPTIONAL host callback, identity.reconcileUser, and then only revokes sessions. plugin-auth passed no identity member, so a deactivation revoked sessions and wrote nothing: sys_user.banned stayed false and a user holding a local password signed straight back in, while ADR-0071, the generated docs and the #13816 construction-time refusal all asserted the ban.

Limb A of the triage ruling, and only limb A:

  • auth-manager.ts — the scim({...}) options gain identity.reconcileUser, routed to a new private method reconcileScimUserLifecycle that reads the user through the vendor's transaction-bound adapter and reconciles: active: false on a row that is not banned ⇒ the platform ban write (reason Deactivated via SCIM, the exact string the vendor wrote itself through 1.6.x; no expiry), and on a row already banned WITH an expiry (an administrator's timed ban) ⇒ the ban is made permanent (banExpires cleared, banned and the administrator's reason untouched — the vendor's session hook auto-lifts an expired ban and would otherwise admit a principal the IdP still holds deactivated); active: true on a row banned WITH that reason ⇒ unban; anything else is a no-op (the callback is contractually idempotent and the vendor calls it on EVERY user mutation).
  • user-ban-write.ts (new, PACKAGE-INTERNAL — not re-exported from src/index.ts) holds the existing /admin/ban-user / /admin/unban-user write as applyUserBan / applyUserUnban over a minimal UserBanWriter surface; admin-ban-endpoints.ts and the SCIM hook both call that ONE write (field for field the vendor's own banUser shape). No new HTTP endpoint, no new public symbol. The admin mount's behaviour is unchanged (its own suite: 88/88 across the six ban-adjacent files).
  • New end-to-end suite scim-deactivation-reconcile-user.test.ts driving @better-auth/scim itself through a real AuthManager over better-sqlite3 (the simulated deprovision() helper in last-admin-guard.test.ts is NOT extended, per triage).
  • One changeset for @objectstack/plugin-auth, level patch (see Contract review).

Admin forcing untouched (admin: pluginConfig.admin ?? scimEffective at three sites) — that is the #13816 shape-1 follow-up, not this card. The five stale assertion sites triage listed are untouched and are true again under this change.

Premise (re-verified on origin/main00ff228fe with the package installed)

  1. grep -ro "ban" node_modules/@better-auth/scim/dist | wc -l = 0; positive control over node_modules/better-auth/dist/plugins/admin = 117. dist/index.d.mts:267 declares reconcileUser?(input: SCIMIdentityState, context: SCIMTransactionContext); SCIMIdentityState carries userId, the aggregate active (true while ANY participating SCIM source is active), profileSourceId and sources; SCIMTransactionContext carries only database (the DBTransactionAdapter). A throw from the hook: runSCIMApplicationCallback rethrows a better-auth APIError unchanged and wraps anything else as a SCIM 500 with detail SCIM identity reconciliation failed and the original as cause — never swallowed.
  2. auth-manager.ts:3265 handed scim() exactly connections: [] and authentication.verifyBearerToken; repo-wide reconcileUser = 0 hits (control verifyBearerToken = 15).
  3. admin-ban-endpoints.ts owns the platform ban write and re-runs the last-LOCAL-credential guard; the last-ADMINISTRATOR guard (ADR-0024 D5.2, plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) is an ENGINE beforeUpdate hook in last-admin-guard.ts, registered by auth-plugin.ts at kernel:ready, so it holds on every banned write regardless of caller. Both writes were endpoint-shaped (they parsed a Request); the write itself is now a plain callable.
  4. The forced admin plugin supplies the banned column and the sign-in refusal: better-auth/dist/plugins/admin/admin.mjs:36-47 reads user?.banned in the session.create hook and throws FORBIDDEN / BANNED_USER.

PM hypotheses

One consequence stated rather than left implicit: on 1.7.2 a SCIM DELETE /Users/{id} tombstones the source instead of deleting the better-auth user, so the aggregate state turns inactive and the hook disables that account (face (f)); re-provisioning through the tombstone re-links the same user and the SCIM ban is lifted.

Tests (head 21c7dbe76)

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/scim-deactivation-reconcile-user.test.ts + the five ban-adjacent siblings (admin-ban-endpoints, admin-remove-user-gate-ordering, break-glass-guard-self-service-target, break-glass-local-credential, session-tombstone) — Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0.
  • The six SCIM-adjacent siblings (auth-manager, better-auth-schema-parity, credential-at-rest-posture, last-admin-guard, managed-extension-fields, scim-case-insensitive-identifier) — Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0.
  • tsc --noEmit (src program) exit 0; pnpm --filter @objectstack/plugin-auth check:test-typecheckOK — the test layer compiles under tsconfig.test.json (the new test file is in that program; the src program excludes tests by design). The tsconfig.examples.json half of the typecheck script resolves the package's own dist types, which this worktree has not built — NOT MEASURED locally; CI builds it.
  • Reverse verification on the committed tree (round-1 head 51000d482; the wiring line it removes is unchanged on 21c7dbe76, so the reading was not re-run) (scratchpad/issue-14360/ablation.sh, trap-restored): removed the three-line identity member from the scim() options — proven on disk (marker count 1 → 0, git diff --stat = 3 deletions) — and ran the suite: 6 red / 3 green, exactly (a), (b), the idempotency case, (c) [the request answers 200 instead of the guard's 403], the (c) positive control and (f) red; (d) attribute-only, (d) administrator-ban and (e) green. Restore via git checkout HEAD -- path: git diff HEAD empty, disk blob = HEAD blob e9d5c5eb…, marker count back to 1. No dist rebuild is involved: the suite imports ./auth-manager.js from src, and no workspace dependency was mutated.

Gates (head 21c7dbe76)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 36 commands from the changeset (re-derived on 21c7dbe76 after the changed-file set grew by user-ban-write.ts: identical list); each run at 21c7dbe76 with its exit captured before any pipe. 33 green, including check-system-context-census (OK — 109 elevation read sites … all anchored; 0 anchors on the two edited files, no census fix needed), check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:logger-receiver-detach, check-adr-0087-registration (non-breaking changeset), check-changeset-no-major. Three answered exit 3 = PREREQUISITE NOT MET by their own verdict text, recorded as NOT MEASURED locally (CI owns them): check-test-completeness (needs a saved turbo test log), check:dual-build-cjs-loads (needs pnpm build), check:type-check-debt (needs the full built closure). Control-character self-scan over the four changed files: clean.

Contract review (Clause ②)

  • Behaviour change, as accept/reject: a principal an identity provider has deactivated over SCIM (active: false, or deleted) is now REJECTED at sign-in with the vendor's 403 BANNED_USER (before: sessions revoked, sign-in ACCEPTED); an IdP active: true re-ACCEPTS a principal whose ban carries the SCIM reason (an administrator's ban stays a rejection). Deactivating the last administrator over SCIM is REJECTED with a 403 SCIM error (before: accepted, disabling nothing).
  • Exported-symbol delta (git diff -U0 00ff228fe...HEAD | grep -E '^[+-].*\bexport\b'): five export lines, ALL in the new package-internal module packages/plugins/plugin-auth/src/user-ban-write.ts (UserBanWriter, UserBanFields, SCIM_DEACTIVATION_BAN_REASON, applyUserBan, applyUserUnban). NO new export is reachable through src/index.ts: the barrel is untouched (84 lines before and after, zero references to user-ban-write), and the same grep over admin-ban-endpoints.ts, auth-manager.ts and index.ts prints nothing. No removals, no signature changes to existing exports. The scim() options gain the vendor's identity member (not an ObjectStack-authored config key).
  • Changeset level:patch — a bug fix restoring the declared behaviour with no new public symbol. Clause-② stays yes: sign-in accept/reject changes for a class of principals; the needs:contract-review carriers stay on the card and the PR.
  • Log levels: no new log site of any level; the last-admin refusal is an error RESPONSE through the vendor's SCIM error mapping.

Out of scope, filed

Patch round 2 (head 21c7dbe76) — the in-seat contract review's round-1 verdict, item by item

  • A (blocking finding 1 — implemented exactly as written).reconcileScimUserLifecycle reads banExpires too; in the !state.active branch a banned row with a non-null expiry gets { banExpires: null, updatedAt } and returns, otherwise returns — banned and the administrator's banReason untouched. Docblock first bullet and the changeset sentence say a deactivation also makes an existing expiring ban permanent. New face under (d): administrator ban with ban_expires 1.5s ahead, PATCH active: false → 200, expiry cleared and reason kept, then after the expiry has passed sign-in still answers 403 BANNED_USER (status AND code).
  • B (note 1, required). Shared write moved to the non-barrelled user-ban-write.ts; admin-ban-endpoints.ts, auth-manager.ts and the suite import it. Barrel export surface unchanged; changeset patch; the "New exports" paragraph removed.
  • C (notes 2, 3, 4, 6, 7). (2) changeset: the last-LOCAL-credential guard is deliberately not applied on the SCIM path — an IdP deprovision can disable the last password-holding account while non-administrator SSO users remain. (3) marker collision documented in the method docblock and the changeset (an administrator who types Deactivated via SCIM produces a ban the IdP can lift); the string is NOT reserved on the admin mount. (4) post-refusal behaviour stated above and on SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522. (6) new face (g): POST /Users with active: false provisions the account disabled — banned with the SCIM reason, sign-in 403 BANNED_USER. (7) SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522 carries the line-487 pin citation.
  • D (verification at 21c7dbe76, verify-lock sessions each under the cap, exits before any pipe). New suite + five ban-adjacent siblings: Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0. Six SCIM-adjacent siblings: Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0. tsc --noEmit exit 0 (0 errors); check:test-typecheckOK — the test layer compiles under tsconfig.test.json. Gates re-derived on 21c7dbe76 (identical 36) and all run there: 33 green, the same three prerequisite-not-met (check-test-completeness, check:dual-build-cjs-loads, check:type-check-debt). git merge-tree --write-tree --name-only origin/main HEAD after git fetch origin main (origin/main = fed4fa409): clean, no conflicted paths. GitHub mergeable_state read after the push: clean.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

… the platform ban write
SCIM active:false revoked sessions and wrote nothing on stable
@better-auth/scim (the vendor's ban write left the package in 1.7.0);
sys_user.banned was never set and a local-password user signed straight
back in. Wire identity.reconcileUser into the scim() options and route it
to the shared ban/unban write in admin-ban-endpoints.ts, inside the SCIM
transaction; the engine-level last-administrator guard judges it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…M transaction scope leaves (#14522)
The (c) face keeps the account enabled and the SCIM 403 shape; the vendor's
own scimUser.active write surviving the refusal is pinned as observed and
attributed to the adapter's SCIM transaction scoping never engaging on
1.7.2, so the fix for that seam flips the pin deliberately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 15 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 32800235fc78c29dffbfc4bdfd601e0028eb72ca — the merge of head 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f into base fed4fa40924bd1366354ac9b8a55db9d94a7e404, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32800235fc78c29dffbfc4bdfd601e0028eb72ca && git checkout 32800235fc78c29dffbfc4bdfd601e0028eb72ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fed4fa40924bd1366354ac9b8a55db9d94a7e404 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f && git checkout -B drift-repro fed4fa40924bd1366354ac9b8a55db9d94a7e404 && git merge --no-ff 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f
node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fed4fa40924bd1366354ac9b8a55db9d94a7e404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-salesClaude

Copy link
Copy Markdown
Collaborator

Contract review round 1 at 51000d482: FAIL — patch round 2 in flight

The in-seat isolated Clause-② review (fable tier, transcript-verified 78/78) is posted verbatim on #14360. One blocking finding: a SCIM active: false over a row that already carries an expiring administrator ban leaves the expiry in place, so the vendor's session.create hook auto-unbans and admits the principal once the expiry passes while the identity provider still holds them deactivated. Fix (a pure tightening): on deactivation, make an existing expiring ban permanent (banExpires: null) without touching banned or the administrator's reason; one new suite face pins it.

Also taken up in the same round: the five new symbols move into a non-barrelled module so the package surface does not grow (changeset → patch; Clause-② stays yes for the sign-in accept-set change), plus one-sentence documentation of the last-LOCAL-credential guard exemption on the SCIM path, the marker-collision case, the post-refusal 403 behaviour (also noted on #14522), and a POST /Usersactive: false pin.

needs:contract-review stays on the PR and the card until a delta review passes on the patched head; the PR stays draft.


Generated by Claude Code

…nt; move the shared ban write off the barrel
Contract review round 1 (#14360): a SCIM active:false over an
administrator's TIMED ban left the expiry in place, and the vendor's
session hook auto-lifts an expired ban - so the principal was re-admitted
while the IdP still held them deactivated. The hook now clears banExpires
on that row (reason and banned untouched). The shared write moves to the
package-internal user-ban-write.ts (not re-exported from index.ts), so no
new public symbol ships and the changeset drops to patch. Two faces added:
the expiring-ban overlap and POST /Users with active:false.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@os-salesClaude

Copy link
Copy Markdown
Collaborator

Landing provenance — ready + auto-merge at head 21c7dbe76


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sales@claude
, '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

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360) - #14540

Merged
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user
Sep 2, 2026
Merged

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360)#14540
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#14360

What this does

Stable @better-auth/scim (1.7.0 and later; the repo pins and installs 1.7.2) no longer writes the admin plugin's banned column when an identity provider sends active: false — it hands the user's aggregate lifecycle state to an OPTIONAL host callback, identity.reconcileUser, and then only revokes sessions. plugin-auth passed no identity member, so a deactivation revoked sessions and wrote nothing: sys_user.banned stayed false and a user holding a local password signed straight back in, while ADR-0071, the generated docs and the #13816 construction-time refusal all asserted the ban.

Limb A of the triage ruling, and only limb A:

  • auth-manager.ts — the scim({...}) options gain identity.reconcileUser, routed to a new private method reconcileScimUserLifecycle that reads the user through the vendor's transaction-bound adapter and reconciles: active: false on a row that is not banned ⇒ the platform ban write (reason Deactivated via SCIM, the exact string the vendor wrote itself through 1.6.x; no expiry), and on a row already banned WITH an expiry (an administrator's timed ban) ⇒ the ban is made permanent (banExpires cleared, banned and the administrator's reason untouched — the vendor's session hook auto-lifts an expired ban and would otherwise admit a principal the IdP still holds deactivated); active: true on a row banned WITH that reason ⇒ unban; anything else is a no-op (the callback is contractually idempotent and the vendor calls it on EVERY user mutation).
  • user-ban-write.ts (new, PACKAGE-INTERNAL — not re-exported from src/index.ts) holds the existing /admin/ban-user / /admin/unban-user write as applyUserBan / applyUserUnban over a minimal UserBanWriter surface; admin-ban-endpoints.ts and the SCIM hook both call that ONE write (field for field the vendor's own banUser shape). No new HTTP endpoint, no new public symbol. The admin mount's behaviour is unchanged (its own suite: 88/88 across the six ban-adjacent files).
  • New end-to-end suite scim-deactivation-reconcile-user.test.ts driving @better-auth/scim itself through a real AuthManager over better-sqlite3 (the simulated deprovision() helper in last-admin-guard.test.ts is NOT extended, per triage).
  • One changeset for @objectstack/plugin-auth, level patch (see Contract review).

Admin forcing untouched (admin: pluginConfig.admin ?? scimEffective at three sites) — that is the #13816 shape-1 follow-up, not this card. The five stale assertion sites triage listed are untouched and are true again under this change.

Premise (re-verified on origin/main00ff228fe with the package installed)

  1. grep -ro "ban" node_modules/@better-auth/scim/dist | wc -l = 0; positive control over node_modules/better-auth/dist/plugins/admin = 117. dist/index.d.mts:267 declares reconcileUser?(input: SCIMIdentityState, context: SCIMTransactionContext); SCIMIdentityState carries userId, the aggregate active (true while ANY participating SCIM source is active), profileSourceId and sources; SCIMTransactionContext carries only database (the DBTransactionAdapter). A throw from the hook: runSCIMApplicationCallback rethrows a better-auth APIError unchanged and wraps anything else as a SCIM 500 with detail SCIM identity reconciliation failed and the original as cause — never swallowed.
  2. auth-manager.ts:3265 handed scim() exactly connections: [] and authentication.verifyBearerToken; repo-wide reconcileUser = 0 hits (control verifyBearerToken = 15).
  3. admin-ban-endpoints.ts owns the platform ban write and re-runs the last-LOCAL-credential guard; the last-ADMINISTRATOR guard (ADR-0024 D5.2, plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) is an ENGINE beforeUpdate hook in last-admin-guard.ts, registered by auth-plugin.ts at kernel:ready, so it holds on every banned write regardless of caller. Both writes were endpoint-shaped (they parsed a Request); the write itself is now a plain callable.
  4. The forced admin plugin supplies the banned column and the sign-in refusal: better-auth/dist/plugins/admin/admin.mjs:36-47 reads user?.banned in the session.create hook and throws FORBIDDEN / BANNED_USER.

PM hypotheses

One consequence stated rather than left implicit: on 1.7.2 a SCIM DELETE /Users/{id} tombstones the source instead of deleting the better-auth user, so the aggregate state turns inactive and the hook disables that account (face (f)); re-provisioning through the tombstone re-links the same user and the SCIM ban is lifted.

Tests (head 21c7dbe76)

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/scim-deactivation-reconcile-user.test.ts + the five ban-adjacent siblings (admin-ban-endpoints, admin-remove-user-gate-ordering, break-glass-guard-self-service-target, break-glass-local-credential, session-tombstone) — Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0.
  • The six SCIM-adjacent siblings (auth-manager, better-auth-schema-parity, credential-at-rest-posture, last-admin-guard, managed-extension-fields, scim-case-insensitive-identifier) — Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0.
  • tsc --noEmit (src program) exit 0; pnpm --filter @objectstack/plugin-auth check:test-typecheckOK — the test layer compiles under tsconfig.test.json (the new test file is in that program; the src program excludes tests by design). The tsconfig.examples.json half of the typecheck script resolves the package's own dist types, which this worktree has not built — NOT MEASURED locally; CI builds it.
  • Reverse verification on the committed tree (round-1 head 51000d482; the wiring line it removes is unchanged on 21c7dbe76, so the reading was not re-run) (scratchpad/issue-14360/ablation.sh, trap-restored): removed the three-line identity member from the scim() options — proven on disk (marker count 1 → 0, git diff --stat = 3 deletions) — and ran the suite: 6 red / 3 green, exactly (a), (b), the idempotency case, (c) [the request answers 200 instead of the guard's 403], the (c) positive control and (f) red; (d) attribute-only, (d) administrator-ban and (e) green. Restore via git checkout HEAD -- path: git diff HEAD empty, disk blob = HEAD blob e9d5c5eb…, marker count back to 1. No dist rebuild is involved: the suite imports ./auth-manager.js from src, and no workspace dependency was mutated.

Gates (head 21c7dbe76)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 36 commands from the changeset (re-derived on 21c7dbe76 after the changed-file set grew by user-ban-write.ts: identical list); each run at 21c7dbe76 with its exit captured before any pipe. 33 green, including check-system-context-census (OK — 109 elevation read sites … all anchored; 0 anchors on the two edited files, no census fix needed), check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:logger-receiver-detach, check-adr-0087-registration (non-breaking changeset), check-changeset-no-major. Three answered exit 3 = PREREQUISITE NOT MET by their own verdict text, recorded as NOT MEASURED locally (CI owns them): check-test-completeness (needs a saved turbo test log), check:dual-build-cjs-loads (needs pnpm build), check:type-check-debt (needs the full built closure). Control-character self-scan over the four changed files: clean.

Contract review (Clause ②)

  • Behaviour change, as accept/reject: a principal an identity provider has deactivated over SCIM (active: false, or deleted) is now REJECTED at sign-in with the vendor's 403 BANNED_USER (before: sessions revoked, sign-in ACCEPTED); an IdP active: true re-ACCEPTS a principal whose ban carries the SCIM reason (an administrator's ban stays a rejection). Deactivating the last administrator over SCIM is REJECTED with a 403 SCIM error (before: accepted, disabling nothing).
  • Exported-symbol delta (git diff -U0 00ff228fe...HEAD | grep -E '^[+-].*\bexport\b'): five export lines, ALL in the new package-internal module packages/plugins/plugin-auth/src/user-ban-write.ts (UserBanWriter, UserBanFields, SCIM_DEACTIVATION_BAN_REASON, applyUserBan, applyUserUnban). NO new export is reachable through src/index.ts: the barrel is untouched (84 lines before and after, zero references to user-ban-write), and the same grep over admin-ban-endpoints.ts, auth-manager.ts and index.ts prints nothing. No removals, no signature changes to existing exports. The scim() options gain the vendor's identity member (not an ObjectStack-authored config key).
  • Changeset level:patch — a bug fix restoring the declared behaviour with no new public symbol. Clause-② stays yes: sign-in accept/reject changes for a class of principals; the needs:contract-review carriers stay on the card and the PR.
  • Log levels: no new log site of any level; the last-admin refusal is an error RESPONSE through the vendor's SCIM error mapping.

Out of scope, filed

Patch round 2 (head 21c7dbe76) — the in-seat contract review's round-1 verdict, item by item

  • A (blocking finding 1 — implemented exactly as written).reconcileScimUserLifecycle reads banExpires too; in the !state.active branch a banned row with a non-null expiry gets { banExpires: null, updatedAt } and returns, otherwise returns — banned and the administrator's banReason untouched. Docblock first bullet and the changeset sentence say a deactivation also makes an existing expiring ban permanent. New face under (d): administrator ban with ban_expires 1.5s ahead, PATCH active: false → 200, expiry cleared and reason kept, then after the expiry has passed sign-in still answers 403 BANNED_USER (status AND code).
  • B (note 1, required). Shared write moved to the non-barrelled user-ban-write.ts; admin-ban-endpoints.ts, auth-manager.ts and the suite import it. Barrel export surface unchanged; changeset patch; the "New exports" paragraph removed.
  • C (notes 2, 3, 4, 6, 7). (2) changeset: the last-LOCAL-credential guard is deliberately not applied on the SCIM path — an IdP deprovision can disable the last password-holding account while non-administrator SSO users remain. (3) marker collision documented in the method docblock and the changeset (an administrator who types Deactivated via SCIM produces a ban the IdP can lift); the string is NOT reserved on the admin mount. (4) post-refusal behaviour stated above and on SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522. (6) new face (g): POST /Users with active: false provisions the account disabled — banned with the SCIM reason, sign-in 403 BANNED_USER. (7) SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522 carries the line-487 pin citation.
  • D (verification at 21c7dbe76, verify-lock sessions each under the cap, exits before any pipe). New suite + five ban-adjacent siblings: Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0. Six SCIM-adjacent siblings: Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0. tsc --noEmit exit 0 (0 errors); check:test-typecheckOK — the test layer compiles under tsconfig.test.json. Gates re-derived on 21c7dbe76 (identical 36) and all run there: 33 green, the same three prerequisite-not-met (check-test-completeness, check:dual-build-cjs-loads, check:type-check-debt). git merge-tree --write-tree --name-only origin/main HEAD after git fetch origin main (origin/main = fed4fa409): clean, no conflicted paths. GitHub mergeable_state read after the push: clean.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

… the platform ban write
SCIM active:false revoked sessions and wrote nothing on stable
@better-auth/scim (the vendor's ban write left the package in 1.7.0);
sys_user.banned was never set and a local-password user signed straight
back in. Wire identity.reconcileUser into the scim() options and route it
to the shared ban/unban write in admin-ban-endpoints.ts, inside the SCIM
transaction; the engine-level last-administrator guard judges it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…M transaction scope leaves (#14522)
The (c) face keeps the account enabled and the SCIM 403 shape; the vendor's
own scimUser.active write surviving the refusal is pinned as observed and
attributed to the adapter's SCIM transaction scoping never engaging on
1.7.2, so the fix for that seam flips the pin deliberately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 15 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 32800235fc78c29dffbfc4bdfd601e0028eb72ca — the merge of head 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f into base fed4fa40924bd1366354ac9b8a55db9d94a7e404, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32800235fc78c29dffbfc4bdfd601e0028eb72ca && git checkout 32800235fc78c29dffbfc4bdfd601e0028eb72ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fed4fa40924bd1366354ac9b8a55db9d94a7e404 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f && git checkout -B drift-repro fed4fa40924bd1366354ac9b8a55db9d94a7e404 && git merge --no-ff 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f
node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fed4fa40924bd1366354ac9b8a55db9d94a7e404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-salesClaude

Copy link
Copy Markdown
Collaborator

Contract review round 1 at 51000d482: FAIL — patch round 2 in flight

The in-seat isolated Clause-② review (fable tier, transcript-verified 78/78) is posted verbatim on #14360. One blocking finding: a SCIM active: false over a row that already carries an expiring administrator ban leaves the expiry in place, so the vendor's session.create hook auto-unbans and admits the principal once the expiry passes while the identity provider still holds them deactivated. Fix (a pure tightening): on deactivation, make an existing expiring ban permanent (banExpires: null) without touching banned or the administrator's reason; one new suite face pins it.

Also taken up in the same round: the five new symbols move into a non-barrelled module so the package surface does not grow (changeset → patch; Clause-② stays yes for the sign-in accept-set change), plus one-sentence documentation of the last-LOCAL-credential guard exemption on the SCIM path, the marker-collision case, the post-refusal 403 behaviour (also noted on #14522), and a POST /Usersactive: false pin.

needs:contract-review stays on the PR and the card until a delta review passes on the patched head; the PR stays draft.


Generated by Claude Code

…nt; move the shared ban write off the barrel
Contract review round 1 (#14360): a SCIM active:false over an
administrator's TIMED ban left the expiry in place, and the vendor's
session hook auto-lifts an expired ban - so the principal was re-admitted
while the IdP still held them deactivated. The hook now clears banExpires
on that row (reason and banned untouched). The shared write moves to the
package-internal user-ban-write.ts (not re-exported from index.ts), so no
new public symbol ships and the changeset drops to patch. Two faces added:
the expiring-ban overlap and POST /Users with active:false.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@os-salesClaude

Copy link
Copy Markdown
Collaborator

Landing provenance — ready + auto-merge at head 21c7dbe76


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sales@claude
, '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

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360) - #14540

Merged
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user
Sep 2, 2026
Merged

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360)#14540
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#14360

What this does

Stable @better-auth/scim (1.7.0 and later; the repo pins and installs 1.7.2) no longer writes the admin plugin's banned column when an identity provider sends active: false — it hands the user's aggregate lifecycle state to an OPTIONAL host callback, identity.reconcileUser, and then only revokes sessions. plugin-auth passed no identity member, so a deactivation revoked sessions and wrote nothing: sys_user.banned stayed false and a user holding a local password signed straight back in, while ADR-0071, the generated docs and the #13816 construction-time refusal all asserted the ban.

Limb A of the triage ruling, and only limb A:

  • auth-manager.ts — the scim({...}) options gain identity.reconcileUser, routed to a new private method reconcileScimUserLifecycle that reads the user through the vendor's transaction-bound adapter and reconciles: active: false on a row that is not banned ⇒ the platform ban write (reason Deactivated via SCIM, the exact string the vendor wrote itself through 1.6.x; no expiry), and on a row already banned WITH an expiry (an administrator's timed ban) ⇒ the ban is made permanent (banExpires cleared, banned and the administrator's reason untouched — the vendor's session hook auto-lifts an expired ban and would otherwise admit a principal the IdP still holds deactivated); active: true on a row banned WITH that reason ⇒ unban; anything else is a no-op (the callback is contractually idempotent and the vendor calls it on EVERY user mutation).
  • user-ban-write.ts (new, PACKAGE-INTERNAL — not re-exported from src/index.ts) holds the existing /admin/ban-user / /admin/unban-user write as applyUserBan / applyUserUnban over a minimal UserBanWriter surface; admin-ban-endpoints.ts and the SCIM hook both call that ONE write (field for field the vendor's own banUser shape). No new HTTP endpoint, no new public symbol. The admin mount's behaviour is unchanged (its own suite: 88/88 across the six ban-adjacent files).
  • New end-to-end suite scim-deactivation-reconcile-user.test.ts driving @better-auth/scim itself through a real AuthManager over better-sqlite3 (the simulated deprovision() helper in last-admin-guard.test.ts is NOT extended, per triage).
  • One changeset for @objectstack/plugin-auth, level patch (see Contract review).

Admin forcing untouched (admin: pluginConfig.admin ?? scimEffective at three sites) — that is the #13816 shape-1 follow-up, not this card. The five stale assertion sites triage listed are untouched and are true again under this change.

Premise (re-verified on origin/main00ff228fe with the package installed)

  1. grep -ro "ban" node_modules/@better-auth/scim/dist | wc -l = 0; positive control over node_modules/better-auth/dist/plugins/admin = 117. dist/index.d.mts:267 declares reconcileUser?(input: SCIMIdentityState, context: SCIMTransactionContext); SCIMIdentityState carries userId, the aggregate active (true while ANY participating SCIM source is active), profileSourceId and sources; SCIMTransactionContext carries only database (the DBTransactionAdapter). A throw from the hook: runSCIMApplicationCallback rethrows a better-auth APIError unchanged and wraps anything else as a SCIM 500 with detail SCIM identity reconciliation failed and the original as cause — never swallowed.
  2. auth-manager.ts:3265 handed scim() exactly connections: [] and authentication.verifyBearerToken; repo-wide reconcileUser = 0 hits (control verifyBearerToken = 15).
  3. admin-ban-endpoints.ts owns the platform ban write and re-runs the last-LOCAL-credential guard; the last-ADMINISTRATOR guard (ADR-0024 D5.2, plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) is an ENGINE beforeUpdate hook in last-admin-guard.ts, registered by auth-plugin.ts at kernel:ready, so it holds on every banned write regardless of caller. Both writes were endpoint-shaped (they parsed a Request); the write itself is now a plain callable.
  4. The forced admin plugin supplies the banned column and the sign-in refusal: better-auth/dist/plugins/admin/admin.mjs:36-47 reads user?.banned in the session.create hook and throws FORBIDDEN / BANNED_USER.

PM hypotheses

One consequence stated rather than left implicit: on 1.7.2 a SCIM DELETE /Users/{id} tombstones the source instead of deleting the better-auth user, so the aggregate state turns inactive and the hook disables that account (face (f)); re-provisioning through the tombstone re-links the same user and the SCIM ban is lifted.

Tests (head 21c7dbe76)

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/scim-deactivation-reconcile-user.test.ts + the five ban-adjacent siblings (admin-ban-endpoints, admin-remove-user-gate-ordering, break-glass-guard-self-service-target, break-glass-local-credential, session-tombstone) — Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0.
  • The six SCIM-adjacent siblings (auth-manager, better-auth-schema-parity, credential-at-rest-posture, last-admin-guard, managed-extension-fields, scim-case-insensitive-identifier) — Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0.
  • tsc --noEmit (src program) exit 0; pnpm --filter @objectstack/plugin-auth check:test-typecheckOK — the test layer compiles under tsconfig.test.json (the new test file is in that program; the src program excludes tests by design). The tsconfig.examples.json half of the typecheck script resolves the package's own dist types, which this worktree has not built — NOT MEASURED locally; CI builds it.
  • Reverse verification on the committed tree (round-1 head 51000d482; the wiring line it removes is unchanged on 21c7dbe76, so the reading was not re-run) (scratchpad/issue-14360/ablation.sh, trap-restored): removed the three-line identity member from the scim() options — proven on disk (marker count 1 → 0, git diff --stat = 3 deletions) — and ran the suite: 6 red / 3 green, exactly (a), (b), the idempotency case, (c) [the request answers 200 instead of the guard's 403], the (c) positive control and (f) red; (d) attribute-only, (d) administrator-ban and (e) green. Restore via git checkout HEAD -- path: git diff HEAD empty, disk blob = HEAD blob e9d5c5eb…, marker count back to 1. No dist rebuild is involved: the suite imports ./auth-manager.js from src, and no workspace dependency was mutated.

Gates (head 21c7dbe76)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 36 commands from the changeset (re-derived on 21c7dbe76 after the changed-file set grew by user-ban-write.ts: identical list); each run at 21c7dbe76 with its exit captured before any pipe. 33 green, including check-system-context-census (OK — 109 elevation read sites … all anchored; 0 anchors on the two edited files, no census fix needed), check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:logger-receiver-detach, check-adr-0087-registration (non-breaking changeset), check-changeset-no-major. Three answered exit 3 = PREREQUISITE NOT MET by their own verdict text, recorded as NOT MEASURED locally (CI owns them): check-test-completeness (needs a saved turbo test log), check:dual-build-cjs-loads (needs pnpm build), check:type-check-debt (needs the full built closure). Control-character self-scan over the four changed files: clean.

Contract review (Clause ②)

  • Behaviour change, as accept/reject: a principal an identity provider has deactivated over SCIM (active: false, or deleted) is now REJECTED at sign-in with the vendor's 403 BANNED_USER (before: sessions revoked, sign-in ACCEPTED); an IdP active: true re-ACCEPTS a principal whose ban carries the SCIM reason (an administrator's ban stays a rejection). Deactivating the last administrator over SCIM is REJECTED with a 403 SCIM error (before: accepted, disabling nothing).
  • Exported-symbol delta (git diff -U0 00ff228fe...HEAD | grep -E '^[+-].*\bexport\b'): five export lines, ALL in the new package-internal module packages/plugins/plugin-auth/src/user-ban-write.ts (UserBanWriter, UserBanFields, SCIM_DEACTIVATION_BAN_REASON, applyUserBan, applyUserUnban). NO new export is reachable through src/index.ts: the barrel is untouched (84 lines before and after, zero references to user-ban-write), and the same grep over admin-ban-endpoints.ts, auth-manager.ts and index.ts prints nothing. No removals, no signature changes to existing exports. The scim() options gain the vendor's identity member (not an ObjectStack-authored config key).
  • Changeset level:patch — a bug fix restoring the declared behaviour with no new public symbol. Clause-② stays yes: sign-in accept/reject changes for a class of principals; the needs:contract-review carriers stay on the card and the PR.
  • Log levels: no new log site of any level; the last-admin refusal is an error RESPONSE through the vendor's SCIM error mapping.

Out of scope, filed

Patch round 2 (head 21c7dbe76) — the in-seat contract review's round-1 verdict, item by item

  • A (blocking finding 1 — implemented exactly as written).reconcileScimUserLifecycle reads banExpires too; in the !state.active branch a banned row with a non-null expiry gets { banExpires: null, updatedAt } and returns, otherwise returns — banned and the administrator's banReason untouched. Docblock first bullet and the changeset sentence say a deactivation also makes an existing expiring ban permanent. New face under (d): administrator ban with ban_expires 1.5s ahead, PATCH active: false → 200, expiry cleared and reason kept, then after the expiry has passed sign-in still answers 403 BANNED_USER (status AND code).
  • B (note 1, required). Shared write moved to the non-barrelled user-ban-write.ts; admin-ban-endpoints.ts, auth-manager.ts and the suite import it. Barrel export surface unchanged; changeset patch; the "New exports" paragraph removed.
  • C (notes 2, 3, 4, 6, 7). (2) changeset: the last-LOCAL-credential guard is deliberately not applied on the SCIM path — an IdP deprovision can disable the last password-holding account while non-administrator SSO users remain. (3) marker collision documented in the method docblock and the changeset (an administrator who types Deactivated via SCIM produces a ban the IdP can lift); the string is NOT reserved on the admin mount. (4) post-refusal behaviour stated above and on SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522. (6) new face (g): POST /Users with active: false provisions the account disabled — banned with the SCIM reason, sign-in 403 BANNED_USER. (7) SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522 carries the line-487 pin citation.
  • D (verification at 21c7dbe76, verify-lock sessions each under the cap, exits before any pipe). New suite + five ban-adjacent siblings: Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0. Six SCIM-adjacent siblings: Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0. tsc --noEmit exit 0 (0 errors); check:test-typecheckOK — the test layer compiles under tsconfig.test.json. Gates re-derived on 21c7dbe76 (identical 36) and all run there: 33 green, the same three prerequisite-not-met (check-test-completeness, check:dual-build-cjs-loads, check:type-check-debt). git merge-tree --write-tree --name-only origin/main HEAD after git fetch origin main (origin/main = fed4fa409): clean, no conflicted paths. GitHub mergeable_state read after the push: clean.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

… the platform ban write
SCIM active:false revoked sessions and wrote nothing on stable
@better-auth/scim (the vendor's ban write left the package in 1.7.0);
sys_user.banned was never set and a local-password user signed straight
back in. Wire identity.reconcileUser into the scim() options and route it
to the shared ban/unban write in admin-ban-endpoints.ts, inside the SCIM
transaction; the engine-level last-administrator guard judges it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…M transaction scope leaves (#14522)
The (c) face keeps the account enabled and the SCIM 403 shape; the vendor's
own scimUser.active write surviving the refusal is pinned as observed and
attributed to the adapter's SCIM transaction scoping never engaging on
1.7.2, so the fix for that seam flips the pin deliberately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 15 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 32800235fc78c29dffbfc4bdfd601e0028eb72ca — the merge of head 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f into base fed4fa40924bd1366354ac9b8a55db9d94a7e404, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32800235fc78c29dffbfc4bdfd601e0028eb72ca && git checkout 32800235fc78c29dffbfc4bdfd601e0028eb72ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fed4fa40924bd1366354ac9b8a55db9d94a7e404 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f && git checkout -B drift-repro fed4fa40924bd1366354ac9b8a55db9d94a7e404 && git merge --no-ff 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f
node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fed4fa40924bd1366354ac9b8a55db9d94a7e404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-salesClaude

Copy link
Copy Markdown
Collaborator

Contract review round 1 at 51000d482: FAIL — patch round 2 in flight

The in-seat isolated Clause-② review (fable tier, transcript-verified 78/78) is posted verbatim on #14360. One blocking finding: a SCIM active: false over a row that already carries an expiring administrator ban leaves the expiry in place, so the vendor's session.create hook auto-unbans and admits the principal once the expiry passes while the identity provider still holds them deactivated. Fix (a pure tightening): on deactivation, make an existing expiring ban permanent (banExpires: null) without touching banned or the administrator's reason; one new suite face pins it.

Also taken up in the same round: the five new symbols move into a non-barrelled module so the package surface does not grow (changeset → patch; Clause-② stays yes for the sign-in accept-set change), plus one-sentence documentation of the last-LOCAL-credential guard exemption on the SCIM path, the marker-collision case, the post-refusal 403 behaviour (also noted on #14522), and a POST /Usersactive: false pin.

needs:contract-review stays on the PR and the card until a delta review passes on the patched head; the PR stays draft.


Generated by Claude Code

…nt; move the shared ban write off the barrel
Contract review round 1 (#14360): a SCIM active:false over an
administrator's TIMED ban left the expiry in place, and the vendor's
session hook auto-lifts an expired ban - so the principal was re-admitted
while the IdP still held them deactivated. The hook now clears banExpires
on that row (reason and banned untouched). The shared write moves to the
package-internal user-ban-write.ts (not re-exported from index.ts), so no
new public symbol ships and the changeset drops to patch. Two faces added:
the expiring-ban overlap and POST /Users with active:false.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@os-salesClaude

Copy link
Copy Markdown
Collaborator

Landing provenance — ready + auto-merge at head 21c7dbe76


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sales@claude
, '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

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360) - #14540

Merged
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user
Sep 2, 2026
Merged

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360)#14540
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#14360

What this does

Stable @better-auth/scim (1.7.0 and later; the repo pins and installs 1.7.2) no longer writes the admin plugin's banned column when an identity provider sends active: false — it hands the user's aggregate lifecycle state to an OPTIONAL host callback, identity.reconcileUser, and then only revokes sessions. plugin-auth passed no identity member, so a deactivation revoked sessions and wrote nothing: sys_user.banned stayed false and a user holding a local password signed straight back in, while ADR-0071, the generated docs and the #13816 construction-time refusal all asserted the ban.

Limb A of the triage ruling, and only limb A:

  • auth-manager.ts — the scim({...}) options gain identity.reconcileUser, routed to a new private method reconcileScimUserLifecycle that reads the user through the vendor's transaction-bound adapter and reconciles: active: false on a row that is not banned ⇒ the platform ban write (reason Deactivated via SCIM, the exact string the vendor wrote itself through 1.6.x; no expiry), and on a row already banned WITH an expiry (an administrator's timed ban) ⇒ the ban is made permanent (banExpires cleared, banned and the administrator's reason untouched — the vendor's session hook auto-lifts an expired ban and would otherwise admit a principal the IdP still holds deactivated); active: true on a row banned WITH that reason ⇒ unban; anything else is a no-op (the callback is contractually idempotent and the vendor calls it on EVERY user mutation).
  • user-ban-write.ts (new, PACKAGE-INTERNAL — not re-exported from src/index.ts) holds the existing /admin/ban-user / /admin/unban-user write as applyUserBan / applyUserUnban over a minimal UserBanWriter surface; admin-ban-endpoints.ts and the SCIM hook both call that ONE write (field for field the vendor's own banUser shape). No new HTTP endpoint, no new public symbol. The admin mount's behaviour is unchanged (its own suite: 88/88 across the six ban-adjacent files).
  • New end-to-end suite scim-deactivation-reconcile-user.test.ts driving @better-auth/scim itself through a real AuthManager over better-sqlite3 (the simulated deprovision() helper in last-admin-guard.test.ts is NOT extended, per triage).
  • One changeset for @objectstack/plugin-auth, level patch (see Contract review).

Admin forcing untouched (admin: pluginConfig.admin ?? scimEffective at three sites) — that is the #13816 shape-1 follow-up, not this card. The five stale assertion sites triage listed are untouched and are true again under this change.

Premise (re-verified on origin/main00ff228fe with the package installed)

  1. grep -ro "ban" node_modules/@better-auth/scim/dist | wc -l = 0; positive control over node_modules/better-auth/dist/plugins/admin = 117. dist/index.d.mts:267 declares reconcileUser?(input: SCIMIdentityState, context: SCIMTransactionContext); SCIMIdentityState carries userId, the aggregate active (true while ANY participating SCIM source is active), profileSourceId and sources; SCIMTransactionContext carries only database (the DBTransactionAdapter). A throw from the hook: runSCIMApplicationCallback rethrows a better-auth APIError unchanged and wraps anything else as a SCIM 500 with detail SCIM identity reconciliation failed and the original as cause — never swallowed.
  2. auth-manager.ts:3265 handed scim() exactly connections: [] and authentication.verifyBearerToken; repo-wide reconcileUser = 0 hits (control verifyBearerToken = 15).
  3. admin-ban-endpoints.ts owns the platform ban write and re-runs the last-LOCAL-credential guard; the last-ADMINISTRATOR guard (ADR-0024 D5.2, plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) is an ENGINE beforeUpdate hook in last-admin-guard.ts, registered by auth-plugin.ts at kernel:ready, so it holds on every banned write regardless of caller. Both writes were endpoint-shaped (they parsed a Request); the write itself is now a plain callable.
  4. The forced admin plugin supplies the banned column and the sign-in refusal: better-auth/dist/plugins/admin/admin.mjs:36-47 reads user?.banned in the session.create hook and throws FORBIDDEN / BANNED_USER.

PM hypotheses

One consequence stated rather than left implicit: on 1.7.2 a SCIM DELETE /Users/{id} tombstones the source instead of deleting the better-auth user, so the aggregate state turns inactive and the hook disables that account (face (f)); re-provisioning through the tombstone re-links the same user and the SCIM ban is lifted.

Tests (head 21c7dbe76)

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/scim-deactivation-reconcile-user.test.ts + the five ban-adjacent siblings (admin-ban-endpoints, admin-remove-user-gate-ordering, break-glass-guard-self-service-target, break-glass-local-credential, session-tombstone) — Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0.
  • The six SCIM-adjacent siblings (auth-manager, better-auth-schema-parity, credential-at-rest-posture, last-admin-guard, managed-extension-fields, scim-case-insensitive-identifier) — Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0.
  • tsc --noEmit (src program) exit 0; pnpm --filter @objectstack/plugin-auth check:test-typecheckOK — the test layer compiles under tsconfig.test.json (the new test file is in that program; the src program excludes tests by design). The tsconfig.examples.json half of the typecheck script resolves the package's own dist types, which this worktree has not built — NOT MEASURED locally; CI builds it.
  • Reverse verification on the committed tree (round-1 head 51000d482; the wiring line it removes is unchanged on 21c7dbe76, so the reading was not re-run) (scratchpad/issue-14360/ablation.sh, trap-restored): removed the three-line identity member from the scim() options — proven on disk (marker count 1 → 0, git diff --stat = 3 deletions) — and ran the suite: 6 red / 3 green, exactly (a), (b), the idempotency case, (c) [the request answers 200 instead of the guard's 403], the (c) positive control and (f) red; (d) attribute-only, (d) administrator-ban and (e) green. Restore via git checkout HEAD -- path: git diff HEAD empty, disk blob = HEAD blob e9d5c5eb…, marker count back to 1. No dist rebuild is involved: the suite imports ./auth-manager.js from src, and no workspace dependency was mutated.

Gates (head 21c7dbe76)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 36 commands from the changeset (re-derived on 21c7dbe76 after the changed-file set grew by user-ban-write.ts: identical list); each run at 21c7dbe76 with its exit captured before any pipe. 33 green, including check-system-context-census (OK — 109 elevation read sites … all anchored; 0 anchors on the two edited files, no census fix needed), check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:logger-receiver-detach, check-adr-0087-registration (non-breaking changeset), check-changeset-no-major. Three answered exit 3 = PREREQUISITE NOT MET by their own verdict text, recorded as NOT MEASURED locally (CI owns them): check-test-completeness (needs a saved turbo test log), check:dual-build-cjs-loads (needs pnpm build), check:type-check-debt (needs the full built closure). Control-character self-scan over the four changed files: clean.

Contract review (Clause ②)

  • Behaviour change, as accept/reject: a principal an identity provider has deactivated over SCIM (active: false, or deleted) is now REJECTED at sign-in with the vendor's 403 BANNED_USER (before: sessions revoked, sign-in ACCEPTED); an IdP active: true re-ACCEPTS a principal whose ban carries the SCIM reason (an administrator's ban stays a rejection). Deactivating the last administrator over SCIM is REJECTED with a 403 SCIM error (before: accepted, disabling nothing).
  • Exported-symbol delta (git diff -U0 00ff228fe...HEAD | grep -E '^[+-].*\bexport\b'): five export lines, ALL in the new package-internal module packages/plugins/plugin-auth/src/user-ban-write.ts (UserBanWriter, UserBanFields, SCIM_DEACTIVATION_BAN_REASON, applyUserBan, applyUserUnban). NO new export is reachable through src/index.ts: the barrel is untouched (84 lines before and after, zero references to user-ban-write), and the same grep over admin-ban-endpoints.ts, auth-manager.ts and index.ts prints nothing. No removals, no signature changes to existing exports. The scim() options gain the vendor's identity member (not an ObjectStack-authored config key).
  • Changeset level:patch — a bug fix restoring the declared behaviour with no new public symbol. Clause-② stays yes: sign-in accept/reject changes for a class of principals; the needs:contract-review carriers stay on the card and the PR.
  • Log levels: no new log site of any level; the last-admin refusal is an error RESPONSE through the vendor's SCIM error mapping.

Out of scope, filed

Patch round 2 (head 21c7dbe76) — the in-seat contract review's round-1 verdict, item by item

  • A (blocking finding 1 — implemented exactly as written).reconcileScimUserLifecycle reads banExpires too; in the !state.active branch a banned row with a non-null expiry gets { banExpires: null, updatedAt } and returns, otherwise returns — banned and the administrator's banReason untouched. Docblock first bullet and the changeset sentence say a deactivation also makes an existing expiring ban permanent. New face under (d): administrator ban with ban_expires 1.5s ahead, PATCH active: false → 200, expiry cleared and reason kept, then after the expiry has passed sign-in still answers 403 BANNED_USER (status AND code).
  • B (note 1, required). Shared write moved to the non-barrelled user-ban-write.ts; admin-ban-endpoints.ts, auth-manager.ts and the suite import it. Barrel export surface unchanged; changeset patch; the "New exports" paragraph removed.
  • C (notes 2, 3, 4, 6, 7). (2) changeset: the last-LOCAL-credential guard is deliberately not applied on the SCIM path — an IdP deprovision can disable the last password-holding account while non-administrator SSO users remain. (3) marker collision documented in the method docblock and the changeset (an administrator who types Deactivated via SCIM produces a ban the IdP can lift); the string is NOT reserved on the admin mount. (4) post-refusal behaviour stated above and on SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522. (6) new face (g): POST /Users with active: false provisions the account disabled — banned with the SCIM reason, sign-in 403 BANNED_USER. (7) SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522 carries the line-487 pin citation.
  • D (verification at 21c7dbe76, verify-lock sessions each under the cap, exits before any pipe). New suite + five ban-adjacent siblings: Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0. Six SCIM-adjacent siblings: Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0. tsc --noEmit exit 0 (0 errors); check:test-typecheckOK — the test layer compiles under tsconfig.test.json. Gates re-derived on 21c7dbe76 (identical 36) and all run there: 33 green, the same three prerequisite-not-met (check-test-completeness, check:dual-build-cjs-loads, check:type-check-debt). git merge-tree --write-tree --name-only origin/main HEAD after git fetch origin main (origin/main = fed4fa409): clean, no conflicted paths. GitHub mergeable_state read after the push: clean.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

… the platform ban write
SCIM active:false revoked sessions and wrote nothing on stable
@better-auth/scim (the vendor's ban write left the package in 1.7.0);
sys_user.banned was never set and a local-password user signed straight
back in. Wire identity.reconcileUser into the scim() options and route it
to the shared ban/unban write in admin-ban-endpoints.ts, inside the SCIM
transaction; the engine-level last-administrator guard judges it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…M transaction scope leaves (#14522)
The (c) face keeps the account enabled and the SCIM 403 shape; the vendor's
own scimUser.active write surviving the refusal is pinned as observed and
attributed to the adapter's SCIM transaction scoping never engaging on
1.7.2, so the fix for that seam flips the pin deliberately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 15 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 32800235fc78c29dffbfc4bdfd601e0028eb72ca — the merge of head 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f into base fed4fa40924bd1366354ac9b8a55db9d94a7e404, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32800235fc78c29dffbfc4bdfd601e0028eb72ca && git checkout 32800235fc78c29dffbfc4bdfd601e0028eb72ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fed4fa40924bd1366354ac9b8a55db9d94a7e404 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f && git checkout -B drift-repro fed4fa40924bd1366354ac9b8a55db9d94a7e404 && git merge --no-ff 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f
node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fed4fa40924bd1366354ac9b8a55db9d94a7e404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-salesClaude

Copy link
Copy Markdown
Collaborator

Contract review round 1 at 51000d482: FAIL — patch round 2 in flight

The in-seat isolated Clause-② review (fable tier, transcript-verified 78/78) is posted verbatim on #14360. One blocking finding: a SCIM active: false over a row that already carries an expiring administrator ban leaves the expiry in place, so the vendor's session.create hook auto-unbans and admits the principal once the expiry passes while the identity provider still holds them deactivated. Fix (a pure tightening): on deactivation, make an existing expiring ban permanent (banExpires: null) without touching banned or the administrator's reason; one new suite face pins it.

Also taken up in the same round: the five new symbols move into a non-barrelled module so the package surface does not grow (changeset → patch; Clause-② stays yes for the sign-in accept-set change), plus one-sentence documentation of the last-LOCAL-credential guard exemption on the SCIM path, the marker-collision case, the post-refusal 403 behaviour (also noted on #14522), and a POST /Usersactive: false pin.

needs:contract-review stays on the PR and the card until a delta review passes on the patched head; the PR stays draft.


Generated by Claude Code

…nt; move the shared ban write off the barrel
Contract review round 1 (#14360): a SCIM active:false over an
administrator's TIMED ban left the expiry in place, and the vendor's
session hook auto-lifts an expired ban - so the principal was re-admitted
while the IdP still held them deactivated. The hook now clears banExpires
on that row (reason and banned untouched). The shared write moves to the
package-internal user-ban-write.ts (not re-exported from index.ts), so no
new public symbol ships and the changeset drops to patch. Two faces added:
the expiring-ban overlap and POST /Users with active:false.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@os-salesClaude

Copy link
Copy Markdown
Collaborator

Landing provenance — ready + auto-merge at head 21c7dbe76


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sales@claude
, '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

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360) - #14540

Merged
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user
Sep 2, 2026
Merged

fix(plugin-auth): SCIM active:false disables the account again — route the vendor's reconcileUser hook to the platform ban write (#14360)#14540
os-sales merged 3 commits into
mainfrom
claude/issue-14360-scim-deactivation-reconcile-user

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#14360

What this does

Stable @better-auth/scim (1.7.0 and later; the repo pins and installs 1.7.2) no longer writes the admin plugin's banned column when an identity provider sends active: false — it hands the user's aggregate lifecycle state to an OPTIONAL host callback, identity.reconcileUser, and then only revokes sessions. plugin-auth passed no identity member, so a deactivation revoked sessions and wrote nothing: sys_user.banned stayed false and a user holding a local password signed straight back in, while ADR-0071, the generated docs and the #13816 construction-time refusal all asserted the ban.

Limb A of the triage ruling, and only limb A:

  • auth-manager.ts — the scim({...}) options gain identity.reconcileUser, routed to a new private method reconcileScimUserLifecycle that reads the user through the vendor's transaction-bound adapter and reconciles: active: false on a row that is not banned ⇒ the platform ban write (reason Deactivated via SCIM, the exact string the vendor wrote itself through 1.6.x; no expiry), and on a row already banned WITH an expiry (an administrator's timed ban) ⇒ the ban is made permanent (banExpires cleared, banned and the administrator's reason untouched — the vendor's session hook auto-lifts an expired ban and would otherwise admit a principal the IdP still holds deactivated); active: true on a row banned WITH that reason ⇒ unban; anything else is a no-op (the callback is contractually idempotent and the vendor calls it on EVERY user mutation).
  • user-ban-write.ts (new, PACKAGE-INTERNAL — not re-exported from src/index.ts) holds the existing /admin/ban-user / /admin/unban-user write as applyUserBan / applyUserUnban over a minimal UserBanWriter surface; admin-ban-endpoints.ts and the SCIM hook both call that ONE write (field for field the vendor's own banUser shape). No new HTTP endpoint, no new public symbol. The admin mount's behaviour is unchanged (its own suite: 88/88 across the six ban-adjacent files).
  • New end-to-end suite scim-deactivation-reconcile-user.test.ts driving @better-auth/scim itself through a real AuthManager over better-sqlite3 (the simulated deprovision() helper in last-admin-guard.test.ts is NOT extended, per triage).
  • One changeset for @objectstack/plugin-auth, level patch (see Contract review).

Admin forcing untouched (admin: pluginConfig.admin ?? scimEffective at three sites) — that is the #13816 shape-1 follow-up, not this card. The five stale assertion sites triage listed are untouched and are true again under this change.

Premise (re-verified on origin/main00ff228fe with the package installed)

  1. grep -ro "ban" node_modules/@better-auth/scim/dist | wc -l = 0; positive control over node_modules/better-auth/dist/plugins/admin = 117. dist/index.d.mts:267 declares reconcileUser?(input: SCIMIdentityState, context: SCIMTransactionContext); SCIMIdentityState carries userId, the aggregate active (true while ANY participating SCIM source is active), profileSourceId and sources; SCIMTransactionContext carries only database (the DBTransactionAdapter). A throw from the hook: runSCIMApplicationCallback rethrows a better-auth APIError unchanged and wraps anything else as a SCIM 500 with detail SCIM identity reconciliation failed and the original as cause — never swallowed.
  2. auth-manager.ts:3265 handed scim() exactly connections: [] and authentication.verifyBearerToken; repo-wide reconcileUser = 0 hits (control verifyBearerToken = 15).
  3. admin-ban-endpoints.ts owns the platform ban write and re-runs the last-LOCAL-credential guard; the last-ADMINISTRATOR guard (ADR-0024 D5.2, plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) is an ENGINE beforeUpdate hook in last-admin-guard.ts, registered by auth-plugin.ts at kernel:ready, so it holds on every banned write regardless of caller. Both writes were endpoint-shaped (they parsed a Request); the write itself is now a plain callable.
  4. The forced admin plugin supplies the banned column and the sign-in refusal: better-auth/dist/plugins/admin/admin.mjs:36-47 reads user?.banned in the session.create hook and throws FORBIDDEN / BANNED_USER.

PM hypotheses

One consequence stated rather than left implicit: on 1.7.2 a SCIM DELETE /Users/{id} tombstones the source instead of deleting the better-auth user, so the aggregate state turns inactive and the hook disables that account (face (f)); re-provisioning through the tombstone re-links the same user and the SCIM ban is lifted.

Tests (head 21c7dbe76)

  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/scim-deactivation-reconcile-user.test.ts + the five ban-adjacent siblings (admin-ban-endpoints, admin-remove-user-gate-ordering, break-glass-guard-self-service-target, break-glass-local-credential, session-tombstone) — Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0.
  • The six SCIM-adjacent siblings (auth-manager, better-auth-schema-parity, credential-at-rest-posture, last-admin-guard, managed-extension-fields, scim-case-insensitive-identifier) — Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0.
  • tsc --noEmit (src program) exit 0; pnpm --filter @objectstack/plugin-auth check:test-typecheckOK — the test layer compiles under tsconfig.test.json (the new test file is in that program; the src program excludes tests by design). The tsconfig.examples.json half of the typecheck script resolves the package's own dist types, which this worktree has not built — NOT MEASURED locally; CI builds it.
  • Reverse verification on the committed tree (round-1 head 51000d482; the wiring line it removes is unchanged on 21c7dbe76, so the reading was not re-run) (scratchpad/issue-14360/ablation.sh, trap-restored): removed the three-line identity member from the scim() options — proven on disk (marker count 1 → 0, git diff --stat = 3 deletions) — and ran the suite: 6 red / 3 green, exactly (a), (b), the idempotency case, (c) [the request answers 200 instead of the guard's 403], the (c) positive control and (f) red; (d) attribute-only, (d) administrator-ban and (e) green. Restore via git checkout HEAD -- path: git diff HEAD empty, disk blob = HEAD blob e9d5c5eb…, marker count back to 1. No dist rebuild is involved: the suite imports ./auth-manager.js from src, and no workspace dependency was mutated.

Gates (head 21c7dbe76)

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 36 commands from the changeset (re-derived on 21c7dbe76 after the changed-file set grew by user-ban-write.ts: identical list); each run at 21c7dbe76 with its exit captured before any pipe. 33 green, including check-system-context-census (OK — 109 elevation read sites … all anchored; 0 anchors on the two edited files, no census fix needed), check:cross-package-test-inputs, check:test-source-alias, check:engine-double-contract, check:logger-receiver-detach, check-adr-0087-registration (non-breaking changeset), check-changeset-no-major. Three answered exit 3 = PREREQUISITE NOT MET by their own verdict text, recorded as NOT MEASURED locally (CI owns them): check-test-completeness (needs a saved turbo test log), check:dual-build-cjs-loads (needs pnpm build), check:type-check-debt (needs the full built closure). Control-character self-scan over the four changed files: clean.

Contract review (Clause ②)

  • Behaviour change, as accept/reject: a principal an identity provider has deactivated over SCIM (active: false, or deleted) is now REJECTED at sign-in with the vendor's 403 BANNED_USER (before: sessions revoked, sign-in ACCEPTED); an IdP active: true re-ACCEPTS a principal whose ban carries the SCIM reason (an administrator's ban stays a rejection). Deactivating the last administrator over SCIM is REJECTED with a 403 SCIM error (before: accepted, disabling nothing).
  • Exported-symbol delta (git diff -U0 00ff228fe...HEAD | grep -E '^[+-].*\bexport\b'): five export lines, ALL in the new package-internal module packages/plugins/plugin-auth/src/user-ban-write.ts (UserBanWriter, UserBanFields, SCIM_DEACTIVATION_BAN_REASON, applyUserBan, applyUserUnban). NO new export is reachable through src/index.ts: the barrel is untouched (84 lines before and after, zero references to user-ban-write), and the same grep over admin-ban-endpoints.ts, auth-manager.ts and index.ts prints nothing. No removals, no signature changes to existing exports. The scim() options gain the vendor's identity member (not an ObjectStack-authored config key).
  • Changeset level:patch — a bug fix restoring the declared behaviour with no new public symbol. Clause-② stays yes: sign-in accept/reject changes for a class of principals; the needs:contract-review carriers stay on the card and the PR.
  • Log levels: no new log site of any level; the last-admin refusal is an error RESPONSE through the vendor's SCIM error mapping.

Out of scope, filed

Patch round 2 (head 21c7dbe76) — the in-seat contract review's round-1 verdict, item by item

  • A (blocking finding 1 — implemented exactly as written).reconcileScimUserLifecycle reads banExpires too; in the !state.active branch a banned row with a non-null expiry gets { banExpires: null, updatedAt } and returns, otherwise returns — banned and the administrator's banReason untouched. Docblock first bullet and the changeset sentence say a deactivation also makes an existing expiring ban permanent. New face under (d): administrator ban with ban_expires 1.5s ahead, PATCH active: false → 200, expiry cleared and reason kept, then after the expiry has passed sign-in still answers 403 BANNED_USER (status AND code).
  • B (note 1, required). Shared write moved to the non-barrelled user-ban-write.ts; admin-ban-endpoints.ts, auth-manager.ts and the suite import it. Barrel export surface unchanged; changeset patch; the "New exports" paragraph removed.
  • C (notes 2, 3, 4, 6, 7). (2) changeset: the last-LOCAL-credential guard is deliberately not applied on the SCIM path — an IdP deprovision can disable the last password-holding account while non-administrator SSO users remain. (3) marker collision documented in the method docblock and the changeset (an administrator who types Deactivated via SCIM produces a ban the IdP can lift); the string is NOT reserved on the admin mount. (4) post-refusal behaviour stated above and on SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522. (6) new face (g): POST /Users with active: false provisions the account disabled — banned with the SCIM reason, sign-in 403 BANNED_USER. (7) SCIM provisioning writes run outside any engine transaction on @better-auth/scim 1.7.2 — the #3653 scimRequestScope stamped in verifyBearerToken is not observed at write time (0 engine.transaction calls across POST + PATCH /Users) #14522 carries the line-487 pin citation.
  • D (verification at 21c7dbe76, verify-lock sessions each under the cap, exits before any pipe). New suite + five ban-adjacent siblings: Test Files 6 passed (6) · Tests 88 passed (88), VERDICT command-exit 0. Six SCIM-adjacent siblings: Test Files 6 passed (6) · Tests 469 passed (469), VERDICT command-exit 0. tsc --noEmit exit 0 (0 errors); check:test-typecheckOK — the test layer compiles under tsconfig.test.json. Gates re-derived on 21c7dbe76 (identical 36) and all run there: 33 green, the same three prerequisite-not-met (check-test-completeness, check:dual-build-cjs-loads, check:type-check-debt). git merge-tree --write-tree --name-only origin/main HEAD after git fetch origin main (origin/main = fed4fa409): clean, no conflicted paths. GitHub mergeable_state read after the push: clean.

Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

… the platform ban write
SCIM active:false revoked sessions and wrote nothing on stable
@better-auth/scim (the vendor's ban write left the package in 1.7.0);
sys_user.banned was never set and a local-password user signed straight
back in. Wire identity.reconcileUser into the scim() options and route it
to the shared ban/unban write in admin-ban-endpoints.ts, inside the SCIM
transaction; the engine-level last-administrator guard judges it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…M transaction scope leaves (#14522)
The (c) face keeps the account enabled and the SCIM 403 shape; the vendor's
own scimUser.active write surviving the refusal is pinned as observed and
attributed to the adapter's SCIM transaction scoping never engaging on
1.7.2, so the fix for that seam flips the pin deliberately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 15 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404packageMentionDocs.

Which tree this was computed on

This run read content/docs from 32800235fc78c29dffbfc4bdfd601e0028eb72ca — the merge of head 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f into base fed4fa40924bd1366354ac9b8a55db9d94a7e404, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32800235fc78c29dffbfc4bdfd601e0028eb72ca && git checkout 32800235fc78c29dffbfc4bdfd601e0028eb72ca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fed4fa40924bd1366354ac9b8a55db9d94a7e404 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f && git checkout -B drift-repro fed4fa40924bd1366354ac9b8a55db9d94a7e404 && git merge --no-ff 21c7dbe76b7d44df32fbd9f6497db6372b9f2d0f
node scripts/docs-audit/affected-docs.mjs --json fed4fa40924bd1366354ac9b8a55db9d94a7e404

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fed4fa40924bd1366354ac9b8a55db9d94a7e404 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-salesClaude

Copy link
Copy Markdown
Collaborator

Contract review round 1 at 51000d482: FAIL — patch round 2 in flight

The in-seat isolated Clause-② review (fable tier, transcript-verified 78/78) is posted verbatim on #14360. One blocking finding: a SCIM active: false over a row that already carries an expiring administrator ban leaves the expiry in place, so the vendor's session.create hook auto-unbans and admits the principal once the expiry passes while the identity provider still holds them deactivated. Fix (a pure tightening): on deactivation, make an existing expiring ban permanent (banExpires: null) without touching banned or the administrator's reason; one new suite face pins it.

Also taken up in the same round: the five new symbols move into a non-barrelled module so the package surface does not grow (changeset → patch; Clause-② stays yes for the sign-in accept-set change), plus one-sentence documentation of the last-LOCAL-credential guard exemption on the SCIM path, the marker-collision case, the post-refusal 403 behaviour (also noted on #14522), and a POST /Usersactive: false pin.

needs:contract-review stays on the PR and the card until a delta review passes on the patched head; the PR stays draft.


Generated by Claude Code

…nt; move the shared ban write off the barrel
Contract review round 1 (#14360): a SCIM active:false over an
administrator's TIMED ban left the expiry in place, and the vendor's
session hook auto-lifts an expired ban - so the principal was re-admitted
while the IdP still held them deactivated. The hook now clears banExpires
on that row (reason and banned untouched). The shared write moves to the
package-internal user-ban-write.ts (not re-exported from index.ts), so no
new public symbol ships and the changeset drops to patch. Two faces added:
the expiring-ban overlap and POST /Users with active:false.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@os-salesClaude

Copy link
Copy Markdown
Collaborator

Landing provenance — ready + auto-merge at head 21c7dbe76


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sales@claude