diff --git a/.changeset/admin-vendor-refusal-envelope.md b/.changeset/admin-vendor-refusal-envelope.md new file mode 100644 index 0000000000..a7788a5489 --- /dev/null +++ b/.changeset/admin-vendor-refusal-envelope.md @@ -0,0 +1,58 @@ +--- +"@objectstack/plugin-auth": minor +--- + +fix(plugin-auth): the better-auth-native `/admin/` routes refuse an anonymous caller with the ADR-0112 envelope (#10349) + +**BREAKING** response-shape change on the `/api/v1/auth/admin/` namespace, +shipped as `minor` under the repo's launch-window convention for breaking +changes. + +`/api/v1/auth/admin/` is served by two implementations and answered the same +question in two shapes. ObjectStack's raw mounts (`create-user`, +`set-user-password`, `unlock-user`, `import-users`, `ban-user`, `unban-user`, +`oauth2/toggle-disabled`, `sso/*`) refuse an anonymous caller through +`judgePlatformAdmin` with the declared envelope and `code: 'UNAUTHENTICATED'`. +The routes better-auth serves itself refuse through the vendor's +`adminMiddleware` — `getAuthoritativeSessionFromCtx(ctx)` then +`APIError.fromStatus('UNAUTHORIZED')`, with no body argument at all. + +Measured on the installed better-auth 1.7.1, anonymous, through +`AuthManager.handleRequest`: ten vendor-lane routes (`impersonate-user`, +`set-role`, `revoke-user-sessions`, `revoke-user-session`, +`list-user-sessions`, `update-user`, `list-users`, `get-user`, +`has-permission`, `stop-impersonating`) answered `401` with a +`content-type: application/json` header and the **empty string** as the body. +A client that believes that header and parses the body throws on the refusal +instead of branching on it, and a client that wants to branch has to know, per +route, which of the two implementations happens to serve it — an +implementation detail, not a contract. + +`AuthManager.handleRequest` now gives those refusals the declared envelope at +the one seam every vendor route passes through. **Statuses are unchanged and +admission is unchanged**: nothing that was refused is now admitted, nothing +that was admitted is now refused, and no status moved. What is added is the +machine-readable `code`, derived from the status by ADR-0112's own +`standardErrorCodeForHttpStatus` map rather than spelled out again — so no new +error code is registered and the vendor lane's anonymous refusal is now +byte-identical to the ObjectStack lane's. + +Scope is the `/admin/` namespace only. Three narrowings hold the rest of the +surface still, and each is pinned: + +- **A refusal that already carried a body keeps it, byte for byte.** The + signed-in non-admin's `403` with the vendor's own + `YOU_ARE_NOT_ALLOWED_TO_*` vocabulary is untouched; this change fills in an + empty body and never rewrites a spoken one. +- **Only the two refusal statuses are named** (`401`, `403`). A bodyless `404` + such as `/admin/oauth2/*` with the `oidcProvider` plugin off, and any + semantic `4xx` the vendor owns, are left exactly as they are. +- **Nothing outside `/admin/` is touched.** `POST /sign-in/email` still answers + `401 {"message":"Invalid email or password","code":"INVALID_EMAIL_OR_PASSWORD"}`, + measured identical on both sides of the change. + +Consumers that branch on the HTTP status are unaffected. Consumers that already +parse the ObjectStack `/admin/*` envelope now get the same shape everywhere in +the namespace, with no per-route knowledge required. + + diff --git a/packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts b/packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts index efde462447..fa021a6b55 100644 --- a/packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts +++ b/packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts @@ -243,15 +243,23 @@ describe('the other direction — a non-entitled caller is still refused', () => // "the gate said no". expect(res.status).toBe(401); - // ⚠️ Measured, and deliberately not dressed up: this refusal carries an - // EMPTY body — no ADR-0112 envelope, no `code`. It comes from the vendor's - // `adminMiddleware` (`APIError.fromStatus('UNAUTHORIZED')`), which runs - // before this endpoint's handler and is byte-identical on stock - // better-auth 1.7.1 for both `/admin/impersonate-user` and - // `/admin/set-role`. This card changes the AUTHORIZATION predicate, not the - // authentication middleware, so the shape is pinned as it is rather than - // asserted to be something it is not. - expect(await res.text()).toBe(''); + // ⚠️ This assertion USED to read `expect(await res.text()).toBe('')` — the + // vendor's `adminMiddleware` (`APIError.fromStatus('UNAUTHORIZED')`, no body + // argument) refused an anonymous caller with the EMPTY STRING under a + // `content-type: application/json` header, on every better-auth-native + // `/admin/` route. #9968 changed the AUTHORIZATION predicate only and pinned + // that shape as it was rather than dressing it up. + // + // #10349 closed it at the ONE seam every vendor route passes through + // (`AuthManager.handleRequest` → `vendor-admin-refusal-envelope.ts`), so the + // anonymous refusal now carries the ADR-0112 envelope — `code` AND `status` + // — and is byte-identical to what the ObjectStack raw `/admin/*` mounts + // answer. Authorization here is still untouched: the 403 assertions above + // and the 200 below are unchanged. + expect(JSON.parse(await res.text())).toEqual({ + success: false, + error: { code: 'UNAUTHENTICATED', message: 'Sign in first' }, + }); }); it('an org owner/admin who is NOT a platform admin is refused', async () => { diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 65dd241024..1044073db0 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -30,6 +30,7 @@ import { type AuthEventAuditSurface, } from './auth-session-audit.js'; import { SESSION_ERASURE_PATHS } from './session-tombstone.js'; +import { envelopeVendorAdminRefusal } from './vendor-admin-refusal-envelope.js'; import { ADMIN_SESSION_COOKIE_KEY, STOP_IMPERSONATING_PATH, @@ -3680,11 +3681,31 @@ export class AuthManager { // is left with an identity that still occupies the org roster and can no // longer sign in. Nothing tells the operator, and there is no way back. const endpointPath = this.betterAuthEndpointPath(request); - const response = + const vendorResponse = endpointPath !== undefined && SESSION_ERASURE_PATHS.has(endpointPath) ? await this.runSubjectErasureAtomically(runHandler) : await runHandler(); + // [#10349] The better-auth-native `/admin/` routes refuse an anonymous + // caller through the vendor's `adminMiddleware` + // (`APIError.fromStatus('UNAUTHORIZED')`, no body argument), so the refusal + // reaches the client as a 401 that announces `application/json` and carries + // the EMPTY STRING — no envelope, nothing to branch on. The ObjectStack raw + // `/admin/*` mounts answer the identical question with the ADR-0112 + // envelope and `code: 'UNAUTHENTICATED'` (`platform-admin-gate.ts`), and + // which of the two a caller gets depends only on which implementation + // happens to serve that route — an implementation detail, not a contract. + // + // This is the ONE seam every vendor route passes through, which is why the + // normalization belongs here and not in ten routes we do not own. It is + // scoped to the `/admin/` NAMESPACE (option C): the prefix test costs no new + // concept, because this method already discriminates on `endpointPath` twice + // above — `STOP_IMPERSONATING_PATH` and `SESSION_ERASURE_PATHS`. + // + // Status and admission are untouched; see the module header for the three + // narrowings and the measurement behind each. + const response = await envelopeVendorAdminRefusal(endpointPath, vendorResponse); + if (response.status >= 500) { try { const body = await response.clone().text(); diff --git a/packages/plugins/plugin-auth/src/platform-admin-gate.ts b/packages/plugins/plugin-auth/src/platform-admin-gate.ts index 1fcb18a1c2..635e882dfd 100644 --- a/packages/plugins/plugin-auth/src/platform-admin-gate.ts +++ b/packages/plugins/plugin-auth/src/platform-admin-gate.ts @@ -40,6 +40,21 @@ export type PlatformAdminVerdict = | { ok: true; actor: PlatformAdminActor } | { ok: false; refusal: PlatformAdminRefusal }; +/** + * The human half of the two refusals, keyed by the status that carries them. + * + * Lifted out of `judgePlatformAdmin` (whose bytes are unchanged) so the + * better-auth-native `/admin/` lane can answer an anonymous caller with the + * SAME body rather than a second string that merely looks the same today — + * see `vendor-admin-refusal-envelope.ts` (#10349). The machine half is not + * duplicated anywhere: it is ADR-0112's own derived-code map, + * `standardErrorCodeForHttpStatus`. + */ +export const PLATFORM_ADMIN_REFUSAL_MESSAGES: Readonly> = { + 401: 'Sign in first', + 403: 'Admin role required', +}; + /** * Is this session user a platform admin under ADR-0068 D2? * @@ -82,7 +97,10 @@ export function judgePlatformAdmin(session: unknown): PlatformAdminVerdict { ok: false, refusal: { status: 401, - body: { success: false, error: { code: 'UNAUTHENTICATED', message: 'Sign in first' } }, + body: { + success: false, + error: { code: 'UNAUTHENTICATED', message: PLATFORM_ADMIN_REFUSAL_MESSAGES[401] }, + }, }, }; } @@ -92,7 +110,10 @@ export function judgePlatformAdmin(session: unknown): PlatformAdminVerdict { ok: false, refusal: { status: 403, - body: { success: false, error: { code: 'PERMISSION_DENIED', message: 'Admin role required' } }, + body: { + success: false, + error: { code: 'PERMISSION_DENIED', message: PLATFORM_ADMIN_REFUSAL_MESSAGES[403] }, + }, }, }; } diff --git a/packages/plugins/plugin-auth/src/vendor-admin-refusal-envelope.test.ts b/packages/plugins/plugin-auth/src/vendor-admin-refusal-envelope.test.ts new file mode 100644 index 0000000000..2ec3c5449a --- /dev/null +++ b/packages/plugins/plugin-auth/src/vendor-admin-refusal-envelope.test.ts @@ -0,0 +1,268 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #10349 — the better-auth-native `/admin/` lane answers an anonymous caller +// with a bodyless 401. This file pins the fix in BOTH directions and at BOTH +// levels: +// +// • the pure normalizer, where the three narrowings are cheap to enumerate +// (empty body only · 401/403 only · `/admin/` only), and +// • the real `AuthManager.handleRequest` seam on the installed better-auth +// 1.7.1, where the vendor's `adminMiddleware` actually produces the refusal. +// +// ⛔ A refusal-only suite is not enough here and the lane has paid for that +// twice: an implementation that refuses EVERYONE passes every refusal +// assertion. So the ADMISSION direction is asserted on the same route with the +// same seam — a platform admin still gets 200 out of `/admin/impersonate-user` +// — and a non-`/admin/` refusal is asserted to come back byte-identical. +// +// ADR-0112 is `code` AND `status`; every refusal assertion below carries both. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { standardErrorCodeForHttpStatus } from '@objectstack/spec/api'; +import { ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; +import { + envelopeVendorAdminRefusal, + isVendorAdminPath, + VENDOR_ADMIN_PATH_PREFIX, +} from './vendor-admin-refusal-envelope'; +import { judgePlatformAdmin } from './platform-admin-gate'; +import { AuthManager } from './auth-manager'; +import { createMemoryEngine } from './impersonation-bearer-rotation.test'; + +// ─────────────────────────────────────────────────────────────────────────── +// The pure normalizer +// ─────────────────────────────────────────────────────────────────────────── + +/** The vendor's shape: a status, `application/json`, and the EMPTY STRING. */ +const vendorBodylessRefusal = (status: number): Response => + new Response('', { status, headers: { 'content-type': 'application/json' } }); + +const bodyOf = async (res: Response) => JSON.parse(await res.text()); + +describe('#10349 — the normalizer fills in a bodyless vendor /admin/ refusal', () => { + it('a bodyless 401 becomes the ADR-0112 envelope, status untouched', async () => { + const out = await envelopeVendorAdminRefusal('/admin/set-role', vendorBodylessRefusal(401)); + + expect(out.status).toBe(401); + expect(await bodyOf(out)).toEqual({ + success: false, + error: { code: 'UNAUTHENTICATED', message: 'Sign in first' }, + }); + }); + + it('a bodyless 403 becomes the envelope too — one rule over both refusal statuses', async () => { + const out = await envelopeVendorAdminRefusal('/admin/set-role', vendorBodylessRefusal(403)); + + expect(out.status).toBe(403); + expect(await bodyOf(out)).toEqual({ + success: false, + error: { code: 'PERMISSION_DENIED', message: 'Admin role required' }, + }); + }); + + it('the code is DERIVED from ADR-0112‘s own map, not written down here', async () => { + // If the derived-code map is ever re-pointed, this file must not be the + // place that keeps the old spelling alive. + for (const status of [401, 403] as const) { + const out = await envelopeVendorAdminRefusal('/admin/x', vendorBodylessRefusal(status)); + expect((await bodyOf(out)).error.code).toBe(standardErrorCodeForHttpStatus(status)); + } + }); + + it('the vendor lane and the ObjectStack lane now answer anonymous BYTE-IDENTICALLY', async () => { + // The asymmetry is the substance of this card, so its disappearance is + // asserted directly rather than inferred from two separate assertions that + // merely happen to agree today. + const objectStackLane = judgePlatformAdmin(null); + expect(objectStackLane.ok).toBe(false); + const raw = objectStackLane.ok ? undefined : objectStackLane.refusal; + + const vendorLane = await envelopeVendorAdminRefusal( + '/admin/impersonate-user', + vendorBodylessRefusal(401), + ); + + expect(vendorLane.status).toBe(raw!.status); + expect(await bodyOf(vendorLane)).toEqual(raw!.body); + }); + + // ── The three narrowings, each asserted by IDENTITY ────────────────────── + // + // `toBe(input)` — the same object, not an equal one. An implementation that + // rebuilt an "equivalent" response on these paths would still be changing + // headers and streams on surfaces this card does not own. + + it('a vendor refusal that DID say something is returned unchanged', async () => { + // The signed-in non-admin's real 403 on this stack. + const spoken = new Response( + JSON.stringify({ message: 'x', code: 'YOU_ARE_NOT_ALLOWED_TO_IMPERSONATE_USERS' }), + { status: 403, headers: { 'content-type': 'application/json' } }, + ); + + expect(await envelopeVendorAdminRefusal('/admin/impersonate-user', spoken)).toBe(spoken); + }); + + it('a non-refusal status is returned unchanged, even bodyless under /admin/', async () => { + // Measured: `/admin/oauth2/resources` is a bodyless 404 when oidcProvider + // is off, and `/admin/remove-user` can answer a semantic 409. Neither is + // this seam's to name. + for (const status of [200, 400, 404, 409, 500]) { + const untouched = vendorBodylessRefusal(status); + expect(await envelopeVendorAdminRefusal('/admin/anything', untouched)).toBe(untouched); + } + }); + + it('a non-/admin/ path is returned unchanged — this is option C, not option B', async () => { + for (const path of ['/sign-in/email', '/get-session', '/organization/add-member', '/admin', undefined]) { + const untouched = vendorBodylessRefusal(401); + expect(await envelopeVendorAdminRefusal(path, untouched)).toBe(untouched); + } + }); + + it('the namespace prefix is a namespace, not a route name', () => { + expect(VENDOR_ADMIN_PATH_PREFIX).toBe('/admin/'); + expect(isVendorAdminPath('/admin/set-role')).toBe(true); + expect(isVendorAdminPath('/admin/sso/register')).toBe(true); + expect(isVendorAdminPath('/admin')).toBe(false); + expect(isVendorAdminPath('/administrator/x')).toBe(false); + expect(isVendorAdminPath(undefined)).toBe(false); + }); + + it('headers the vendor attached survive, and content-length does not lie', async () => { + const withCookie = new Response('', { + status: 401, + headers: { 'content-type': 'application/json', 'content-length': '0', 'set-cookie': 'a=b' }, + }); + + const out = await envelopeVendorAdminRefusal('/admin/set-role', withCookie); + + expect(out.headers.get('set-cookie')).toBe('a=b'); + expect(out.headers.get('content-type')).toBe('application/json'); + const text = await out.text(); + const declared = out.headers.get('content-length'); + expect(declared === null || Number(declared) === new TextEncoder().encode(text).length).toBe(true); + }); +}); + +// ─────────────────────────────────────────────────────────────────────────── +// The real seam, on the installed better-auth 1.7.1 +// ─────────────────────────────────────────────────────────────────────────── + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-10349'; +const BASE = 'http://localhost:3000/api/v1/auth'; +const PS_ADMIN = 'ps_admin_full_access'; + +const makeManager = (engine: any) => + new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + dataEngine: engine, + plugins: { admin: true }, + } as any); + +const post = (manager: AuthManager, path: string, body: unknown, bearer?: string) => + manager.handleRequest( + new Request(`${BASE}${path}`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + ...(bearer ? { authorization: `Bearer ${bearer}` } : {}), + }, + body: JSON.stringify(body), + }), + ); + +/** + * Anonymous probes across the vendor `/admin/` lane. Derived by hand rather + * than from the route table on purpose: this is a package-level test with no + * booted stack, and the DERIVED sweep over the live route table is the dogfood + * suite's job (`admin-route-nonadmin-refusal.dogfood.test.ts`). + */ +const VENDOR_ADMIN_PROBES: Array<[string, unknown]> = [ + ['/admin/impersonate-user', { userId: 'usr_probe' }], + ['/admin/set-role', { userId: 'usr_probe', role: 'admin' }], + ['/admin/revoke-user-sessions', { userId: 'usr_probe' }], + ['/admin/list-user-sessions', { userId: 'usr_probe' }], + ['/admin/update-user', { userId: 'usr_probe', data: { name: 'X' } }], +]; + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => vi.restoreAllMocks()); + +describe('#10349 — through AuthManager.handleRequest on the real vendor pipeline', () => { + it('every vendor /admin/ route refuses an anonymous caller 401 UNAUTHENTICATED', async () => { + const manager = makeManager(createMemoryEngine()); + + for (const [path, body] of VENDOR_ADMIN_PROBES) { + const res = await post(manager, path, body); + const text = await res.text(); + + // status AND code — a lone status cannot tell the vendor's authentication + // refusal from a validation error, and was green through the defect. + expect(res.status, `${path}: ${text}`).toBe(401); + expect(JSON.parse(text), `${path}`).toEqual({ + success: false, + error: { code: 'UNAUTHENTICATED', message: 'Sign in first' }, + }); + } + }, 60_000); + + it('a non-/admin/ vendor refusal keeps the vendor‘s own body — option C holds at the seam', async () => { + const manager = makeManager(createMemoryEngine()); + + const res = await post(manager, '/sign-in/email', { + email: 'nobody@example.com', + password: 'wrong-password-entirely', + }); + + expect(res.status).toBe(401); + // Measured before AND after the change: the vendor's flat `{message,code}`. + // If option C ever silently became option B, this is what would move. + expect(await bodyOf(res)).toEqual({ + message: 'Invalid email or password', + code: 'INVALID_EMAIL_OR_PASSWORD', + }); + }, 60_000); + + it('ADMISSION is unchanged — a platform admin still gets 200 from /admin/impersonate-user', async () => { + // ⛔ The load-bearing half. Everything above stays green on an + // implementation that refuses every caller; only this fails there. + const engine = createMemoryEngine(); + const manager = makeManager(engine); + + for (const [email, name] of [ + ['padmin.10349@example.com', 'Platform Admin'], + ['target.10349@example.com', 'Target'], + ]) { + await post(manager, '/sign-up/email', { email, password: PASSWORD, name }); + } + const rows = (engine.tables.get('sys_user') ?? []) as any[]; + const idFor = (email: string) => String(rows.find((r) => r.email === email)!.id); + const adminId = idFor('padmin.10349@example.com'); + const targetId = idFor('target.10349@example.com'); + + // ADR-0068 D2 grant — deliberately NOT the legacy `role` scalar. + await engine.insert('sys_permission_set', { id: PS_ADMIN, name: ADMIN_FULL_ACCESS }); + await engine.insert('sys_user_permission_set', { + user_id: adminId, + permission_set_id: PS_ADMIN, + organization_id: null, + }); + + const signIn = await post(manager, '/sign-in/email', { + email: 'padmin.10349@example.com', + password: PASSWORD, + }); + const bearer = signIn.headers.get('set-auth-token'); + expect(bearer, 'sign-in must mint a bearer or this test proves nothing').toBeTruthy(); + + const res = await post(manager, '/admin/impersonate-user', { userId: targetId }, bearer!); + + expect(res.status, await res.clone().text()).toBe(200); + expect((await bodyOf(res))?.user?.id).toBe(targetId); + }, 60_000); +}); diff --git a/packages/plugins/plugin-auth/src/vendor-admin-refusal-envelope.ts b/packages/plugins/plugin-auth/src/vendor-admin-refusal-envelope.ts new file mode 100644 index 0000000000..06ef895bf8 --- /dev/null +++ b/packages/plugins/plugin-auth/src/vendor-admin-refusal-envelope.ts @@ -0,0 +1,154 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0112 envelope for the better-auth-native `/admin/` refusals (#10349). + * + * ## The defect, measured + * + * `/api/v1/auth/admin/*` is served by TWO implementations and answers the same + * question in two shapes. ObjectStack's raw Hono mounts (`create-user`, + * `set-user-password`, `unlock-user`, `import-users`, `ban-user`, `unban-user`, + * `oauth2/toggle-disabled`, `sso/*`) run `judgePlatformAdmin` and refuse an + * anonymous caller with the declared envelope — `401 { success: false, error: { + * code: 'UNAUTHENTICATED', message } }` (see `platform-admin-gate.ts`). The + * routes better-auth serves itself refuse the same caller through the vendor's + * `adminMiddleware`, which is `getAuthoritativeSessionFromCtx(ctx)` followed by + * `APIError.fromStatus('UNAUTHORIZED')` — no body argument at all. + * + * Measured on the installed better-auth 1.7.1, anonymous, through + * `AuthManager.handleRequest`, ten vendor-lane routes answered: + * + * POST /admin/impersonate-user -> 401 content-type: application/json body '' + * POST /admin/set-role -> 401 content-type: application/json body '' + * POST /admin/revoke-user-sessions -> 401 content-type: application/json body '' + * POST /admin/revoke-user-session -> 401 content-type: application/json body '' + * POST /admin/list-user-sessions -> 401 content-type: application/json body '' + * POST /admin/update-user -> 401 content-type: application/json body '' + * GET /admin/list-users -> 401 content-type: application/json body '' + * GET /admin/get-user -> 401 content-type: application/json body '' + * POST /admin/has-permission -> 401 content-type: application/json body '' + * POST /admin/stop-impersonating -> 401 content-type: application/json body '' + * + * The body is the EMPTY STRING, not an empty JSON object — which is why this + * module ADDS an envelope and never rewrites one. The `content-type` still + * announces `application/json`, so a client that believes the header and calls + * `JSON.parse` throws on the refusal instead of branching on it. A consumer + * cannot tell 401-means-sign-in from 401-means-anything-else without knowing, + * per route, which of the two implementations happens to serve it — and that + * split is an implementation detail, not a contract. + * + * ## What is normalized, and what is deliberately left alone + * + * Scope is the `/admin/` NAMESPACE only (triage's option C, not option B). + * Statuses are unchanged and admission is unchanged: this module never turns a + * 2xx into a refusal and never turns a refusal into a 2xx. It fills in a body + * that was empty, and nothing else. + * + * Three deliberate narrowings, each of which a broader rule would have broken: + * + * 1. **Empty body only.** A vendor refusal that DID say something keeps saying + * it byte-for-byte — the signed-in non-admin's + * `403 {"message":…,"code":"YOU_ARE_NOT_ALLOWED_TO_IMPERSONATE_USERS"}` is + * the vendor's own denial vocabulary and is what `admin-route-nonadmin- + * refusal.dogfood.test.ts` asserts on. Rewriting it would be a second, + * larger contract change smuggled in behind this one. + * 2. **Refusal statuses only** (401 / 403). `/admin/oauth2/*` answers 404 with + * an empty body when the `oidcProvider` plugin is off, and a 404 discloses + * nothing that needs a code; a semantic 4xx the vendor owns (409, 400) is + * not this seam's to name. + * 3. **The `/admin/` prefix.** Non-`/admin/` vendor routes are untouched, which + * is what makes this option C. `POST /sign-in/email` already answers + * `401 {"message":"Invalid email or password","code":"INVALID_EMAIL_OR_ + * PASSWORD"}` and stays exactly that. + * + * ## Why the code is DERIVED rather than written down here + * + * `standardErrorCodeForHttpStatus` (`@objectstack/spec/api`) is the one place a + * code is spelled for a producer that knows only a status — ADR-0112's own + * derived-code map, `401 -> UNAUTHENTICATED`, `403 -> PERMISSION_DENIED`. Using + * it means this module registers no vocabulary of its own and cannot drift from + * the catalog: there is no string literal here to drift. + * + * The messages come from `platform-admin-gate.ts`, the module that already owns + * ObjectStack's refusal wording. That is not decoration — it is what makes the + * two lanes answer an anonymous caller with BYTE-IDENTICAL bodies rather than + * with two independently-maintained strings that merely look alike today. + */ + +import { standardErrorCodeForHttpStatus } from '@objectstack/spec/api'; +import { PLATFORM_ADMIN_REFUSAL_MESSAGES } from './platform-admin-gate.js'; + +/** + * The `/admin/` namespace, in better-auth's own `ctx.path` spelling — the same + * spelling `AuthManager.betterAuthEndpointPath` returns and the same one + * `SESSION_ERASURE_PATHS` and the stop-impersonating recovery seam are keyed + * on. The trailing slash is load-bearing: it addresses the namespace and not a + * hypothetical route literally named `/admin`. + */ +export const VENDOR_ADMIN_PATH_PREFIX = '/admin/'; + +/** + * The statuses this seam will name. Exactly the two `judgePlatformAdmin` + * itself emits, and exactly the two the `/admin/` dogfood sweep asserts a + * non-admin receives. + * + * 403 is included although no bodyless 403 occurs on the installed vendor + * version (measured: the signed-in non-admin's 403 carries the vendor's + * `YOU_ARE_NOT_ALLOWED_*` body). One rule over both refusal statuses means a + * vendor change that starts refusing bodylessly with the other one cannot + * silently re-open this hole — which is the failure mode the card records: + * the gap was known, documented, and tracked by nothing. + */ +const NORMALIZED_REFUSAL_STATUSES: ReadonlySet = new Set([401, 403]); + +/** Is `endpointPath` inside the better-auth `/admin/` namespace? */ +export function isVendorAdminPath(endpointPath: string | undefined): boolean { + return endpointPath !== undefined && endpointPath.startsWith(VENDOR_ADMIN_PATH_PREFIX); +} + +/** + * Give a bodyless vendor-lane `/admin/` refusal the ADR-0112 envelope. + * + * Returns the response UNCHANGED — the same object, not a copy — whenever any + * of the three narrowings above applies, so the untouched paths are untouched + * by identity and a test can assert that with `toBe`. + * + * Headers are carried over rather than rebuilt: better-auth attaches + * `Set-Cookie` to refusals on some paths, and dropping them would change + * behaviour well outside this card. Only `content-type` is asserted (the + * vendor already claims `application/json`; now it is telling the truth) and + * `content-length` is dropped, since the body length changed. + */ +export async function envelopeVendorAdminRefusal( + endpointPath: string | undefined, + response: Response, +): Promise { + if (!isVendorAdminPath(endpointPath)) return response; + if (!NORMALIZED_REFUSAL_STATUSES.has(response.status)) return response; + + let body: string; + try { + body = await response.clone().text(); + } catch { + // Unreadable body (already-disturbed stream) → leave it exactly as it is. + // A refusal we cannot inspect is not one we may rewrite. + return response; + } + if (body !== '') return response; + + const code = standardErrorCodeForHttpStatus(response.status); + const message = + PLATFORM_ADMIN_REFUSAL_MESSAGES[response.status as 401 | 403] ?? + /* istanbul ignore next — unreachable while the set above holds two members */ + 'Refused'; + + const headers = new Headers(response.headers); + headers.set('content-type', 'application/json'); + headers.delete('content-length'); + + return new Response(JSON.stringify({ success: false, error: { code, message } }), { + status: response.status, + statusText: response.statusText, + headers, + }); +} 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 c1d1433459..8ba8b6f0e7 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,16 @@ // 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. On this +// `better-auth-gate` (9 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 +// anonymous caller with a bodyless 401 and there was no code to assert; +// `AuthManager.handleRequest` now envelopes that refusal at the one seam +// every vendor route passes through, so the asymmetry this file used to +// DOCUMENT is closed and this bucket pins the closure. +// +// The signed-in-non-admin half stays the vendor's own vocabulary. On this // stack the platform admin is refused these routes too, with the same // `YOU_ARE_NOT_ALLOWED_TO_*` code as the member. That is not a harness // artifact: better-auth's admin plugin authorizes on the legacy @@ -549,11 +558,23 @@ describe('#9482 C9: every derived /admin/ route refuses a non-admin', () => { expect(routes.length, 'no better-auth-gate routes were derived').toBeGreaterThan(0); for (const route of routes) { + // ── #10349: this used to be `[401, 403].includes(anon.status)` ──────── + // + // It had to be, because there was no code to assert: the vendor's + // `adminMiddleware` refused an anonymous caller with a bodyless 401 — + // `content-type: application/json` and the EMPTY STRING — so this suite + // DOCUMENTED the asymmetry against the `objectstack-gate` bucket above + // rather than closing it, and nothing tracked closing it. + // + // `AuthManager.handleRequest` now gives the vendor lane's `/admin/` + // refusals the ADR-0112 envelope (`vendor-admin-refusal-envelope.ts`), + // so the two buckets answer an anonymous caller identically and this + // assertion is the FIX'S OWN falsifiable pin: status AND code, exactly + // as the ObjectStack bucket asserts them thirty lines up. Loosening it + // back re-opens the gap silently. const anon = await fire(route, undefined); - expect( - [401, 403].includes(anon.status), - `${route} anonymous should be refused, got ${anon.status} ${anon.body}`, - ).toBe(true); + 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( @@ -563,12 +584,33 @@ describe('#9482 C9: every derived /admin/ route refuses a non-admin', () => { // When the vendor answers with a body, it must be its own denial // vocabulary — not a validation error, which would mean the request died // before the gate and this assertion measured nothing. + // + // ⚠️ #10792, found the moment #10349 made this branch executable at all. + // It was guarded by `member.code !== undefined`, and the code WAS + // undefined on every bodyless refusal — so for those routes this check + // had never once run. On the first run where it did, `remove-user` came + // back `401 UNAUTHENTICATED` for a SIGNED-IN member while its siblings + // `set-role` and `update-user` answered the same bearer + // `403 YOU_ARE_NOT_ALLOWED_*`: on that path alone the session is re-read + // inside the #7724 erasure transaction and comes back empty, so + // authentication answers a question authorization should have. + // + // Recorded as an ADDITIONAL accepted code for that one route, never as a + // pin — same reasoning as the platform-admin arm below. Pinning today's + // 401 would turn the fix red; pinning the 403 is red today; and widening + // the vocabulary for EVERY route would let the next route drift into the + // same state in silence. Delete this arm when #10792 closes. + const KNOWN_AUTHN_BEFORE_AUTHZ = 'POST /api/v1/auth/admin/remove-user'; // #10792 + const denialCodes = + route === KNOWN_AUTHN_BEFORE_AUTHZ + ? /^(YOU_ARE_NOT_ALLOWED|UNAUTHENTICATED$)/ + : /^YOU_ARE_NOT_ALLOWED/; if (member.code !== undefined) { expect( member.code, `${route} member: refused with ${member.code}, which is not a denial code. A ` + `VALIDATION_ERROR here means the payload never reached the gate.`, - ).toMatch(/^YOU_ARE_NOT_ALLOWED/); + ).toMatch(denialCodes); } } // ⛔ No allowed-side assertion in this bucket — see the header: the platform