From 9e41285ee295b710f0adee52dc6b896379e9200d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 05:19:41 +0000 Subject: [PATCH 1/3] fix(auth): authorize before the break-glass guard on /admin/remove-user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The break-glass last-local-credential guard is a global better-auth `hooks.before`, which runs ahead of an endpoint's own middleware. On /admin/remove-user — served directly by better-auth's router, whose adminMiddleware establishes only a session — the guard's lookup and its target-dependent refusal were reached by any authenticated caller before either authorization layer had run. /admin/ban-user already ran the same guard AFTER authorization, because #9652 shades that path with an ObjectStack raw mount whose platform-admin gate fires first. One guard, two routes, opposite orders, nothing asserting either. /admin/remove-user now carries the same shading, converging the /admin/* family on authorization before the guard. The mount reuses the landed #9652 / #9653 gate-then-delegate pattern; no new mechanism. It DELEGATES rather than re-implementing removal, so the request re-enters better-auth's router: the path-keyed hook still fires and the guard still decides, just after authorization. A platform admin is unaffected in every respect, including the vendor's own admission decision (#9969). An ordering pin ships with the fix: one authenticated non-admin naming two different targets must receive indistinguishable responses, and — so the pin cannot be satisfied by deleting the guard — an admitted platform admin must still hit the guard's refusal and still succeed on an ordinary user. Part of the /admin/* ordering convergence; dogfood bucket for the route reclassified to `shaded-vendor-gate` (ObjectStack gate answers the refusal, better-auth still owns admission). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .changeset/admin-remove-user-gate-ordering.md | 40 +++ .../admin-remove-user-gate-ordering.test.ts | 321 ++++++++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 13 + .../plugins/plugin-auth/src/auth-plugin.ts | 67 ++++ ...in-platform-admin-standing.dogfood.test.ts | 2 +- ...min-route-nonadmin-refusal.dogfood.test.ts | 109 +++++- 6 files changed, 547 insertions(+), 5 deletions(-) create mode 100644 .changeset/admin-remove-user-gate-ordering.md create mode 100644 packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts diff --git a/.changeset/admin-remove-user-gate-ordering.md b/.changeset/admin-remove-user-gate-ordering.md new file mode 100644 index 0000000000..0c31ac97f9 --- /dev/null +++ b/.changeset/admin-remove-user-gate-ordering.md @@ -0,0 +1,40 @@ +--- +"@objectstack/plugin-auth": patch +--- + +fix(auth): authorize before the break-glass guard on `POST /api/v1/auth/admin/remove-user` (#11477) + +The break-glass last-local-credential guard is registered as a global better-auth +`hooks.before`, which runs ahead of an endpoint's own middleware. On +`/admin/remove-user` — served directly by better-auth's router, whose +`adminMiddleware` establishes only a session, with the role decision landing +later inside the vendor's handler — that ordering let the guard's lookup and its +distinctive refusal be reached by any **authenticated** caller before either +authorization layer had run. Because that refusal is target-dependent, the +refusal itself carried a per-record fact about a user the caller was not +entitled to ask about. + +`/admin/ban-user` already ran the same guard **after** authorization: #9652 +shades that path with an ObjectStack raw mount whose platform-admin gate fires +first. One guard, two routes, opposite orders, and nothing asserting either. + +`/admin/remove-user` now carries the same shading, converging the whole +`/admin/*` family on **authorization before the guard**. The mount reuses the +landed #9652 / #9653 pattern and introduces no new mechanism. + +What changes is **when** the guard decides, never **what** it decides: + +- an anonymous caller still gets `401 UNAUTHENTICATED`; +- an authenticated non-admin now gets `403 PERMISSION_DENIED` for every target, + so the guard is unreachable before authorization and its answer no longer + varies with the named user; +- a platform admin is unaffected in every respect — the mount **delegates** into + better-auth rather than re-implementing removal, so the path-keyed hook still + fires and the guard still refuses the removal of the last local password + login, and admission remains the vendor's own decision (#9969). + +An ordering pin ships with the fix so the sequence is mechanically checkable +rather than re-argued: it asserts that one authenticated non-admin naming two +different targets receives **indistinguishable** responses, and — so the pin +cannot be satisfied by deleting the guard — that an admitted platform admin +still hits the guard's refusal, and still succeeds on an ordinary user. diff --git a/packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts b/packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts new file mode 100644 index 0000000000..7b831a9fef --- /dev/null +++ b/packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts @@ -0,0 +1,321 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #11477 — ORDERING PIN: on `/admin/remove-user` the break-glass guard must run +// AFTER authorization, and the whole `/admin/*` family must agree on that order. +// +// ── The defect this pins, and why a pin is half the fix ───────────────────── +// +// The break-glass last-local-credential guard is a global `hooks.before` keyed +// on `ctx.path` (auth-manager.ts). A better-auth `before` hook runs ahead of the +// endpoint's own `use: [adminMiddleware]`, and on this route that middleware +// only establishes a SESSION — the role decision happens later still, inside +// the vendor's handler. So the guard's lookup, and its distinctive refusal, +// were reachable by any AUTHENTICATED caller before either authorization layer +// had run. +// +// The same guard on `/admin/ban-user` ran AFTER authorization, because #9652 +// shades that path with a raw mount whose `gateAdmin` fires first. One guard, +// two routes, OPPOSITE orders, and nothing asserting either. That is why the +// maintainer's 2026-08-25 ruling (option A, verbatim 「全部同意」) shipped an +// ordering pin alongside the shading: the defect exists precisely because two +// routes drifted apart with no test able to notice. +// +// ── Why the assertion is INDISTINGUISHABILITY, not "it is refused" ────────── +// +// ⛔ Asserting merely that a non-admin is refused would be satisfied by the +// DEFECTIVE build: the defective build refuses too — it just refuses the +// break-glass holder with a different status and a different code than it +// refuses everyone else, and that difference IS the leak. The finding is about +// what a non-admin can TELL APART, so the load-bearing assertion is that one +// authenticated non-admin, naming two different targets, gets responses that +// are byte-identical. +// +// ── Why this file drives the REAL seam ────────────────────────────────────── +// +// Raw-mount-vs-vendor-router ordering does not exist inside `AuthManager`: the +// mounts live on the plugin's Hono app, and `AuthManager.handleRequest` is what +// they delegate INTO. A test that drove `handleRequest` directly (as +// `break-glass-guard-authentication-order.test.ts` correctly does for the hook's +// own predicate) would bypass every mount and be structurally blind to this +// defect. So the fixture here mounts `AuthPlugin.registerAuthRoutes` on a real +// Hono app in front of a real `AuthManager` on the installed better-auth, and +// every assertion reads a status and a body off a real `Response`. +// +// ── The load-bearing half ─────────────────────────────────────────────────── +// +// ⛔ An implementation that "fixed" the ordering by DELETING the guard would +// satisfy every indistinguishability assertion here. Two further describe +// blocks make that impossible: the guard must still refuse an admitted platform +// admin who really is removing the last local credential (409), and the same +// admin must still succeed on an ordinary user (200) — so the still-refused leg +// cannot be satisfied by refusing everyone. +// +// ADR-0112 is `code` AND `status`; every refusal assertion carries both. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { Hono } from 'hono'; +import { AuthManager } from './auth-manager'; +import { AuthPlugin } from './auth-plugin'; +import { createMemoryEngine } from './impersonation-bearer-rotation.test'; +import { LAST_LOCAL_CREDENTIAL_CODE } from './last-local-credential'; +import type { PluginContext } from '@objectstack/core'; + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-11477'; +const ORIGIN = 'http://localhost:3000'; +const BASE = '/api/v1/auth'; + +const mockCtx = (): PluginContext => + ({ + registerService: vi.fn(), + getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)), + getServices: vi.fn(() => new Map()), + hook: vi.fn(), + trigger: vi.fn(), + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + }) as any; + +/** Status + whatever error code the body carries, in either envelope shape. */ +async function verdict(res: Response): Promise<{ status: number; code?: string; text: string }> { + const text = await res.text(); + let code: string | undefined; + try { + const parsed = JSON.parse(text); + // ObjectStack's ADR-0112 envelope nests it; better-auth's flat shape does not. + code = parsed?.error?.code ?? parsed?.code; + } catch { + /* non-JSON body → no code */ + } + return { status: res.status, code, text }; +} + +/** + * One deployment, staged to the exact posture the guard exists to protect, and + * served through the REAL mount chain. + * + * - `owner` holds the ONLY local-password (`credential`) account — the + * break-glass escape hatch itself, and the per-record fact the defect leaked. + * - `admin` is a platform admin. The legacy `role` scalar is set because the + * VENDOR's own `adminMiddleware` still authorizes on it (#9969 keeps this + * route on the vendor gate); `isPlatformAdminUser` accepts it as its + * documented back-compat signal, so one fixture drives both layers. + * - `member` is an ordinary authenticated user holding no credential — the + * caller the finding is about. + * - `ordinary` is a second credential-less user: the CONTRAST target, so + * "same caller, two targets" is a real comparison. + */ +async function stage() { + const engine = createMemoryEngine(); + const manager = new AuthManager({ + secret: SECRET, + baseUrl: ORIGIN, + dataEngine: engine, + plugins: { admin: true }, + } as any); + + const direct = (path: string, body: unknown) => + manager.handleRequest( + new Request(`${ORIGIN}${BASE}${path}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }), + ); + + for (const [email, name] of [ + ['owner.11477@example.com', 'Break Glass Owner'], + ['admin.11477@example.com', 'Managed Admin'], + ['member.11477@example.com', 'Plain Member'], + ['ordinary.11477@example.com', 'Ordinary User'], + ]) { + const res = await direct('/sign-up/email', { email, password: PASSWORD, name }); + expect(res.status, `sign-up ${email}: ${await res.clone().text()}`).toBe(200); + } + + const users = (engine.tables.get('sys_user') ?? []) as any[]; + const idFor = (email: string) => String(users.find((r) => r.email === email)!.id); + const ownerId = idFor('owner.11477@example.com'); + const adminId = idFor('admin.11477@example.com'); + const ordinaryId = idFor('ordinary.11477@example.com'); + + users.find((r) => String(r.id) === adminId)!.role = 'admin'; + + const bearerFor = async (email: string) => { + const res = await direct('/sign-in/email', { email, password: PASSWORD }); + const token = res.headers.get('set-auth-token'); + expect(token, `sign-in ${email} must mint a bearer or the authenticated legs prove nothing`).toBeTruthy(); + return token!; + }; + const adminBearer = await bearerFor('admin.11477@example.com'); + const memberBearer = await bearerFor('member.11477@example.com'); + + // Leave exactly ONE local-credential holder: `owner`. This is the state the + // guard guards, and the state whose disclosure the ordering controlled. + const accounts = (engine.tables.get('sys_account') ?? []) as any[]; + engine.tables.set( + 'sys_account', + accounts.filter((r) => !(r.provider_id === 'credential' && String(r.user_id ?? '') !== ownerId)), + ); + expect( + ((engine.tables.get('sys_account') ?? []) as any[]) + .filter((r) => r.provider_id === 'credential') + .map((r) => String(r.user_id)), + 'fixture invariant: `owner` must be the SOLE local-credential holder', + ).toEqual([ownerId]); + + // The REAL route registration — raw mounts ahead of the catch-all — on a real + // Hono app in front of the real AuthManager. Without this the mounts under + // test are not in the request path at all. + const app = new Hono(); + const ctx = mockCtx(); + const plugin = new AuthPlugin({ secret: SECRET }); + await plugin.init(ctx); + (plugin as any).authManager = manager; + (plugin as any).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx); + + const fire = (path: string, body: unknown, bearer?: string) => + app.request(`${ORIGIN}${BASE}${path}`, { + method: 'POST', + headers: { + 'content-type': 'application/json', + origin: ORIGIN, + ...(bearer ? { authorization: `Bearer ${bearer}` } : {}), + }, + body: JSON.stringify(body), + }); + + return { engine, fire, ownerId, ordinaryId, adminBearer, memberBearer }; +} + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => vi.restoreAllMocks()); + +// ─────────────────────────────────────────────────────────────────────────── +// THE ORDERING PIN +// ─────────────────────────────────────────────────────────────────────────── + +describe('#11477 — /admin/remove-user: the guard runs AFTER authorization', () => { + it('an authenticated NON-ADMIN cannot tell the break-glass holder from anyone else', async () => { + // ⛔ THE assertion of this file. On the defective (unshaded) build this + // fails: naming the holder answered 409 LAST_LOCAL_CREDENTIAL while naming + // an ordinary user answered 403 YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS — + // measured, both on the same caller. + const { fire, ownerId, ordinaryId, memberBearer } = await stage(); + + const holder = await verdict(await fire('/admin/remove-user', { userId: ownerId }, memberBearer)); + const other = await verdict(await fire('/admin/remove-user', { userId: ordinaryId }, memberBearer)); + + expect(holder.status, `holder: ${holder.text} | other: ${other.text}`).toBe(other.status); + expect(holder.text, 'the two answers must be byte-identical').toBe(other.text); + }, 120_000); + + it('...and what it hears is the AUTHORIZATION verdict, never the guard‘s', async () => { + // Indistinguishability alone would also be satisfied by a build that + // answered the guard's 409 for BOTH targets — indistinguishable, and + // strictly worse. This names which of the two layers spoke. + const { fire, ownerId, ordinaryId, memberBearer } = await stage(); + + for (const [label, userId] of [ + ['break-glass holder', ownerId], + ['ordinary user', ordinaryId], + ] as const) { + const v = await verdict(await fire('/admin/remove-user', { userId }, memberBearer)); + expect(v.status, `${label}: ${v.text}`).toBe(403); + expect(v.code, `${label}: ${v.text}`).toBe('PERMISSION_DENIED'); + expect(v.code, `${label}: the guard‘s code must not reach a non-admin`).not.toBe( + LAST_LOCAL_CREDENTIAL_CODE, + ); + expect(v.status, `${label}: the guard‘s status must not reach a non-admin`).not.toBe(409); + } + }, 120_000); + + it('an ANONYMOUS caller is unchanged: 401 UNAUTHENTICATED, and equally indistinguishable', async () => { + // #10776 already closed the anonymous half. Pinned here so this card's + // shading cannot regress it — a mount is a new first responder for the + // path, and the anonymous answer is the one it is easiest to change by + // accident. + const { fire, ownerId, ordinaryId } = await stage(); + + const holder = await verdict(await fire('/admin/remove-user', { userId: ownerId })); + const other = await verdict(await fire('/admin/remove-user', { userId: ordinaryId })); + + expect(holder.status, holder.text).toBe(401); + expect(holder.code, holder.text).toBe('UNAUTHENTICATED'); + expect(holder.status).toBe(other.status); + expect(holder.text).toBe(other.text); + }, 120_000); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// THE LOAD-BEARING HALF — the guard still exists, and still decides +// ─────────────────────────────────────────────────────────────────────────── + +describe('#11477 — the break-glass invariant survives the reordering', () => { + it('still-refused: an ADMITTED platform admin removing the last local credential is still 409', async () => { + // ⛔ The leg that fails on an implementation that "fixed" the disclosure by + // deleting the guard, or by shadowing the path with a mount that forgot to + // re-enter better-auth's router (which would DETACH the path-keyed hook — + // the trap recorded in last-local-credential.ts). Everything in the block + // above stays green there. + // + // It is also the positive statement of the ordering itself: the guard's + // answer is reachable ONLY past authorization, and past authorization it is + // unchanged. + const { fire, ownerId, adminBearer } = await stage(); + + const v = await verdict(await fire('/admin/remove-user', { userId: ownerId }, adminBearer)); + + expect(v.status, v.text).toBe(409); + expect(v.code, v.text).toBe(LAST_LOCAL_CREDENTIAL_CODE); + }, 120_000); + + it('admission: the same admin removing an ordinary user still succeeds', async () => { + // Without this, the still-refused leg is satisfiable by refusing every + // caller — the failure mode this lane has already paid for twice. + const { fire, ordinaryId, adminBearer } = await stage(); + + const v = await verdict(await fire('/admin/remove-user', { userId: ordinaryId }, adminBearer)); + + expect(v.status, v.text).toBe(200); + expect(v.code, v.text).not.toBe(LAST_LOCAL_CREDENTIAL_CODE); + }, 120_000); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// THE COUPLING CONTROL — #9652's mounts must not move +// ─────────────────────────────────────────────────────────────────────────── + +describe('#11477 — /admin/ban-user‘s sequence is UNCHANGED (the #9652 coupling)', () => { + // The card and the ruling both record that this card and #9652's raw mounts + // change each other's ordering BIDIRECTIONALLY. `/admin/ban-user` is the + // route that already had the order this card is converging on, so it is the + // control: if shading `remove-user` moved `ban-user`, the convergence would + // have been bought by breaking the reference implementation. + it('a non-admin still gets the same authorization verdict for both targets', async () => { + const { fire, ownerId, ordinaryId, memberBearer } = await stage(); + + const holder = await verdict(await fire('/admin/ban-user', { userId: ownerId }, memberBearer)); + const other = await verdict(await fire('/admin/ban-user', { userId: ordinaryId }, memberBearer)); + + expect(holder.status, holder.text).toBe(403); + expect(holder.code, holder.text).toBe('PERMISSION_DENIED'); + expect(holder.status).toBe(other.status); + expect(holder.text).toBe(other.text); + }, 120_000); + + it('and the guard still refuses an admin banning the last local credential', async () => { + // `ban-user` re-runs the guard by hand from the shared module (its mount + // does NOT delegate). This pins that the hand-rolled call site is still + // wired, which is the half of #9652 most easily lost. + const { fire, ownerId, adminBearer } = await stage(); + + const v = await verdict(await fire('/admin/ban-user', { userId: ownerId }, adminBearer)); + + expect(v.status, v.text).toBe(409); + expect(v.code, v.text).toBe(LAST_LOCAL_CREDENTIAL_CODE); + }, 120_000); +}); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index bc1825a4f5..43cea24f14 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -1655,6 +1655,19 @@ export class AuthManager { // the mount is conditional on the admin plugin, and because the // guard itself now lives in ONE module both call sites share — // `last-local-credential.ts`, whose header records this trap. + // + // ⚠️ `/admin/remove-user` IS ALSO SHADED NOW (#11477) — and it DOES + // still reach this hook, which is the opposite of the line above + // and is the point. Its mount only runs `gateAdmin` and then + // RE-DISPATCHES the request through `handleRequest`, so it re-enters + // better-auth's router and this hook fires exactly as before — + // just AFTER authorization instead of before it. That is the whole + // fix: an authenticated non-admin is now refused by the mount and + // never reaches the lookup below, while an admitted platform admin + // reaches the identical lookup and the identical `CONFLICT`. + // ⛔ Do not "reconcile" the two notes by deleting this path from + // the list — that would silently drop the guard on the one route + // that still depends on this hook to run it. // ── [#10776] AUTHENTICATE FIRST ──────────────────────────── // A `hooks.before` runs AHEAD of the endpoint's own // `use: [adminMiddleware]`, and that middleware is the only layer diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 3829572b6a..b64db15ba6 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -2138,6 +2138,73 @@ export class AuthPlugin implements Plugin { } }); + // ── #11477: /admin/remove-user — AUTHORIZATION BEFORE THE GUARD ────── + // + // The break-glass last-local-credential guard is a global + // `hooks.before` in auth-manager.ts keyed on `ctx.path`. A better-auth + // `before` hook runs ahead of the endpoint's own `use: [adminMiddleware]` + // — and on this route that middleware is only a SESSION check, with the + // role decision landing later still, inside the vendor's handler. So on + // the unshaded route the guard's lookup and its distinctive refusal were + // reached by any AUTHENTICATED caller, admin or not, before either + // authorization layer had run. + // + // MEASURED on the installed better-auth 1.7.1 before this mount existed, + // one authenticated non-admin, two targets: naming the break-glass + // holder answered `409 LAST_LOCAL_CREDENTIAL` while naming an ordinary + // user answered `403 YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS`. Two different + // answers to the same caller IS the finding — the refusal itself carried + // a per-record fact about a user the caller was never entitled to ask + // about. The same measurement on `/admin/ban-user` answered + // `403 PERMISSION_DENIED` for BOTH targets, because #9652 already shades + // that path and `gateAdmin` runs first there. + // + // Maintainer ruling 2026-08-25 (decision-inbox batch 5, accepted + // verbatim 「全部同意」): option A — give this route the same raw-mount + // shading `/admin/ban-user` has, converging the whole `/admin/*` family + // on authorization before the guard. No new mechanism. + // + // ⚠️ This mount DELEGATES; it does not re-implement. That is the whole + // difference from the ban/unban mounts above, and it is deliberate: + // + // • #9969 (closed `not_planned`) ruled that the consumer-less vendor + // routes — this one included — are NOT re-implemented; their 403 to + // a platform admin is a recorded, intended state. Re-implementing + // removal here would quietly overturn that ruling. + // • Delegating keeps the request inside better-auth's router, so the + // path-keyed `hooks.before` still fires and the guard KEEPS working. + // Shadowing normally DETACHES such hooks (the trap written up in + // last-local-credential.ts, which is why ban-user must re-run the + // guard by hand); re-dispatching through `handleRequest` is what + // avoids paying that cost twice. The `/admin/sso/*` bridges use this + // exact gate-then-delegate shape (#9653). + // + // ⇒ WHAT CHANGES is only WHEN the guard decides, never WHAT it decides: + // anonymous → 401 UNAUTHENTICATED (unchanged) + // authenticated member → 403 PERMISSION_DENIED for EVERY target — the + // guard is now unreachable before authorization + // platform admin → unchanged in every respect, including the + // vendor's own 403 (#9969) and the guard's 409 + // when the target really is the last holder + // + // The vendor Response is returned VERBATIM so the delegated answer — + // status, body and the #10349 ADR-0112 envelope `handleRequest` applies + // to vendor `/admin/` refusals — is byte-identical to the unshaded route. + // + // Pinned by `admin-remove-user-gate-ordering.test.ts`, which fails if + // this mount is removed. + rawApp.post(`${basePath}/admin/remove-user`, async (c: any) => { + try { + const gated = await gateAdmin(c); + if (gated instanceof Response) return gated; + return await this.authManager!.handleRequest(c.req.raw); + } catch (error) { + const err = error instanceof Error ? error : new Error(String(error)); + ctx.logger.error('[AuthPlugin] admin/remove-user failed', err); + return c.json({ success: false, error: { code: 'INTERNAL_ERROR', message: err.message } }, 500); + } + }); + rawApp.post(`${basePath}/admin/set-user-password`, async (c: any) => { try { const actor = await gateAdmin(c); diff --git a/packages/qa/dogfood/test/admin-platform-admin-standing.dogfood.test.ts b/packages/qa/dogfood/test/admin-platform-admin-standing.dogfood.test.ts index b71de29fd6..7c0b845ff4 100644 --- a/packages/qa/dogfood/test/admin-platform-admin-standing.dogfood.test.ts +++ b/packages/qa/dogfood/test/admin-platform-admin-standing.dogfood.test.ts @@ -151,7 +151,7 @@ function refusedByDesignFor(targetUserId: string): Record { code: 'YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS', body: { userId: targetUserId }, ruledBy: '#9969 (closed not_planned)', - why: 'no ObjectStack consumer; re-implement on demand. It also still carries better-auth\'s break-glass last-local-credential before-hook precisely BECAUSE it is not shadowed by a raw mount', + why: 'no ObjectStack consumer; re-implement on demand — so ADMISSION is still better-auth\'s adminMiddleware on the legacy `role` scalar and this refusal is intended. ⚠️ #11477 DID shade the path with a raw mount (gateAdmin first, to stop the break-glass before-hook answering an authenticated non-admin ahead of authorization), but that mount DELEGATES through handleRequest instead of re-implementing removal — so the request re-enters better-auth\'s router, the break-glass before-hook still fires, and this vendor refusal is unchanged. A future mount that stopped delegating would break BOTH this row and the guard', }, 'POST /api/v1/auth/admin/revoke-user-session': { code: 'YOU_ARE_NOT_ALLOWED_TO_REVOKE_USERS_SESSIONS', diff --git a/packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts b/packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts index fde50ec010..90d1bc1a05 100644 --- a/packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts +++ b/packages/qa/dogfood/test/admin-route-nonadmin-refusal.dogfood.test.ts @@ -75,7 +75,25 @@ // bridges while SSO is off). That is what proves the member's 403 is a // gate verdict and not a payload the server rejects for everyone. // -// `better-auth-gate` (9 routes) — refusal side only, DELIBERATELY. The +// `shaded-vendor-gate` (1 route: `remove-user`) — the two halves belong to +// DIFFERENT layers, which is why it is neither of its neighbours. #11477 +// gave the route the raw-mount shading `ban-user` already had, so an +// ObjectStack gate answers the refusal (member 403 PERMISSION_DENIED, anon +// 401 UNAUTHENTICATED) — but the mount DELEGATES rather than +// re-implementing, so admission is still better-auth's `adminMiddleware` on +// the legacy `role` scalar and a platform admin is still refused +// `403 YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS` (#9969, closed `not_planned`: +// consumer-less vendor routes are not re-implemented). +// +// The both-sides contrast is therefore not a 2xx but a DIFFERENCE: the +// member and the admin hear two different refusals, which is what proves +// the member's 403 is an authorization verdict and not a blanket refusal. +// The bucket also carries the #11477 negative — a member must never again +// see the break-glass guard's `409 LAST_LOCAL_CREDENTIAL`, which before the +// shading was answered ahead of every authorization layer and VARIED WITH +// THE TARGET, disclosing per-record state to a caller entitled to none. +// +// `better-auth-gate` (8 routes) — refusal side only, DELIBERATELY. The // ANONYMOUS half is now the full ADR-0112 pin, identical to the bucket // above — `401` and `code: 'UNAUTHENTICATED'`. It was `[401, 403].includes( // status)` until #10349, because the vendor's `adminMiddleware` answered an @@ -113,8 +131,9 @@ // a session-scoped synthesis is discarded before the check runs. It moved // the two routes `sys_user` actions call — `ban-user` / `unban-user` — // onto ObjectStack mounts, where the allowed side IS pinned above. The -// nine below still answer the platform admin with the vendor's own -// `YOU_ARE_NOT_ALLOWED_*`; that is a known, filed gap, not drift. +// eight below — and `remove-user`, one bucket up — still answer the +// platform admin with the vendor's own `YOU_ARE_NOT_ALLOWED_*`; that is a +// known, filed state (#9969, closed `not_planned`), not drift. // // `self-scoped` (2 routes) — `has-permission` and `stop-impersonating` answer // a non-admin without a refusal BY DESIGN, and the invariant is asserted in @@ -164,6 +183,7 @@ const AUTH_BASE = '/api/v1/auth'; /** How a non-admin must be answered by one derived route. */ type Bucket = | 'objectstack-gate' + | 'shaded-vendor-gate' | 'better-auth-gate' | 'self-scoped' | 'not-mounted'; @@ -285,6 +305,36 @@ function expectationsFor(targetUserId: string): Record body: { userId: targetUserId }, }, + // ── #11477 — shaded for ORDERING, still admitted by the vendor ───────── + // + // The only member of its bucket, and the bucket exists because this route + // genuinely has a third shape rather than because the other two did not + // fit. Its raw mount runs `gateAdmin` and then RE-DISPATCHES into + // better-auth instead of re-implementing removal, so the two halves are + // owned by different layers: + // + // refusal → ObjectStack's gate (403 PERMISSION_DENIED), because the + // mount answers first. That is #11477's whole point: the + // break-glass `hooks.before` guard used to answer an + // authenticated non-admin BEFORE any authorization ran, and + // its 409 differed per target — a per-record disclosure. + // admission → still better-auth's `adminMiddleware` on the legacy `role` + // scalar, so a platform admin is still refused + // 403 YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS. #9969 closed + // `not_planned`: consumer-less vendor routes are not + // re-implemented, and that refusal is intended. + // + // ⛔ Do not "simplify" this into either neighbouring bucket. In + // `objectstack-gate` the admin-is-not-refused assertion would be red (the + // vendor still refuses); in `better-auth-gate` the member's code assertion + // would be red (`PERMISSION_DENIED` is not `YOU_ARE_NOT_ALLOWED_*`). + // Forcing either one would mean weakening a live security assertion. + 'POST /api/v1/auth/admin/remove-user': { + bucket: 'shaded-vendor-gate', + body: { userId: targetUserId }, + note: '#11477 — ObjectStack gate answers the refusal, better-auth still owns admission (#9969)', + }, + // ── better-auth admin plugin (legacy `role` scalar gate) ──────────────── // // Still refusal-side only, and still for the reason in the header: the @@ -293,7 +343,6 @@ function expectationsFor(targetUserId: string): Record // re-implementable); the rest stay on the vendor's gate pending the // maintainer's call on the remaining surface. 'POST /api/v1/auth/admin/set-role': { bucket: 'better-auth-gate', body: { userId: targetUserId, role: 'admin' } }, - 'POST /api/v1/auth/admin/remove-user': { bucket: 'better-auth-gate', body: { userId: targetUserId } }, 'POST /api/v1/auth/admin/impersonate-user': { bucket: 'better-auth-gate', body: { userId: targetUserId } }, 'POST /api/v1/auth/admin/revoke-user-sessions': { bucket: 'better-auth-gate', body: { userId: targetUserId } }, 'POST /api/v1/auth/admin/revoke-user-session': { bucket: 'better-auth-gate', body: { sessionToken: 'probe-session-token' } }, @@ -553,6 +602,50 @@ describe('#9482 C9: every derived /admin/ route refuses a non-admin', () => { } }, 600_000); + it('the shaded vendor route refuses a non-admin from the ObjectStack gate, before the break-glass guard', async () => { + // #11477. The both-sides contrast here is NOT a 2xx — it is that the two + // callers hear DIFFERENT refusals. A member is turned away by ObjectStack's + // gate (`PERMISSION_DENIED`) and a platform admin gets past it only to be + // turned away by the vendor's (`YOU_ARE_NOT_ALLOWED_*`, #9969). Two + // distinct codes on the same route and payload is what proves the member's + // 403 is an authorization verdict rather than a blanket refusal — the same + // job the 2xx does in the `objectstack-gate` bucket above. + const routes = derived.all.filter((r) => expectations[r]?.bucket === 'shaded-vendor-gate'); + expect(routes.length, 'no shaded-vendor-gate routes were derived').toBeGreaterThan(0); + + for (const route of routes) { + const anon = await fire(route, undefined); + expect(anon.status, `${route} anonymous: ${anon.body}`).toBe(401); + expect(anon.code, `${route} anonymous code: ${anon.body}`).toBe('UNAUTHENTICATED'); + + const member = await fire(route, memberToken); + expect(member.status, `${route} member: ${member.body}`).toBe(403); + expect(member.code, `${route} member code: ${member.body}`).toBe('PERMISSION_DENIED'); + + // ⛔ The load-bearing negative. Before #11477 the break-glass + // `hooks.before` guard answered an authenticated non-admin ahead of every + // authorization layer, and its answer varied with the TARGET — a + // per-record disclosure to a caller entitled to nothing. A member must + // never see the guard's verdict on this route again. + expect(member.code, `${route} member must not reach the break-glass guard`).not.toBe( + 'LAST_LOCAL_CREDENTIAL', + ); + expect(member.status, `${route} member must not reach the break-glass guard`).not.toBe(409); + + const admin = await fire(route, adminToken); + expect( + admin.code, + `${route} platform admin: the vendor gate still owns admission (#9969), ` + + `so this must be the vendor's own code, got ${admin.status} ${admin.body}`, + ).toMatch(/^YOU_ARE_NOT_ALLOWED/); + expect( + admin.code, + `${route} platform admin was refused by the OBJECTSTACK gate — the member's ` + + `403 above therefore proves nothing about authorization`, + ).not.toBe('PERMISSION_DENIED'); + } + }, 600_000); + it('the better-auth admin routes refuse a non-admin with a named vendor code', async () => { const routes = derived.all.filter((r) => expectations[r]?.bucket === 'better-auth-gate'); expect(routes.length, 'no better-auth-gate routes were derived').toBeGreaterThan(0); @@ -596,6 +689,14 @@ describe('#9482 C9: every derived /admin/ route refuses a non-admin', () => { // transaction, so this route answers the authorization question like // every other member of the bucket and needs no exception. // + // ⚠️ #11477 moved `remove-user` OUT of this bucket entirely — its raw + // mount now answers a member from ObjectStack's gate + // (`403 PERMISSION_DENIED`) before better-auth is reached at all, so the + // vendor-vocabulary rule below no longer describes it. It lives in + // `shaded-vendor-gate`, which asserts that code exactly. This is the + // reverse of re-widening: the vocabulary here stayed narrow and the route + // that stopped matching it was reclassified. + // // ⛔ Do not re-widen the vocabulary — for this route or for all of them. // A route that answers `UNAUTHENTICATED` to a signed-in caller is // announcing that authentication ran where authorization should have, and From b4c02724527e240174c990094f69fd87e4d0bbb6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 09:42:21 +0000 Subject: [PATCH 2/3] test(verify): expect the gate's verdict for a plain member on /admin/remove-user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #10792 pin at erasure-transaction-authorization.test.ts asserted the vendor's YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS for a signed-in plain member — a literal from the pre-#11477 surface, where better-auth's router served the route unshaded. Ruled option A on #11477 shades /admin/remove-user with the #9652 pattern (gateAdmin before the break-glass guard), so through the real mount chain the member now hears the ObjectStack gate's target-independent 403 PERMISSION_DENIED. The pin's intent is unchanged and its other legs are untouched: the member still hears an authorization refusal (403, never 401), the answer still arrives fast on the pool max=1 dialect, and a refusal still erases nothing. Only the code literal moves to the converged verdict. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01UQgPSniH1GFM9ZDeGyuGUa --- .../src/erasure-transaction-authorization.test.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/verify/src/erasure-transaction-authorization.test.ts b/packages/verify/src/erasure-transaction-authorization.test.ts index af52f4e924..f44740ce0f 100644 --- a/packages/verify/src/erasure-transaction-authorization.test.ts +++ b/packages/verify/src/erasure-transaction-authorization.test.ts @@ -160,9 +160,16 @@ describe('#10792 — the erasure route answers authorization on a pool max=1 dia it('a signed-in plain member gets the AUTHORIZATION refusal, not 401', async () => { const answer = await fire('POST', '/auth/admin/remove-user', { userId: targets[2] }, memberToken); expect(answer.status, `member remove-user: ${answer.status} ${answer.body}`).toBe(403); - // The vendor's own denial vocabulary. Asserting only the status would let a - // 403 from some unrelated gate stand in for the authorization answer. - expect(answer.code).toBe('YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS'); + // #11477 (maintainer-ruled option A): the route is now shaded by an + // ObjectStack raw mount whose gateAdmin runs BEFORE the break-glass guard, + // so the refusal a plain member hears is the gate's target-independent + // PERMISSION_DENIED — no longer the vendor's + // YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS, which the pre-#11477 route only + // reached after the guard had already answered. This pin's intent is + // unchanged: the member hears an AUTHORIZATION verdict, and asserting the + // code (not just the status) keeps a 403 from some unrelated layer from + // standing in for it. + expect(answer.code).toBe('PERMISSION_DENIED'); expect(answer.ms).toBeLessThan(NOT_BLOCKED_MS); // …and the member's target survives: a refusal must erase nothing. const survivors = await ql.find('sys_user', { where: { id: targets[2] }, limit: 1 }, { context: SYS }); From a2caffa63e9990fa42dd0e8456e00e906ceb6d2e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 11:01:37 +0000 Subject: [PATCH 3/3] test(auth): stage the #11477 ordering-pin fixture through the invitation carve-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Main moved under the branch: #11767 made the platform's default audience posture invite_only, so the pin's bare-AuthManager fixture had every self-serve sign-up past the first refused with 403 SELF_REGISTRATION_CLOSED during staging — all 7 pin tests died before reaching their assertions. The fixture now seeds a pending invitation per staged user via inviteForAudienceGate, the house lane the neighbouring green fixtures already use (an `open` posture would force email verification on and stop sign-up from minting the bearers the authenticated legs need). No assertion changed; the pin still asserts exactly what it asserted. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01UQgPSniH1GFM9ZDeGyuGUa --- .../plugin-auth/src/admin-remove-user-gate-ordering.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts b/packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts index 7b831a9fef..279f02a889 100644 --- a/packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts +++ b/packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts @@ -57,6 +57,7 @@ import { Hono } from 'hono'; import { AuthManager } from './auth-manager'; import { AuthPlugin } from './auth-plugin'; import { createMemoryEngine } from './impersonation-bearer-rotation.test'; +import { inviteForAudienceGate } from './audience-gate-test-support'; import { LAST_LOCAL_CREDENTIAL_CODE } from './last-local-credential'; import type { PluginContext } from '@objectstack/core'; @@ -129,6 +130,11 @@ async function stage() { ['member.11477@example.com', 'Plain Member'], ['ordinary.11477@example.com', 'Ordinary User'], ]) { + // [#11767] the default audience posture is now invite_only, so fixture + // users beyond the first enter through the invitation carve-out — the + // house lane (see audience-gate-test-support; `open` would force email + // verification on and stop sign-up from minting the bearers below). + await inviteForAudienceGate(manager, email); const res = await direct('/sign-up/email', { email, password: PASSWORD, name }); expect(res.status, `sign-up ${email}: ${await res.clone().text()}`).toBe(200); }