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
15 changes: 15 additions & 0 deletions .changeset/24243-plugin-auth-hook-refusal-codes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@objectstack/plugin-auth": patch
---

fix(plugin-auth): the organization, invitation-placement and phone-OTP hook refusals carry their ADR-0112 codes

Clause-②: no

Eight refusals thrown from plugin-auth's better-auth hooks answered with `message` alone, so a client could only show or match English prose. Each now carries a `code` it can branch on, and keeps its HTTP status.

- `POST /auth/organization/create`: the reserved `__global__` id or slug answers 400 `VALIDATION_ERROR`. Where the deployment's effective tenancy posture enforces no organization wall (`/auth/config` publishes `features.multiOrgEnabled: false`), creating an organization answers 403 `ORGANIZATION_CREATION_DISABLED`.
- `POST /auth/organization/invite-member` with placement intent (`businessUnitId` and `positions`): with no delegated-administration runtime to authorize it, 403 `INVITE_PLACEMENT_UNSUPPORTED`; refused by the delegated-administration gate, 403 `INVITE_PLACEMENT_OUTSIDE_ISSUER_SCOPE`.
- `POST /auth/organization/update` with a new slug: while the organization has an active environment (any environment that is neither archived nor failed), 403 `ORGANIZATION_SLUG_LOCKED_BY_ACTIVE_ENVIRONMENTS`. Its message no longer says environments "still reference" the organization or tells you to rename their hostnames: the guard reads no hostname, and archiving those environments is the remedy. When the guard's own read cannot answer, 503 `SERVICE_UNAVAILABLE`.
- Phone OTP sends (`/auth/phone-number/send-otp`, `/auth/phone-number/request-password-reset`): the per-number limit and the deployment's daily SMS quota both answer 429 `RATE_LIMIT_EXCEEDED`. It is one code for both walls, so a caller still cannot tell which budget it hit. The body is better-auth's flat `code` and `message`, with no `Retry-After` header.
- **One behaviour change:** a placement check that fails before the gate reaches a verdict (the issuer's grants could not be read from the store) now answers 503 `SERVICE_UNAVAILABLE`, not a 403 scope refusal carrying the store's message. No invitation is created either way.
17 changes: 11 additions & 6 deletions content/docs/protocol/kernel/error-handling.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -738,8 +738,9 @@ have let them count the dependents; `dependentObject` is named either way. Treat
#### `RATE_LIMIT_EXCEEDED`
**HTTP Status:** 429
**Meaning:** Too many requests, rate limit exceeded
**Envelope:** nested — built by the inbound rate limiter, which sits **ahead of every
route**, so it is never flat whichever family you were calling
**Envelope:** nested from the inbound rate limiter, which sits **ahead of every
route**, so its refusal is never flat whichever family you were calling. The phone-OTP
send walls answer this code too, flat (see the end of `QUOTA_EXCEEDED` below)

**Example:**
```json
Expand Down Expand Up @@ -835,10 +836,14 @@ Don't feed this 429 to a retry loop that only checks the status, such as the
`RATE_LIMIT_EXCEEDED` example above. It has no `Retry-After` to wait on, and every
retry before `resetAt` is refused again.

Other quota enforcement answers with its own code rather than this one
(the SMS daily quota answers `TOO_MANY_REQUESTS`). For request pacing, the code
on the wire is `RATE_LIMIT_EXCEEDED` above, whose `Retry-After` header and
`retryAfterSeconds` / `resetAt` details are what a client acts on.
Other quota enforcement answers with a code other than this one. The phone-OTP send
walls (`/auth/phone-number/send-otp` and `/auth/phone-number/request-password-reset`), a
phone number's own limit and the deployment's daily SMS quota, both answer 429
`RATE_LIMIT_EXCEEDED`: one code for both, so a caller cannot tell which budget it hit.
That body is better-auth's flat `code` and `message`, with no `Retry-After` header and no
`details`. At the inbound rate limiter, the code on the wire is `RATE_LIMIT_EXCEEDED`
above, whose `Retry-After` header and `retryAfterSeconds` / `resetAt` details are what a
client acts on.

### Server Errors

Expand Down
244 changes: 244 additions & 0 deletions packages/plugins/plugin-auth/src/api-error-code.pin.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#24243, ADR-0112] Every `new APIError(` this package throws carries a `code`.
*
* better-auth serialises an `APIError` body as it is given. A body without a
* `code` reaches the caller as `message` alone, so a client (the console among
* them) can only show or pattern-match English prose, on doors where
* better-auth's own refusals carry codes. Eight throws in `auth-manager.ts`
* shipped that way, found by hand; this pin is the enumeration, so the next one
* cannot ship without a code. It is a test in this package, not a CI gate.
*
* It PARSES every non-test source file under `src/` and finds every
* `new APIError(...)`. A site passes when its body carries the code in one of
* two shapes, and is reported with its file and line otherwise:
*
* 1. an object literal with a top-level `code` property; or
* 2. a body destructured, in the same function, from the result of
* `judgeHeldSession(...)`: the held-session judge's ADR-0112 envelope,
* whose code sits under `error` (`platform-admin-gate.held-session.test.ts`
* pins that body's shape).
*
* No body, a literal without `code`, a spread, or an identifier from anywhere
* else is a site this pin cannot read a code from, so it fails.
*
* It parses and does not type-check, so the code's VALUE is not judged here.
* Each stamp is a constant typed `RegisteredErrorCode` or `StandardErrorCode`,
* so `tsc` judges the value, and `check:error-code-provenance` judges whether a
* registered one is listed under this package.
*
* Two controls keep the walk honest:
* - a textual count over the comment-free source (the TypeScript printer, with
* comments removed) must equal the sites the walk found in each file, so a
* constructor spelling the walk does not recognise cannot slip past it;
* - the classifier must report each shape that shipped without a code and
* accept each shape that carries one (the fixture battery below).
*/

import { describe, it, expect } from 'vitest';
import { readdirSync, readFileSync } from 'node:fs';
import { dirname, join, relative } from 'node:path';
import { fileURLToPath } from 'node:url';
import ts from 'typescript';

const HERE = dirname(fileURLToPath(import.meta.url));

interface ApiErrorSite {
readonly file: string;
readonly line: number;
/** `null` when the body carries a code; otherwise why this pin cannot read one. */
readonly missing: string | null;
}

const isTestFile = (name: string): boolean => /\.(test|spec)\.tsx?$/.test(name) || name.endsWith('.d.ts');

function sourceFiles(dir: string): string[] {
const out: string[] = [];
for (const entry of readdirSync(dir, { withFileTypes: true })) {
const path = join(dir, entry.name);
if (entry.isDirectory()) out.push(...sourceFiles(path));
else if (/\.tsx?$/.test(entry.name) && !isTestFile(entry.name)) out.push(path);
}
return out.sort();
}

const isApiErrorCallee = (callee: ts.Expression): boolean =>
(ts.isIdentifier(callee) && callee.text === 'APIError') ||
(ts.isPropertyAccessExpression(callee) && callee.name.text === 'APIError');

function propertyName(name: ts.PropertyName): string | undefined {
if (ts.isIdentifier(name) || ts.isStringLiteral(name)) return name.text;
return undefined;
}

function enclosingFunction(node: ts.Node): ts.Node | undefined {
for (let cur = node.parent; cur; cur = cur.parent) {
if (ts.isFunctionLike(cur)) return cur;
}
return undefined;
}

/** Every variable declaration inside `scope`, nested blocks included. */
function declarationsIn(scope: ts.Node): ts.VariableDeclaration[] {
const found: ts.VariableDeclaration[] = [];
const visit = (node: ts.Node): void => {
if (ts.isVariableDeclaration(node)) found.push(node);
ts.forEachChild(node, visit);
};
visit(scope);
return found;
}

const unwrapAwait = (e: ts.Expression | undefined): ts.Expression | undefined =>
e && ts.isAwaitExpression(e) ? e.expression : e;

/**
* Shape 2: `const { body } = held;` (or `{ body: X }`) where `held` is
* `judgeHeldSession(…)`'s result, both in the throw's own function.
*/
function isHeldSessionBody(body: ts.Identifier, site: ts.NewExpression): boolean {
const scope = enclosingFunction(site);
if (!scope) return false;
const decls = declarationsIn(scope);
const holders = decls
.filter((d) => ts.isObjectBindingPattern(d.name) && d.initializer && ts.isIdentifier(d.initializer))
.filter((d) =>
(d.name as ts.ObjectBindingPattern).elements.some((el) => {
const key = el.propertyName ? propertyName(el.propertyName) : ts.isIdentifier(el.name) ? el.name.text : undefined;
return key === 'body' && ts.isIdentifier(el.name) && el.name.text === body.text;
}),
)
.map((d) => (d.initializer as ts.Identifier).text);
return holders.some((holder) =>
decls.some((d) => {
if (!ts.isIdentifier(d.name) || d.name.text !== holder) return false;
const init = unwrapAwait(d.initializer);
return (
!!init && ts.isCallExpression(init) && ts.isIdentifier(init.expression) && init.expression.text === 'judgeHeldSession'
);
}),
);
}

function whyCodeless(site: ts.NewExpression, sf: ts.SourceFile): string | null {
const body = site.arguments?.[1];
if (!body) return 'it passes no body at all';
if (ts.isObjectLiteralExpression(body)) {
const carriesCode = body.properties.some(
(p) => (ts.isPropertyAssignment(p) || ts.isShorthandPropertyAssignment(p)) && propertyName(p.name) === 'code',
);
return carriesCode ? null : 'its body has no top-level `code` property';
}
if (ts.isIdentifier(body) && isHeldSessionBody(body, site)) return null;
return `its body is \`${body.getText(sf)}\`, which this pin cannot read a code from`;
}

/** Every `new APIError(…)` in one source text, each with its verdict. */
function apiErrorSites(file: string, text: string): ApiErrorSite[] {
const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
const sites: ApiErrorSite[] = [];
const visit = (node: ts.Node): void => {
if (ts.isNewExpression(node) && isApiErrorCallee(node.expression)) {
sites.push({
file,
line: sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1,
missing: whyCodeless(node, sf),
});
}
ts.forEachChild(node, visit);
};
visit(sf);
return sites;
}

/** The textual control: constructor calls counted in the source with its comments removed. */
function textualCount(file: string, text: string): number {
const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, false, ts.ScriptKind.TS);
const printed = ts.createPrinter({ removeComments: true }).printFile(sf);
return printed.match(/\bnew\s+(?:[A-Za-z_$][\w$]*\s*\.\s*)*APIError\s*\(/g)?.length ?? 0;
}

describe('every new APIError( in plugin-auth/src carries an ADR-0112 code (#24243)', () => {
const files = sourceFiles(HERE);
const perFile = files.map((path) => {
const text = readFileSync(path, 'utf8');
const file = relative(HERE, path);
return { file, sites: apiErrorSites(file, text), textual: textualCount(file, text) };
});
const sites = perFile.flatMap((f) => f.sites);

it('the walk reaches the throws: a floor of sites and files, so a walk that reads nothing cannot pass', () => {
// Measured when this pin landed: 30 sites in 4 files (`auth-manager.ts`
// 24, `objectql-adapter.ts` 3, `impersonation-bearer-rotation.ts` 2,
// `admin-impersonate-endpoint.ts` 1). A floor, not an exact count, so a new
// throw does not have to edit this line; only losing them does.
expect(sites.length).toBeGreaterThanOrEqual(30);
expect(new Set(sites.map((s) => s.file)).size).toBeGreaterThanOrEqual(4);
expect(perFile.find((f) => f.file === 'auth-manager.ts')?.sites.length ?? 0).toBeGreaterThanOrEqual(24);
});

it('the textual control agrees with the walk in every file', () => {
const disagreements = perFile
.filter((f) => f.textual !== f.sites.length)
.map((f) => `${f.file}: ${f.textual} textual, ${f.sites.length} walked`);
expect(disagreements).toEqual([]);
});

it('no site throws a body without a code', () => {
const codeless = sites.filter((s) => s.missing !== null).map((s) => `${s.file}:${s.line}: ${s.missing}`);
expect(codeless).toEqual([]);
});
});

describe('the classifier, against the shapes it must tell apart', () => {
const verdicts = (body: string) => apiErrorSites('fixture.ts', body).map((s) => s.missing === null);

it('reports both shapes the eight code-less throws had (the card enumeration)', () => {
// As they stood before #24243: a status read off a constant with a
// message-only body (the SMS quota wall), and a status literal with a
// message-only body (the other seven).
const fixture = `
async function quota(message: string) { return new APIError(SMS_QUOTA_EXCEEDED_CODE, { message }); }
async function hook() {
throw new APIError('FORBIDDEN', { message: 'Creating additional organizations is disabled.' });
}
async function relayed(err: any) {
throw new APIError('FORBIDDEN', { message: err?.message ?? 'outside your scope' });
}`;
expect(verdicts(fixture)).toEqual([false, false, false]);
});

it('reports a site with no body, a spread with no code, and an identifier it cannot trace', () => {
const fixture = `
function a() { throw new APIError('FORBIDDEN'); }
function b(base: object) { throw new APIError('FORBIDDEN', { ...base }); }
function c(body: object) { throw new APIError('FORBIDDEN', body); }
function d() { const { body } = somethingElse(); throw new APIError('FORBIDDEN', body); }`;
expect(verdicts(fixture)).toEqual([false, false, false, false]);
});

it('accepts a literal or shorthand `code`, a string key, a member callee and the held-session relay', () => {
const fixture = `
function a() { throw new APIError('FORBIDDEN', { message: 'm', code: SOME_CODE }); }
function b(code: string) { throw new APIError('FORBIDDEN', { message: 'm', code }); }
function c() { throw new APIError('FORBIDDEN', { 'code': 'X', message: 'm' }); }
function d() { throw new api.APIError('FORBIDDEN', { message: 'm', code: SOME_CODE }); }
async function e() {
const held = judgeHeldSession({ authGate: await gateOf() });
if (!held) return;
const { body } = held;
throw new APIError('FORBIDDEN', body);
}`;
expect(verdicts(fixture)).toEqual([true, true, true, true, true]);
});

it('the textual control sees past comments, and counts a member-callee site', () => {
const fixture = `
// new APIError('FORBIDDEN', { message: 'in a comment' })
/** new APIError('X') in a doc block */
function a() { throw new api.APIError('FORBIDDEN', { message: 'm', code: C }); }`;
expect(textualCount('fixture.ts', fixture)).toBe(1);
expect(apiErrorSites('fixture.ts', fixture)).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
* `sys_environment` (the open-source composition) declares the guard
* inapplicable without reading anything. The real ObjectQL registry
* answers that question in the last section.
*
* [#24243] Both answers are pinned on their ADR-0112 codes, the refusal's
* registered `ORGANIZATION_SLUG_LOCKED_BY_ACTIVE_ENVIRONMENTS` and the read
* fault's standard `SERVICE_UNAVAILABLE`, at this level rather than on the
* wire: `sys_environment` is a cloud-provided object that no composition in this
* repository registers, so on a stock dogfood boot the guard does not apply and
* neither answer has a door.
*/

import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
Expand Down Expand Up @@ -108,15 +115,19 @@ async function slugGuard(dataEngine: unknown) {
return (slug: string) => guard({ organization: { slug }, member: { organizationId: ORG.id } });
}

/** The guard's refusal, asserted on its envelope: better-auth's `FORBIDDEN` / 403, naming the active environments. */
/**
* The guard's refusal, asserted on its envelope: better-auth's `FORBIDDEN` / 403
* with its registered ADR-0112 code (#24243), naming the active environments.
*/
async function expectSlugRefusal(attempt: Promise<unknown>) {
const err = (await attempt.then(
() => undefined,
(e: unknown) => e,
)) as { status?: unknown; statusCode?: unknown; body?: { message?: unknown } } | undefined;
)) as { status?: unknown; statusCode?: unknown; body?: { code?: unknown; message?: unknown } } | undefined;
expect(err, 'the slug change was not refused').toBeTruthy();
expect(err?.statusCode).toBe(403);
expect(err?.status).toBe('FORBIDDEN');
expect(err?.body?.code).toBe('ORGANIZATION_SLUG_LOCKED_BY_ACTIVE_ENVIRONMENTS');
expect(String(err?.body?.message)).toMatch(/active.*environment/i);
}

Expand Down Expand Up @@ -180,18 +191,20 @@ describe('organization slug guard — on an engine that refuses a principal-less

/**
* A read that could not answer, asserted on its envelope: better-auth's
* `SERVICE_UNAVAILABLE` / 503, naming the object whose read failed — and
* explicitly NOT the guard's `FORBIDDEN` / 403, which is a verdict about the
* slug change that the guard never reached.
* `SERVICE_UNAVAILABLE` / 503 with the standard code of the same name (#24243),
* naming the object whose read failed — and explicitly NOT the guard's
* `FORBIDDEN` / 403, which is a verdict about the slug change that the guard
* never reached.
*/
async function expectReadFaultRefusal(attempt: Promise<unknown>, object: string) {
const err = (await attempt.then(
() => undefined,
(e: unknown) => e,
)) as { status?: unknown; statusCode?: unknown; body?: { message?: unknown }; cause?: unknown } | undefined;
)) as { status?: unknown; statusCode?: unknown; body?: { code?: unknown; message?: unknown }; cause?: unknown } | undefined;
expect(err, 'the slug change was let through on a read that could not answer').toBeTruthy();
expect(err?.statusCode).toBe(503);
expect(err?.status).toBe('SERVICE_UNAVAILABLE');
expect(err?.body?.code).toBe('SERVICE_UNAVAILABLE');
expect(String(err?.body?.message)).toContain(`\`${object}\``);
return err;
}
Expand Down
Loading
Loading