Skip to content

fix(plugin-auth): serve /admin/ban-user and /admin/unban-user with the ADR-0068 platform-admin gate - #9970

Merged
os-warren merged 2 commits into
mainfrom
claude/issue-9652-vendor-admin-predicate
Aug 19, 2026
Merged

fix(plugin-auth): serve /admin/ban-user and /admin/unban-user with the ADR-0068 platform-admin gate#9970
os-warren merged 2 commits into
mainfrom
claude/issue-9652-vendor-admin-predicate

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Part of #9652

⚠️ Deliberately Part of, not the closing keyword. #9652's title is
"every better-auth-native /admin/ route"; this PR repairs two of eleven and
splits the rest into #9968 / #9969 with the measurement attached. Merging must
not close the parent while its scope is still open. Switch this line to the
closing form only if the maintainer accepts the split as the final scope.

The first deliverable: the measured branch answer is Option 2

The 2026-08-18 ruling asked for a measurement before code: can better-auth's
admin plugin be pointed at ObjectStack's predicate (Option 1), or must the
routes be re-implemented as ObjectStack raw mounts (Option 2)?

Measured against the installed better-auth 1.7.1 (the family moved onto
stable in #9869 / 03520ebef, so the card's rc-era text was not used).

The vendor cannot express the predicate. Evidence, in the order it decides
the question:

  1. The whole option surface. AdminOptions declares exactly defaultRole,
    adminRoles, defaultBanReason, defaultBanExpiresIn,
    impersonationSessionDuration, schema, ac, roles, adminUserIds,
    bannedUserMessage, allowImpersonatingAdmins. There is no predicate
    hook, no roles-source callback, and no async resolver
    — the shapes the
    ruling asked me to look for do not exist.

  2. Authorization has exactly two inputs. Every gated route calls
    hasPermission({ userId, role: session.user.role, options: opts, permissions }),
    whose body short-circuits on options.adminUserIds?.includes(userId) and
    otherwise splits session.user.role on commas against options.roles.

  3. adminUserIds cannot carry a dynamic predicate. opts is built once at
    plugin construction (const opts = { ...options, ... }), so the array is
    frozen there, while ObjectStack's platform-admin set is a per-request read of
    sys_user_permission_set for a row pointing at admin_full_access with
    organization_id = null. Every way to make that array dynamic is worse than
    the bug: mutating a shared array leaves a demoted admin a stale pass
    (fail-open — turning a broken-capability defect into a security one), and
    overriding includes needs a synchronous answer to an asynchronous question.

  4. The session-scoped middle path is mechanically shut — this is the one the
    dispatch explicitly asked me to watch for, and it is the reason the answer is
    not "configure it". Every /admin/* route mounts adminMiddleware, which
    calls getAuthoritativeSessionFromCtx. On any deployment carrying a
    database — i.e. every ObjectStack deployment, since
    hasServerSessionStore(options) is !!options.database || !!options.secondaryStorage
    — that helper sets ctx.context.session = null and re-reads the session from
    the DB with disableCookieCache: true. Anything ObjectStack writes onto
    the in-memory session user is discarded before hasPermission sees it.

    customSession is not a second door either: it overrides the /get-session
    endpoint, not the session the admin routes resolve internally.

So the only input the vendor will accept is the persisted user.role scalar,
and producing that is Option 3 — permanently vetoed. Option 2 it is.

What this PR ships

/admin/ban-user and /admin/unban-user become ObjectStack raw mounts carrying
the ADR-0068 gate, joining create-user / set-user-password / unlock-user /
import-users / oauth2/toggle-disabled. These are the two routes that (a) a
sys_user action actually calls and (b) re-implement faithfully — the vendor
handlers are internalAdapter.updateUser plus deleteUserSessions, mirrored
field for field (banned / banReason / banExpires / updatedAt, default
reason 'No reason'). A banned user is still refused at sign-in by the vendor's
own untouched session hook (BANNED_USER).

Two supporting extractions, both of which close a class rather than an instance:

A 501 NOT_IMPLEMENTED guard mirrors create-user's: without it the mounts would
answer 200 on a deployment with no admin plugin, writing banned: true while
the vendor hook that enforces a ban is not loaded — a ban the console reports
as succeeding and the banned user signs straight through.

What this PR deliberately does not ship

Verification

Ablation, per the standing lane clause — the predicate wiring removed,
plugin-auth rebuilt, and the mutation proven to have reached dist
(ablation-dist-preflight ... --absent gives marker absent from all 12 built
files
) before any colour was read:

/auth/admin/ban-user     admin  -> 403 {"code":"YOU_ARE_NOT_ALLOWED_TO_BAN_USERS"}
/auth/admin/ban-user     member -> 403 {"code":"YOU_ARE_NOT_ALLOWED_TO_BAN_USERS"}
/auth/admin/unban-user   admin  -> 403 {"code":"YOU_ARE_NOT_ALLOWED_TO_BAN_USERS"}
/auth/admin/unban-user   member -> 403 {"code":"YOU_ARE_NOT_ALLOWED_TO_BAN_USERS"}

The platform admin and the plain member receive byte-identical refusals —
the defect, reproduced by construction at this branch point, which also
re-confirms the card's repro set on 1.7.1. The pin goes red in that state
(1 failed / 6 passed) and the control holds: the universal-invariant test — no
anonymous caller and no plain member gets a 2xx from any of the 31 derived
/admin/ routes — stayed green through the ablated leg.

The restore leg was rebuilt too, and the marker proven present in dist
(2 built files) before re-reading: 7 passed (7). The source file was restored
byte-identically, verified by git hash-object
(9a502aae96527ac9d87cf2a88efe2214287905e7 before and after), not by a matching
diffstat.

Gates and suites, all at final head d9a09995:

check result
pnpm --filter @objectstack/plugin-auth typecheck pass (tsc --noEmit, no output)
pnpm --filter @objectstack/plugin-auth test 1283 passed (1283), 56 files
dogfood admin-route-nonadmin-refusal 7 passed (7)
check:slot-lookup · check:test-source-alias · check:type-source-resolution · check-affected-docs pass (the four named at dispatch)
check:route-envelope · check:changeset-gate-self-tests · check:objectui-changeset · check:engine-double-contract · check:where-matcher · check:query-options-erasure · check:nul-bytes pass
spec check:liveness · check:empty-state · check:strictness-ledger · check:variant-docs pass
check-adr-0087-registration · check-changeset-no-major · check-empty-changeset pass

The set beyond the four named at dispatch came from re-deriving against the
actual diff with node scripts/pm/dispatch-gates.mjs (no hand-built path list).
check:route-envelope was the notable addition — it is triggered by
auth-plugin.ts specifically and the dispatch list did not name it.

One declared narrowing: check:type-check-debt --re-measure was not run
locally — it requires the whole workspace closure built and CI runs it on every
PR regardless.

The refactor's oracle is the pre-existing pins on the five already-gated routes:
the objectstack-gate bucket asserts anonymous 401 UNAUTHENTICATED, member
403 PERMISSION_DENIED and admin-not-refused on each of its routes, and it is
green across all 7 (5 pre-existing + 2 new) after the gate was collapsed into
one judge — zero behaviour change on the five.

Rejection cases assert code and status per ADR-0112, and the two
envelopes are kept apart on purpose: ObjectStack's {success,error:{code}} where
these mounts answer, better-auth's flat {message,code} where the vendor still
does.

Notes


Generated by Claude Code

claude added 2 commits August 19, 2026 09:53
…e ADR-0068 platform-admin gate

better-auth's admin plugin authorizes on the legacy `user.role === 'admin'`
scalar that ADR-0068 D2 stopped synthesizing, and its option surface at the
installed 1.7.1 cannot be pointed at ObjectStack's predicate. Mount both routes
as ObjectStack raw routes ahead of the catch-all, carrying the platform-admin
gate — the create-user / set-user-password pattern.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
… routes

Moves /admin/ban-user and /admin/unban-user from the better-auth-gate bucket
(refusal side only) into objectstack-gate, which asserts the full contrast:
anon 401 UNAUTHENTICATED, member 403 PERMISSION_DENIED, platform admin NOT
refused. Adds the changeset.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

33 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 6aceca97143088c9c33e568c5dd49e5b7cfdf347.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-auth/src/index.ts) — pages documenting those are invisible to this run
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

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 6aceca97143088c9c33e568c5dd49e5b7cfdf347packageMentionDocs.

Which tree this was computed on

This run read content/docs from f4ef3fd964be59afa01fbb263170f5d31625ebae — the merge of head d9a099951478b643d754a998856924dd6642105c into base 6aceca97143088c9c33e568c5dd49e5b7cfdf347, 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 f4ef3fd964be59afa01fbb263170f5d31625ebae && git checkout f4ef3fd964be59afa01fbb263170f5d31625ebae
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6aceca97143088c9c33e568c5dd49e5b7cfdf347 d9a099951478b643d754a998856924dd6642105c && git checkout -B drift-repro 6aceca97143088c9c33e568c5dd49e5b7cfdf347 && git merge --no-ff d9a099951478b643d754a998856924dd6642105c

node scripts/docs-audit/affected-docs.mjs --json 6aceca97143088c9c33e568c5dd49e5b7cfdf347

⚠️ 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 6aceca97143088c9c33e568c5dd49e5b7cfdf347 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants