Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions .changeset/admin-vendor-refusal-envelope.md
Original file line numberDiff line numberDiff line change
@@ -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.

<!-- 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. -->
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 () => {
Expand Down
23 changes: 22 additions & 1 deletion packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand DownExpand Up@@ -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();
Expand Down
25 changes: 23 additions & 2 deletions packages/plugins/plugin-auth/src/platform-admin-gate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<Record<401 | 403, string>> = {
401: 'Sign in first',
403: 'Admin role required',
};

/**
* Is this session user a platform admin under ADR-0068 D2?
*
Expand DownExpand Up@@ -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] },
},
},
};
}
Expand All@@ -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] },
},
},
};
}
Expand Down
Loading
Loading