Skip to content

Commit 950f949

Browse files
fix(plugin-auth): a refused OAuth application registration answers the status the refusal declared, not 500 (#24853)
Part of #24773 — this PR does the first half only: a refused registration now answers 4xx instead of 500. The second half, whether a `web` client may register an `http://localhost` / `http://127.0.0.1` redirect URI, is an authentication-flow acceptance rule. It was measured here and nothing about it changed. It goes to a decision card. #24773 remains open. Clause-②: no ## What changed `POST /api/v1/auth/sys-oauth-application/register` (`packages/plugins/plugin-auth/src/auth-plugin.ts`) answered most refusals from better-auth as `500 OAUTH_REGISTER_FAILED`. - **Mechanism, measured against the installed `@better-auth/oauth-provider@1.7.3` / `better-call@1.4.0`.** Every refusal better-auth raises is better-call's `APIError`. It carries its status twice: as the number `statusCode` (`400`) and as the name `status` (`'BAD_REQUEST'`). The door read only a numeric `status`, so every refusal with a named status fell through to `500`. The body-schema `ValidationError` has the number `400` in its `status`, so that refusal answered `400` from the same door. One door gave two answers for the same class of refusal. - **Fix.** The door now reads the status through `resolveThrownHttpError` (`@objectstack/types`). That is the repo's one rule for which status a thrown error declares: a numeric `status`, then `statusCode`, and the caller's fallback (`500`) when the throw declares neither. The code stays `OAUTH_REGISTER_FAILED` (already in the ledger; no new code). `details.upstreamError` still carries the vendor's discriminator. - **Effect, measured on the real stack.** `web` + `http://localhost:4001/callback` or `http://127.0.0.1:4001/callback`: `500` → `400` (`invalid_redirect_uri`). No session: `500` → `401`. A storage fault while persisting the client: still `500`. Body-schema refusal (`http://app.example.com/callback`): still `400`. `https://app.example.com/callback`: still `200`. ## Tests New `packages/plugins/plugin-auth/src/oauth-register-refusal-status.test.ts`. It runs real better-auth with the OIDC provider on, behind the plugin's real route registration on a real Hono app, using the `in-process-session-renewal.pin.test.ts` harness. Each refusal asserts the ADR-0112 envelope (`code` + `status`) and `details.upstreamError` where the vendor sends one, and asserts that no `sys_oauth_application` row was written. - A precondition asserts that the fixture's refusal is a **named-status** `APIError` (`status: 'BAD_REQUEST'`, `statusCode: 400`). If the vendor ever switched to a numeric `status`, the pins below would pass on the old door too, and this precondition says so. - **Ablation, leg 1** (put the old read back: `const status = typeof err?.status === 'number' ? err.status : 500;`). The mutation was applied with `scripts/ablation-replace.mjs`, which confirmed it on disk (anchor 1 → 0, replacement 0 → 1, blob `03e026dea` → `a6bb1a7f3`). Result: `Tests 3 failed | 14 passed (17)`. The failures were `expected 500 to be 400` (localhost), `expected 500 to be 400` (127.0.0.1) and `expected 500 to be 401` (no session). Restored: blob equal to HEAD, `git diff HEAD` empty. - **Ablation, leg 2** (unknown throw falls back to 400: `resolveThrownHttpError(err, 400)`). Disk-verified the same way (blob `03e026dea` → `16aa6cab0`). Result: `Tests 1 failed | 16 passed (17)` with `expected 400 to be 500` on the storage-fault case. Restored the same way. - Both legs went red in the direction predicted and nothing else moved. Both ran on head `5dc8add4f`, the commit that carries the fix. The merge of `main` that followed left both files byte-identical (blobs `03e026dea` and `8b2336493`). **Head `622e30aa9`** is the merge of `main` at `5cb6e6e0a`. The incoming commits touch none of this diff's files, and `packages/spec` `check:generated` is all up to date after the rebuild. On that head: - `pnpm --filter @objectstack/plugin-auth exec vitest run`: `Test Files 169 passed (169)`, `Tests 3241 passed | 10 skipped (3251)`. - `pnpm --filter @objectstack/plugin-auth typecheck`: exit 0, with `check:test-typecheck: OK`. - The derived local gate union (`node scripts/pm/dispatch-gates.mjs --commands`, 72 commands) ran with every exit code captured before any pipe: 72 of 72 exit 0. The `--ran` reconciliation printed `72 derived, 72 run, 0 NOT-MEASURED, 0 UNRUN`. ## Half 2 — measured, not changed (for the decision card) - **The vendor's predicate.** This is `validateClientRedirectUri`, `@better-auth/oauth-provider@1.7.3`. A `web` client is refused unless the URI is `https:` **and** the host is not loopback (`localhost` or a loopback IP). The message "web clients require https redirect URIs on non-loopback hosts" states that conjunction. It does not exempt loopback hosts. Measured through this door, every loopback form is refused for `web`: `http://localhost`, `http://localhost:4001`, `http://127.0.0.1:4001`, `http://[::1]:4001`, and also `https://localhost` and `https://127.0.0.1`. Port and IP family make no difference. A `native` client may use `http` on exactly `localhost`, `127.0.0.1` or `[::1]`, with any port. Called directly with `application_type: 'native'`, the vendor accepted `http://localhost:4001/callback` and `http://127.0.0.1:4001/callback`. The predicate takes no options, so no configuration plugin-auth passes (trusted origins, dev mode, dynamic client registration) governs it. - **The door drops the Console's Application Type.** The form's `type` select (web / native / user-agent-based) is forwarded as the body key `type`. The installed vendor's `/oauth2/create-client` body schema declares no `type`. It declares `application_type: 'web' | 'native'`. So the key is stripped, and every registration is judged and stored as `web`. Measured: `type: 'native'` + `http://127.0.0.1:4001/callback` is refused with the web rule. `type: 'native'` + an `https` URI is stored with `application_type: "web"`. - **The field help.** "One URL per line. Must use https:// in production." (`packages/platform-objects`, `sys_oauth_application` create action) does not match the measured rule for `web`: https always, never a loopback host. The options, analysed on the four axes, are in the dev report on the card. This PR changes none of them. ## Acceptance notes - Not changed here, and noted without a card: on a server fault the door answers with the thrown error's raw `message` (in the test, `storage fault (test)`) and logs nothing on that branch. This is the existing behaviour and out of this card's scope. - `test:repo` (vitest `repo` project) was not run locally; it is CI's (`Test Core`). --- _Generated by [Claude Code](https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent df8bb40 commit 950f949

3 files changed

Lines changed: 227 additions & 2 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
fix(plugin-auth): a refused OAuth application registration answers the status the refusal declared, not 500 (#24773)
6+
7+
Clause-②: no
8+
9+
`POST /api/v1/auth/sys-oauth-application/register` answered most refusals from better-auth as `500 OAUTH_REGISTER_FAILED`. better-auth raises each refusal as an `APIError` whose HTTP status is the number `statusCode` (`400`) and the name `status` (`'BAD_REQUEST'`). The route read only a numeric `status`, so a named refusal fell through to 500. Two examples are a redirect URI the provider refuses, such as `http://localhost:4001/callback` on a `web` client (`invalid_redirect_uri`), and a request with no session (`UNAUTHORIZED`).
10+
11+
- **Now.** The route answers the refusal's own status: `400` for the redirect-URI refusal and `401` for the missing session. The code stays `OAUTH_REGISTER_FAILED`, and `error.details.upstreamError` still carries the provider's discriminator (`invalid_redirect_uri`).
12+
- **Unchanged.** A throw that declares no status, such as a storage fault while the client is persisted, still answers `500`. The body-schema refusal keeps answering `400`. Which redirect URIs a client may register is unchanged: a `web` client still needs an `https` URI on a non-loopback host.

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
} from '@objectstack/platform-objects/apps';
3030
import { SysOrganizationDetailPage, SysUserDetailPage } from '@objectstack/platform-objects/pages';
3131
import { PLATFORM_OWNER_EMAIL_ENV, resolvePlatformOwnerEmail, resolveTenancyPosture } from '@objectstack/types';
32-
import { inProcessSessionReadInput } from '@objectstack/types';
32+
import { inProcessSessionReadInput, resolveThrownHttpError } from '@objectstack/types';
3333
import { postureEnforcesWall, type OrgScopingEntitlement } from '@objectstack/spec/security';
3434
import type { AuthSessionApi, IDataEngine, IEmailService, II18nService, IObjectQLEngine, ISmsService } from '@objectstack/spec/contracts';
3535
import {
@@ -3624,7 +3624,17 @@ export class AuthPlugin implements Plugin {
36243624
...inProcessSessionReadInput(c.req.raw.headers),
36253625
});
36263626
} catch (err: any) {
3627-
const status = typeof err?.status === 'number' ? err.status : 500;
3627+
// [#24773] Answer the status the refusal DECLARED, by the repo's one
3628+
// rule for a thrown error's status (`resolveThrownHttpError`).
3629+
// better-call's `APIError` — every refusal better-auth raises —
3630+
// carries its status twice: as the NUMBER `statusCode` (400) and as
3631+
// the NAME `status` ('BAD_REQUEST'). This door used to read only a
3632+
// numeric `status`, so each named refusal answered 500 — an
3633+
// `invalid_redirect_uri` redirect URI, a request with no session
3634+
// (`UNAUTHORIZED`) — while the body-schema `ValidationError`, whose
3635+
// `status` is the number 400, answered 400 from the same door. A
3636+
// throw that declares no status is a server fault and stays 500.
3637+
const { status } = resolveThrownHttpError(err, 500);
36283638
// better-auth's `body.error` is an arbitrary upstream string, so it
36293639
// cannot be `error.code` — that field is a closed set (ADR-0112) and
36303640
// an unbounded source defeats it. Keep the code ours and carry the
Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#24773] `POST /api/v1/auth/sys-oauth-application/register` answers a
5+
* refused registration with the status the refusal DECLARED — a client error
6+
* is a coded 4xx (`OAUTH_REGISTER_FAILED`, ADR-0112), never 500 — and a throw
7+
* that declares no status stays 500.
8+
*
9+
* The defect, measured against the installed better-auth: every refusal
10+
* better-auth raises is better-call's `APIError`, which carries its status
11+
* twice — the NUMBER `statusCode` (400) and the NAME `status` ('BAD_REQUEST').
12+
* The wrapper read only a numeric `status`, so each named refusal answered 500:
13+
* an `http://localhost` redirect URI on a `web` client (the vendor's
14+
* `invalid_redirect_uri`), and a request with no session (`UNAUTHORIZED`).
15+
* The body-schema `ValidationError`, whose `status` IS the number 400, answered
16+
* 400 from the same door — one door, two answers for the same class.
17+
*
18+
* This file pins the STATUS only. Which redirect URIs a client may register is
19+
* the vendor's acceptance rule and is not changed here: the loopback refusals
20+
* below are the vendor's own answer, used as the fixture's named-status
21+
* refusal.
22+
*
23+
* Driven against REAL better-auth (the installed version, OIDC provider on)
24+
* behind the plugin's REAL route registration on a real Hono app — the
25+
* `in-process-session-renewal.pin.test.ts` harness: a real `AuthManager` over
26+
* the shared in-memory engine. Each refusal asserts the ADR-0112 envelope
27+
* (`code` + `status`), the vendor's discriminator in `details`, and that no
28+
* `sys_oauth_application` row was written.
29+
*/
30+
31+
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
32+
import { Hono } from 'hono';
33+
import { envelopeViolations } from '@objectstack/spec/api';
34+
import type { PluginContext } from '@objectstack/core';
35+
import { AuthManager } from './auth-manager';
36+
import { AuthPlugin } from './auth-plugin';
37+
import { createMemoryEngine } from './impersonation-bearer-rotation.test';
38+
39+
const SECRET = 'test-secret-at-least-32-chars-long!!';
40+
const PASSWORD = 'S3cure!Passw0rd-24773';
41+
const EMAIL = 'owner.24773@example.com';
42+
const ORIGIN = 'http://localhost:3000';
43+
const BASE = '/api/v1/auth';
44+
const DOOR = '/sys-oauth-application/register';
45+
const CLIENTS = 'sys_oauth_application';
46+
47+
const mockCtx = (): PluginContext =>
48+
({
49+
registerService: vi.fn(),
50+
getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)),
51+
getServices: vi.fn(() => new Map()),
52+
hook: vi.fn(),
53+
trigger: vi.fn(),
54+
logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() },
55+
getKernel: vi.fn(),
56+
}) as unknown as PluginContext;
57+
58+
let engine: ReturnType<typeof createMemoryEngine>;
59+
let manager: AuthManager;
60+
let app: Hono;
61+
let cookie: string;
62+
63+
const clientRows = (): number => (engine.tables.get(CLIENTS) ?? []).length;
64+
65+
const register = (body: unknown, withSession = true) =>
66+
app.request(`${ORIGIN}${BASE}${DOOR}`, {
67+
method: 'POST',
68+
headers: { 'content-type': 'application/json', origin: ORIGIN, ...(withSession ? { cookie } : {}) },
69+
body: JSON.stringify(body),
70+
});
71+
72+
type Answer = {
73+
success: boolean;
74+
data?: { client?: { client_id?: unknown } };
75+
error?: { code: string; message: string; details?: { upstreamError?: string } };
76+
};
77+
78+
/** The refusal envelope: ADR-0112 `code` + `status`, and nothing persisted. */
79+
async function expectRefused(res: Response, status: number, rowsBefore: number, upstreamError?: string) {
80+
const body = (await res.json()) as Answer;
81+
expect(res.status, JSON.stringify(body)).toBe(status);
82+
expect(envelopeViolations(body)).toEqual([]);
83+
expect(body.success).toBe(false);
84+
expect(body.error?.code).toBe('OAUTH_REGISTER_FAILED');
85+
if (upstreamError) expect(body.error?.details?.upstreamError).toBe(upstreamError);
86+
expect(clientRows(), 'a refused registration wrote a client row').toBe(rowsBefore);
87+
}
88+
89+
beforeAll(async () => {
90+
vi.spyOn(console, 'warn').mockImplementation(() => {});
91+
vi.spyOn(console, 'error').mockImplementation(() => {});
92+
93+
engine = createMemoryEngine();
94+
manager = new AuthManager({
95+
secret: SECRET,
96+
baseUrl: ORIGIN,
97+
dataEngine: engine,
98+
plugins: { oidcProvider: true },
99+
} as unknown as ConstructorParameters<typeof AuthManager>[0]);
100+
101+
const direct = (path: string, body: unknown) =>
102+
manager.handleRequest(
103+
new Request(`${ORIGIN}${BASE}${path}`, {
104+
method: 'POST',
105+
headers: { 'Content-Type': 'application/json' },
106+
body: JSON.stringify(body),
107+
}),
108+
);
109+
const signUp = await direct('/sign-up/email', { email: EMAIL, password: PASSWORD, name: 'Client Owner' });
110+
expect(signUp.status, `sign-up: ${await signUp.clone().text()}`).toBe(200);
111+
const signIn = await direct('/sign-in/email', { email: EMAIL, password: PASSWORD });
112+
expect(signIn.status, `sign-in: ${await signIn.clone().text()}`).toBe(200);
113+
const staged = signIn.headers.getSetCookie().find((c) => /(?:^|\.)session_token=/.test(c.split(';')[0]));
114+
if (!staged) throw new Error('sign-in staged no session cookie');
115+
cookie = staged.split(';')[0];
116+
117+
app = new Hono();
118+
const ctx = mockCtx();
119+
const plugin = new AuthPlugin({ secret: SECRET });
120+
await plugin.init(ctx);
121+
(plugin as unknown as { authManager: AuthManager }).authManager = manager;
122+
(plugin as unknown as {
123+
registerAuthRoutes(server: unknown, ctx: PluginContext): void;
124+
}).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx);
125+
});
126+
127+
afterAll(() => vi.restoreAllMocks());
128+
129+
describe('[#24773] precondition — the fixture refusal is a NAMED-status APIError', () => {
130+
it('better-auth refuses a loopback redirect URI on a web client with status "BAD_REQUEST" / statusCode 400', async () => {
131+
// Without this, a vendor that switched to a numeric `status` would let the
132+
// pins below pass on the pre-fix wrapper too, and prove nothing.
133+
const api = (await manager.getApi()) as unknown as {
134+
createOAuthClient(input: unknown): Promise<unknown>;
135+
};
136+
const thrown = await api
137+
.createOAuthClient({
138+
body: { client_name: 'precondition', redirect_uris: ['http://localhost:4001/callback'], application_type: 'web' },
139+
headers: new Headers({ cookie }),
140+
})
141+
.then(() => undefined, (e: unknown) => e as { name?: unknown; status?: unknown; statusCode?: unknown; body?: { error?: unknown } });
142+
expect(thrown, 'the vendor accepted the fixture URI — nothing below is a refusal').toBeDefined();
143+
expect(thrown?.name).toBe('APIError');
144+
expect(thrown?.status).toBe('BAD_REQUEST');
145+
expect(thrown?.statusCode).toBe(400);
146+
expect(thrown?.body?.error).toBe('invalid_redirect_uri');
147+
});
148+
});
149+
150+
describe('[#24773] a refusal answers the status it declared, never 500', () => {
151+
it('http://localhost redirect URI on a web client: 400 OAUTH_REGISTER_FAILED, invalid_redirect_uri', async () => {
152+
const before = clientRows();
153+
const res = await register({ name: 'Local Dev', redirectURLs: 'http://localhost:4001/callback', type: 'web' });
154+
await expectRefused(res, 400, before, 'invalid_redirect_uri');
155+
});
156+
157+
it('http://127.0.0.1 redirect URI on a web client: 400 OAUTH_REGISTER_FAILED, invalid_redirect_uri', async () => {
158+
const before = clientRows();
159+
const res = await register({ name: 'Local Dev', redirectURLs: 'http://127.0.0.1:4001/callback', type: 'web' });
160+
await expectRefused(res, 400, before, 'invalid_redirect_uri');
161+
});
162+
163+
it('no session: the vendor\'s UNAUTHORIZED answers 401 OAUTH_REGISTER_FAILED', async () => {
164+
const before = clientRows();
165+
const res = await register({ name: 'Anonymous', redirectURLs: 'https://app.example.com/callback' }, false);
166+
await expectRefused(res, 401, before);
167+
});
168+
169+
it('preserved: the body-schema refusal (numeric status) still answers 400', async () => {
170+
const before = clientRows();
171+
const res = await register({ name: 'Plain HTTP', redirectURLs: 'http://app.example.com/callback' });
172+
await expectRefused(res, 400, before);
173+
});
174+
});
175+
176+
describe('[#24773] a throw that declares no status is a server fault: 500', () => {
177+
it('a storage fault while persisting the client answers 500, not a client error', async () => {
178+
const before = clientRows();
179+
const insert = engine.insert;
180+
engine.insert = async (name: string, data: unknown) => {
181+
if (name === CLIENTS) throw new Error('storage fault (test)');
182+
return insert(name, data);
183+
};
184+
try {
185+
const res = await register({ name: 'Fault', redirectURLs: 'https://app.example.com/callback' });
186+
await expectRefused(res, 500, before);
187+
} finally {
188+
engine.insert = insert;
189+
}
190+
});
191+
});
192+
193+
describe('[#24773] control — an accepted registration', () => {
194+
it('https redirect URI on a web client: 200 with the minted client, one row written', async () => {
195+
const before = clientRows();
196+
const res = await register({ name: 'Production', redirectURLs: 'https://app.example.com/callback', type: 'web' });
197+
const body = (await res.json()) as Answer;
198+
expect(res.status, JSON.stringify(body)).toBe(200);
199+
expect(envelopeViolations(body)).toEqual([]);
200+
expect(typeof body.data?.client?.client_id).toBe('string');
201+
expect(clientRows()).toBe(before + 1);
202+
});
203+
});

0 commit comments

Comments
 (0)