Skip to content

Commit 4d7c564

Browse files
os-warrenclaude
andauthored
fix(plugin-auth): the better-auth-native /admin/ routes refuse an anonymous caller with the ADR-0112 envelope (#10800)
* fix(plugin-auth): envelope the better-auth-native /admin/ refusals (#10349) The `/api/v1/auth/admin/` namespace answered the same question in two shapes. ObjectStack's raw mounts refuse an anonymous caller with the ADR-0112 envelope and `code: 'UNAUTHENTICATED'` (`platform-admin-gate.ts`); the routes better-auth serves itself refuse through the vendor's `adminMiddleware` (`APIError.fromStatus('UNAUTHORIZED')`, no body argument), which reaches the client as a 401 announcing `application/json` and carrying the EMPTY STRING. Measured on better-auth 1.7.1 through `AuthManager.handleRequest`, anonymous: ten vendor-lane routes answered `401 ct=application/json len=0 body=""`. `handleRequest` now gives those refusals the declared envelope at the one seam every vendor route passes through, with the code DERIVED from the status by ADR-0112's own `standardErrorCodeForHttpStatus` map — no new error code, no literal to drift. Statuses and admission are unchanged. Scope is the `/admin/` namespace (option C, not option B). The prefix test costs no new concept: `handleRequest` already discriminates on `betterAuthEndpointPath` twice, for `STOP_IMPERSONATING_PATH` and `SESSION_ERASURE_PATHS`. Three narrowings, each pinned: a refusal that already carried a body keeps it byte-for-byte; only 401/403 are named (a bodyless `/admin/oauth2/*` 404 and any semantic 4xx the vendor owns stay as they are); nothing outside `/admin/` is touched. The dogfood sweep's `better-auth-gate` bucket tightens from `[401, 403].includes(anon.status)` to the full ADR-0112 pin — status AND code — which is the fix's own falsifiable assertion. That bucket documented this gap instead of closing it, and nothing tracked closing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx * test(dogfood): record the #10792 member-arm exception the tightened bucket exposed The `better-auth-gate` bucket's member-arm vocabulary check was guarded by `if (member.code !== undefined)`, and on every bodyless refusal the code WAS undefined — so for those routes the check had never executed. Giving the vendor lane an envelope made it executable, and it went red on the first run: `/admin/remove-user` answers a SIGNED-IN member `401 UNAUTHENTICATED`, while `set-role` and `update-user` answer the same bearer `403 YOU_ARE_NOT_ALLOWED_*`. Measured on the booted showcase stack and controlled hermetically: the same three fires against the in-memory engine give the member 403, both with no `transaction` on the engine and with a pass-through one. So it is the real erasure transaction (#7724, `SESSION_ERASURE_PATHS`) that the session re-read inside `adminMiddleware` does not survive. Filed as #10792. Recorded here as an ADDITIONAL accepted code for that one route, never as a pin — the same reasoning the platform-admin arm below already carries. 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 one drift in silence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent c5d0c2f commit 4d7c564

7 files changed

Lines changed: 590 additions & 18 deletions
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
"@objectstack/plugin-auth": minor
3+
---
4+
5+
fix(plugin-auth): the better-auth-native `/admin/` routes refuse an anonymous caller with the ADR-0112 envelope (#10349)
6+
7+
**BREAKING** response-shape change on the `/api/v1/auth/admin/` namespace,
8+
shipped as `minor` under the repo's launch-window convention for breaking
9+
changes.
10+
11+
`/api/v1/auth/admin/` is served by two implementations and answered the same
12+
question in two shapes. ObjectStack's raw mounts (`create-user`,
13+
`set-user-password`, `unlock-user`, `import-users`, `ban-user`, `unban-user`,
14+
`oauth2/toggle-disabled`, `sso/*`) refuse an anonymous caller through
15+
`judgePlatformAdmin` with the declared envelope and `code: 'UNAUTHENTICATED'`.
16+
The routes better-auth serves itself refuse through the vendor's
17+
`adminMiddleware``getAuthoritativeSessionFromCtx(ctx)` then
18+
`APIError.fromStatus('UNAUTHORIZED')`, with no body argument at all.
19+
20+
Measured on the installed better-auth 1.7.1, anonymous, through
21+
`AuthManager.handleRequest`: ten vendor-lane routes (`impersonate-user`,
22+
`set-role`, `revoke-user-sessions`, `revoke-user-session`,
23+
`list-user-sessions`, `update-user`, `list-users`, `get-user`,
24+
`has-permission`, `stop-impersonating`) answered `401` with a
25+
`content-type: application/json` header and the **empty string** as the body.
26+
A client that believes that header and parses the body throws on the refusal
27+
instead of branching on it, and a client that wants to branch has to know, per
28+
route, which of the two implementations happens to serve it — an
29+
implementation detail, not a contract.
30+
31+
`AuthManager.handleRequest` now gives those refusals the declared envelope at
32+
the one seam every vendor route passes through. **Statuses are unchanged and
33+
admission is unchanged**: nothing that was refused is now admitted, nothing
34+
that was admitted is now refused, and no status moved. What is added is the
35+
machine-readable `code`, derived from the status by ADR-0112's own
36+
`standardErrorCodeForHttpStatus` map rather than spelled out again — so no new
37+
error code is registered and the vendor lane's anonymous refusal is now
38+
byte-identical to the ObjectStack lane's.
39+
40+
Scope is the `/admin/` namespace only. Three narrowings hold the rest of the
41+
surface still, and each is pinned:
42+
43+
- **A refusal that already carried a body keeps it, byte for byte.** The
44+
signed-in non-admin's `403` with the vendor's own
45+
`YOU_ARE_NOT_ALLOWED_TO_*` vocabulary is untouched; this change fills in an
46+
empty body and never rewrites a spoken one.
47+
- **Only the two refusal statuses are named** (`401`, `403`). A bodyless `404`
48+
such as `/admin/oauth2/*` with the `oidcProvider` plugin off, and any
49+
semantic `4xx` the vendor owns, are left exactly as they are.
50+
- **Nothing outside `/admin/` is touched.** `POST /sign-in/email` still answers
51+
`401 {"message":"Invalid email or password","code":"INVALID_EMAIL_OR_PASSWORD"}`,
52+
measured identical on both sides of the change.
53+
54+
Consumers that branch on the HTTP status are unaffected. Consumers that already
55+
parse the ObjectStack `/admin/*` envelope now get the same shape everywhere in
56+
the namespace, with no per-route knowledge required.
57+
58+
<!-- adr-0087: not-required (no-migration-prescription) nothing is removed, renamed or narrowed: a refusal that carried an empty body under an `application/json` header now carries the declared envelope at the same status. No consumer expression has to be rewritten — a status branch keeps working unchanged, and an envelope branch that only ever matched the ObjectStack lane now also matches the vendor lane. There is no old spelling to migrate off, so there is nothing for `os migrate meta` to rewrite and no ADR-0087 ledger entry to make. -->

packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -243,15 +243,23 @@ describe('the other direction — a non-entitled caller is still refused', () =>
243243
// "the gate said no".
244244
expect(res.status).toBe(401);
245245

246-
// ⚠️ Measured, and deliberately not dressed up: this refusal carries an
247-
// EMPTY body — no ADR-0112 envelope, no `code`. It comes from the vendor's
248-
// `adminMiddleware` (`APIError.fromStatus('UNAUTHORIZED')`), which runs
249-
// before this endpoint's handler and is byte-identical on stock
250-
// better-auth 1.7.1 for both `/admin/impersonate-user` and
251-
// `/admin/set-role`. This card changes the AUTHORIZATION predicate, not the
252-
// authentication middleware, so the shape is pinned as it is rather than
253-
// asserted to be something it is not.
254-
expect(await res.text()).toBe('');
246+
// ⚠️ This assertion USED to read `expect(await res.text()).toBe('')` — the
247+
// vendor's `adminMiddleware` (`APIError.fromStatus('UNAUTHORIZED')`, no body
248+
// argument) refused an anonymous caller with the EMPTY STRING under a
249+
// `content-type: application/json` header, on every better-auth-native
250+
// `/admin/` route. #9968 changed the AUTHORIZATION predicate only and pinned
251+
// that shape as it was rather than dressing it up.
252+
//
253+
// #10349 closed it at the ONE seam every vendor route passes through
254+
// (`AuthManager.handleRequest` → `vendor-admin-refusal-envelope.ts`), so the
255+
// anonymous refusal now carries the ADR-0112 envelope — `code` AND `status`
256+
// — and is byte-identical to what the ObjectStack raw `/admin/*` mounts
257+
// answer. Authorization here is still untouched: the 403 assertions above
258+
// and the 200 below are unchanged.
259+
expect(JSON.parse(await res.text())).toEqual({
260+
success: false,
261+
error: { code: 'UNAUTHENTICATED', message: 'Sign in first' },
262+
});
255263
});
256264

257265
it('an org owner/admin who is NOT a platform admin is refused', async () => {

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
type AuthEventAuditSurface,
3131
} from './auth-session-audit.js';
3232
import { SESSION_ERASURE_PATHS } from './session-tombstone.js';
33+
import { envelopeVendorAdminRefusal } from './vendor-admin-refusal-envelope.js';
3334
import {
3435
ADMIN_SESSION_COOKIE_KEY,
3536
STOP_IMPERSONATING_PATH,
@@ -3680,11 +3681,31 @@ export class AuthManager {
36803681
// is left with an identity that still occupies the org roster and can no
36813682
// longer sign in. Nothing tells the operator, and there is no way back.
36823683
const endpointPath = this.betterAuthEndpointPath(request);
3683-
const response =
3684+
const vendorResponse =
36843685
endpointPath !== undefined && SESSION_ERASURE_PATHS.has(endpointPath)
36853686
? await this.runSubjectErasureAtomically(runHandler)
36863687
: await runHandler();
36873688

3689+
// [#10349] The better-auth-native `/admin/` routes refuse an anonymous
3690+
// caller through the vendor's `adminMiddleware`
3691+
// (`APIError.fromStatus('UNAUTHORIZED')`, no body argument), so the refusal
3692+
// reaches the client as a 401 that announces `application/json` and carries
3693+
// the EMPTY STRING — no envelope, nothing to branch on. The ObjectStack raw
3694+
// `/admin/*` mounts answer the identical question with the ADR-0112
3695+
// envelope and `code: 'UNAUTHENTICATED'` (`platform-admin-gate.ts`), and
3696+
// which of the two a caller gets depends only on which implementation
3697+
// happens to serve that route — an implementation detail, not a contract.
3698+
//
3699+
// This is the ONE seam every vendor route passes through, which is why the
3700+
// normalization belongs here and not in ten routes we do not own. It is
3701+
// scoped to the `/admin/` NAMESPACE (option C): the prefix test costs no new
3702+
// concept, because this method already discriminates on `endpointPath` twice
3703+
// above — `STOP_IMPERSONATING_PATH` and `SESSION_ERASURE_PATHS`.
3704+
//
3705+
// Status and admission are untouched; see the module header for the three
3706+
// narrowings and the measurement behind each.
3707+
const response = await envelopeVendorAdminRefusal(endpointPath, vendorResponse);
3708+
36883709
if (response.status >= 500) {
36893710
try {
36903711
const body = await response.clone().text();

packages/plugins/plugin-auth/src/platform-admin-gate.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,21 @@ export type PlatformAdminVerdict =
4040
| { ok: true; actor: PlatformAdminActor }
4141
| { ok: false; refusal: PlatformAdminRefusal };
4242

43+
/**
44+
* The human half of the two refusals, keyed by the status that carries them.
45+
*
46+
* Lifted out of `judgePlatformAdmin` (whose bytes are unchanged) so the
47+
* better-auth-native `/admin/` lane can answer an anonymous caller with the
48+
* SAME body rather than a second string that merely looks the same today —
49+
* see `vendor-admin-refusal-envelope.ts` (#10349). The machine half is not
50+
* duplicated anywhere: it is ADR-0112's own derived-code map,
51+
* `standardErrorCodeForHttpStatus`.
52+
*/
53+
export const PLATFORM_ADMIN_REFUSAL_MESSAGES: Readonly<Record<401 | 403, string>> = {
54+
401: 'Sign in first',
55+
403: 'Admin role required',
56+
};
57+
4358
/**
4459
* Is this session user a platform admin under ADR-0068 D2?
4560
*
@@ -82,7 +97,10 @@ export function judgePlatformAdmin(session: unknown): PlatformAdminVerdict {
8297
ok: false,
8398
refusal: {
8499
status: 401,
85-
body: { success: false, error: { code: 'UNAUTHENTICATED', message: 'Sign in first' } },
100+
body: {
101+
success: false,
102+
error: { code: 'UNAUTHENTICATED', message: PLATFORM_ADMIN_REFUSAL_MESSAGES[401] },
103+
},
86104
},
87105
};
88106
}
@@ -92,7 +110,10 @@ export function judgePlatformAdmin(session: unknown): PlatformAdminVerdict {
92110
ok: false,
93111
refusal: {
94112
status: 403,
95-
body: { success: false, error: { code: 'PERMISSION_DENIED', message: 'Admin role required' } },
113+
body: {
114+
success: false,
115+
error: { code: 'PERMISSION_DENIED', message: PLATFORM_ADMIN_REFUSAL_MESSAGES[403] },
116+
},
96117
},
97118
};
98119
}

0 commit comments

Comments
 (0)