Skip to content

Commit fd686be

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-24773-oauth-form-types
2 parents 2dc0745 + fbb42d9 commit fd686be

3 files changed

Lines changed: 379 additions & 3 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
fix(plugin-auth): the Console's OAuth Application Type reaches the provider as `application_type`, so a Native client can register a loopback redirect URI (#24773)
6+
7+
Clause-②: no — the declared type now takes effect under the OAuth provider's own redirect-URI rule, an authentication behaviour the maintainer ruled on; nothing published widens.
8+
9+
`POST /api/v1/auth/sys-oauth-application/register` (the Setup → OAuth Applications form) forwarded the form's Application Type as `type`. The provider's `/oauth2/create-client` body declares `application_type` (`web` | `native`) and no `type`, so the value was dropped and every client was judged and stored as `web`. A `native` client therefore could not register a loopback redirect URI such as `http://localhost:4001/callback`, and its `sys_oauth_application.type` read `web`.
10+
11+
- **Now.** `web` and `native` are forwarded as `application_type`, and the provider's own rule decides. A `native` client may use http only on exactly `localhost`, `127.0.0.1` or `[::1]` (any port), and is stored as `native`. A `web` client still needs an https URI on a non-loopback host.
12+
- **Unchanged, with a warning.** Any other type, including the form's `user-agent-based` and `public`, has no `application_type` in the provider. It keeps today's outcome: registered and judged as `web`, with a client secret. The server now logs one warning per such registration, naming the value. To use a loopback redirect URI, register the client as `native`.
13+
- **Unchanged.** No type at all is `web`. Refusals keep the codes and statuses they had: `400 OAUTH_REGISTER_FAILED`, with `error.details.upstreamError` `invalid_redirect_uri` when the redirect-URI rule refuses.

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

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3637,8 +3637,37 @@ export class AuthPlugin implements Plugin {
36373637
return c.json({ success: false, error: { code: 'INVALID_REQUEST', message: 'redirectURLs must contain at least one URL' } }, 400);
36383638
}
36393639

3640-
const allowedTypes = new Set(['web', 'native', 'user-agent-based']);
3641-
const safeType = typeof type === 'string' && allowedTypes.has(type) ? type : 'web';
3640+
// [#24773] The form's Application Type reaches the provider as the OIDC
3641+
// `application_type` its `/oauth2/create-client` body declares (`web` |
3642+
// `native`). It was forwarded as `type`, a member that body does not
3643+
// declare, so the provider stripped it and judged and stored every
3644+
// client as `web`: a `native` client could not register the RFC 8252
3645+
// loopback redirect URI the provider grants native clients. The
3646+
// provider's own rule is the one authority — `web` needs https on a
3647+
// non-loopback host, `native` may use http only on exactly `localhost`,
3648+
// `127.0.0.1` or `[::1]` — and nothing here re-judges a URI.
3649+
//
3650+
// Any other value (the form's `user-agent-based` and `public`, which
3651+
// have no `application_type`, or anything else) keeps the outcome it
3652+
// always had: registered and judged as `web`. Refusing it would narrow
3653+
// what this door accepts, and such a narrowing ships its loud half
3654+
// first (the standing post-GA rule, #6015 `6292005854`): one warning
3655+
// per registration, naming the value. No type at all is the
3656+
// provider's own default, `web`, and says nothing.
3657+
let applicationType: 'web' | 'native' = 'web';
3658+
if (type === 'web' || type === 'native') {
3659+
applicationType = type;
3660+
} else if (type !== undefined && type !== null) {
3661+
const requestedType = typeof type === 'string' ? type.slice(0, 64) : `(${typeof type})`;
3662+
ctx.logger.warn(
3663+
`[AuthPlugin] sys-oauth-application/register: application type ${JSON.stringify(requestedType)} is not ` +
3664+
"one the OAuth provider judges (it knows 'web' and 'native'), so this client is registered and judged " +
3665+
"as 'web': https redirect URIs on non-loopback hosts only, and a client secret is issued. Register it " +
3666+
"as 'web', or as 'native' to use an http://localhost, http://127.0.0.1 or http://[::1] redirect URI. " +
3667+
'A future major version refuses this value.',
3668+
{ requestedType, registeredAs: 'web' },
3669+
);
3670+
}
36423671

36433672
const authApi: any = await this.authManager!.getApi();
36443673
if (!authApi?.createOAuthClient) {
@@ -3658,7 +3687,7 @@ export class AuthPlugin implements Plugin {
36583687
body: {
36593688
client_name: name.trim(),
36603689
redirect_uris: redirectUris,
3661-
type: safeType,
3690+
application_type: applicationType,
36623691
},
36633692
...inProcessSessionReadInput(c.req.raw.headers),
36643693
});
Lines changed: 334 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,334 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#24773] The Console's Application Type reaches the OAuth provider as the
5+
* OIDC `application_type` the provider judges and stores.
6+
*
7+
* The defect, measured against the installed `@better-auth/oauth-provider`:
8+
* `POST /api/v1/auth/sys-oauth-application/register` forwarded the form's
9+
* choice as `type`. The provider's `/oauth2/create-client` body declares
10+
* `application_type` (`web` | `native`) and no `type`, so the member was
11+
* stripped and every registration was judged and stored as `web`. A `native`
12+
* client could not register the RFC 8252 loopback redirect URI the provider
13+
* grants native clients, and its row read `web`.
14+
*
15+
* What the door does now:
16+
* - `web` and `native` are forwarded as `application_type`, and the
17+
* provider's own rule applies unchanged: `web` needs https on a non-loopback
18+
* host; `native` may use http only on exactly `localhost`, `127.0.0.1` or
19+
* `[::1]`. Nothing in the door re-judges a URI.
20+
* - Any other value (the form's `user-agent-based` and `public`, which have no
21+
* `application_type`, or anything else) keeps the outcome it always had:
22+
* registered, judged and stored as `web`. It is NOT refused — that would
23+
* narrow what the door accepts — and the server logs exactly ONE warning
24+
* per registration, naming the value.
25+
* - No type at all is the provider's default, `web`, with no warning.
26+
*
27+
* Driven against REAL better-auth (OIDC provider on) behind the plugin's REAL
28+
* route registration on a real Hono app, over a REAL ObjectQL engine on SQLite
29+
* with the auth manifest's own objects. "Stored as" is read from the
30+
* `sys_oauth_application.type` column at driver level (the column the
31+
* provider's `applicationType` maps onto), never from the provider's answer.
32+
*/
33+
34+
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
35+
import { Hono } from 'hono';
36+
import { ObjectQL } from '@objectstack/objectql';
37+
import { SqlDriver } from '@objectstack/driver-sql';
38+
import { envelopeViolations } from '@objectstack/spec/api';
39+
import type { PluginContext } from '@objectstack/core';
40+
import { AuthManager } from './auth-manager';
41+
import { AuthPlugin } from './auth-plugin';
42+
import { authIdentityObjects } from './manifest';
43+
44+
const SECRET = 'test-secret-at-least-32-chars-long-24773';
45+
const PASSWORD = 'S3cure!Passw0rd-24773-type';
46+
const EMAIL = 'owner.type.24773@example.com';
47+
const ORIGIN = 'http://localhost:3000';
48+
const BASE = '/api/v1/auth';
49+
const DOOR = '/sys-oauth-application/register';
50+
const CLIENTS = 'sys_oauth_application';
51+
52+
const LOOPBACK = 'http://localhost:4001/callback';
53+
const LOOPBACK_IP = 'http://127.0.0.1:4001/callback';
54+
const PLAIN_HTTP = 'http://app.example.com/callback';
55+
const HTTPS = 'https://app.example.com/callback';
56+
57+
/**
58+
* The three authorization objects a real sign-in makes core's
59+
* `resolveUserAuthzGrants` read, which the auth manifest does not declare —
60+
* spelled locally with only the columns the resolver reads, as
61+
* `account-issuer-upgrade-path.test.ts` does, so the sign-in below exercises
62+
* its resolver leg instead of reading missing tables.
63+
*/
64+
const AUTHZ_OBJECTS = [
65+
{
66+
name: 'sys_user_position',
67+
label: 'User Position',
68+
fields: {
69+
id: { name: 'id', type: 'text' as const, primaryKey: true },
70+
user_id: { name: 'user_id', type: 'text' as const },
71+
position: { name: 'position', type: 'text' as const },
72+
organization_id: { name: 'organization_id', type: 'text' as const },
73+
},
74+
},
75+
{
76+
name: 'sys_user_permission_set',
77+
label: 'User Permission Set',
78+
fields: {
79+
id: { name: 'id', type: 'text' as const, primaryKey: true },
80+
user_id: { name: 'user_id', type: 'text' as const },
81+
permission_set_id: { name: 'permission_set_id', type: 'text' as const },
82+
organization_id: { name: 'organization_id', type: 'text' as const },
83+
},
84+
},
85+
{
86+
name: 'sys_position',
87+
label: 'Position',
88+
fields: {
89+
id: { name: 'id', type: 'text' as const, primaryKey: true },
90+
name: { name: 'name', type: 'text' as const },
91+
label: { name: 'label', type: 'text' as const },
92+
active: { name: 'active', type: 'boolean' as const },
93+
organization_id: { name: 'organization_id', type: 'text' as const },
94+
},
95+
},
96+
];
97+
98+
const warn = vi.fn();
99+
const mockCtx = (): PluginContext =>
100+
({
101+
registerService: vi.fn(),
102+
getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)),
103+
getServices: vi.fn(() => new Map()),
104+
hook: vi.fn(),
105+
trigger: vi.fn(),
106+
logger: { info: vi.fn(), error: vi.fn(), warn, debug: vi.fn() },
107+
getKernel: vi.fn(),
108+
}) as unknown as PluginContext;
109+
110+
let engine: ObjectQL;
111+
let manager: AuthManager;
112+
let app: Hono;
113+
let cookie: string;
114+
115+
type Row = Record<string, unknown>;
116+
117+
/** Every stored client, read at DRIVER level — below every engine read mask. */
118+
async function storedClients(): Promise<Row[]> {
119+
const driver = (
120+
engine as unknown as { getDriver(o: string): { find(o: string, q: unknown): Promise<unknown> } }
121+
).getDriver(CLIENTS);
122+
const found = await driver.find(CLIENTS, { where: {} });
123+
return (Array.isArray(found) ? found : [found]).filter(Boolean) as Row[];
124+
}
125+
126+
async function storedClient(clientId: unknown): Promise<Row> {
127+
const rows = (await storedClients()).filter((row) => row.client_id === clientId);
128+
expect(rows, `exactly one stored row for client ${String(clientId)}`).toHaveLength(1);
129+
return rows[0];
130+
}
131+
132+
type Answer = {
133+
success: boolean;
134+
data?: { client?: { client_id?: unknown; application_type?: unknown } };
135+
error?: { code: string; message: string; details?: { upstreamError?: string } };
136+
};
137+
138+
/**
139+
* One registration through the door. Returns the answer and the warnings the
140+
* plugin logged while serving THIS request (the mock is cleared first).
141+
*/
142+
async function register(body: Record<string, unknown>) {
143+
warn.mockClear();
144+
const res = await app.request(`${ORIGIN}${BASE}${DOOR}`, {
145+
method: 'POST',
146+
headers: { 'content-type': 'application/json', origin: ORIGIN, cookie },
147+
body: JSON.stringify(body),
148+
});
149+
const json = (await res.json()) as Answer;
150+
return { status: res.status, body: json, warnings: [...warn.mock.calls] };
151+
}
152+
153+
/** Accepted: 200, a minted client, its row stored with `type`. */
154+
async function expectStoredAs(answer: Awaited<ReturnType<typeof register>>, storedType: 'web' | 'native') {
155+
expect(answer.status, JSON.stringify(answer.body)).toBe(200);
156+
expect(envelopeViolations(answer.body)).toEqual([]);
157+
expect(answer.body.success).toBe(true);
158+
const clientId = answer.body.data?.client?.client_id;
159+
expect(typeof clientId).toBe('string');
160+
const row = await storedClient(clientId);
161+
expect(row.type, `stored row: ${JSON.stringify(row)}`).toBe(storedType);
162+
return row;
163+
}
164+
165+
/**
166+
* Refused by the provider: ADR-0112 `code` + `status`, nothing stored. A
167+
* refusal from the provider's redirect-URI rule names `invalid_redirect_uri`
168+
* in `details`; one from its body schema (a numeric-status `ValidationError`)
169+
* carries no upstream discriminator.
170+
*/
171+
async function expectRefused(
172+
answer: Awaited<ReturnType<typeof register>>,
173+
rowsBefore: number,
174+
refusedBy: 'redirect-uri-rule' | 'body-schema' = 'redirect-uri-rule',
175+
) {
176+
expect(answer.status, JSON.stringify(answer.body)).toBe(400);
177+
expect(envelopeViolations(answer.body)).toEqual([]);
178+
expect(answer.body.success).toBe(false);
179+
expect(answer.body.error?.code).toBe('OAUTH_REGISTER_FAILED');
180+
expect(answer.body.error?.details?.upstreamError).toBe(
181+
refusedBy === 'redirect-uri-rule' ? 'invalid_redirect_uri' : undefined,
182+
);
183+
expect((await storedClients()).length, 'a refused registration wrote a client row').toBe(rowsBefore);
184+
}
185+
186+
beforeAll(async () => {
187+
vi.spyOn(console, 'warn').mockImplementation(() => {});
188+
vi.spyOn(console, 'error').mockImplementation(() => {});
189+
190+
engine = new ObjectQL();
191+
engine.registerDriver(
192+
new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }),
193+
true,
194+
);
195+
await engine.init();
196+
for (const object of authIdentityObjects) {
197+
engine.registry.registerObject(object as never, '@objectstack/plugin-auth');
198+
}
199+
for (const object of AUTHZ_OBJECTS) {
200+
engine.registry.registerObject(object as never, '@objectstack/plugin-security');
201+
}
202+
await engine.syncSchemas();
203+
204+
manager = new AuthManager({
205+
secret: SECRET,
206+
baseUrl: ORIGIN,
207+
dataEngine: engine as never,
208+
plugins: { oidcProvider: true },
209+
} as unknown as ConstructorParameters<typeof AuthManager>[0]);
210+
211+
const direct = (path: string, body: unknown) =>
212+
manager.handleRequest(
213+
new Request(`${ORIGIN}${BASE}${path}`, {
214+
method: 'POST',
215+
headers: { 'Content-Type': 'application/json', origin: ORIGIN },
216+
body: JSON.stringify(body),
217+
}),
218+
);
219+
const signUp = await direct('/sign-up/email', { email: EMAIL, password: PASSWORD, name: 'Client Owner' });
220+
expect(signUp.status, `sign-up: ${await signUp.clone().text()}`).toBe(200);
221+
const signIn = await direct('/sign-in/email', { email: EMAIL, password: PASSWORD });
222+
expect(signIn.status, `sign-in: ${await signIn.clone().text()}`).toBe(200);
223+
const staged = signIn.headers.getSetCookie().find((c) => /(?:^|\.)session_token=/.test(c.split(';')[0]));
224+
if (!staged) throw new Error('sign-in staged no session cookie');
225+
cookie = staged.split(';')[0];
226+
227+
app = new Hono();
228+
const ctx = mockCtx();
229+
const plugin = new AuthPlugin({ secret: SECRET });
230+
await plugin.init(ctx);
231+
(plugin as unknown as { authManager: AuthManager }).authManager = manager;
232+
(plugin as unknown as {
233+
registerAuthRoutes(server: unknown, ctx: PluginContext): void;
234+
}).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx);
235+
}, 60_000);
236+
237+
afterAll(async () => {
238+
vi.restoreAllMocks();
239+
await (engine as unknown as { destroy?(): Promise<void> })?.destroy?.();
240+
});
241+
242+
describe('[#24773] precondition — the provider judges `application_type`, and strips `type`', () => {
243+
// Without these, a provider that started reading `type` (or stopped granting
244+
// native clients the loopback rule) would let the door pins below pass or
245+
// fail for a reason that is not the door.
246+
const createClient = async (body: Record<string, unknown>) => {
247+
const api = (await manager.getApi()) as unknown as { createOAuthClient(input: unknown): Promise<unknown> };
248+
return api
249+
.createOAuthClient({ body: { client_name: 'precondition', ...body }, headers: new Headers({ cookie }) })
250+
.then(
251+
(client) => ({ client: client as { client_id?: unknown }, refusal: undefined }),
252+
(e: unknown) => ({ client: undefined, refusal: e as { body?: { error?: unknown } } }),
253+
);
254+
};
255+
256+
it('`type: "native"` is stripped: a loopback redirect URI is refused under the web rule', async () => {
257+
const { client, refusal } = await createClient({ type: 'native', redirect_uris: [LOOPBACK] });
258+
expect(client, 'the provider honoured `type` — the defect this file pins no longer exists as measured').toBeUndefined();
259+
expect(refusal?.body?.error).toBe('invalid_redirect_uri');
260+
});
261+
262+
it('`application_type: "native"` is honoured: the same loopback URI is accepted and stored as native', async () => {
263+
const { client, refusal } = await createClient({ application_type: 'native', redirect_uris: [LOOPBACK] });
264+
expect(refusal, JSON.stringify(refusal?.body)).toBeUndefined();
265+
expect((await storedClient(client?.client_id)).type).toBe('native');
266+
});
267+
});
268+
269+
describe('[#24773] web and native reach the provider as `application_type`', () => {
270+
it('web + http://localhost: refused by the provider\'s web rule — 400 OAUTH_REGISTER_FAILED, nothing stored, no warning', async () => {
271+
const before = (await storedClients()).length;
272+
const answer = await register({ name: 'Local Web', redirectURLs: LOOPBACK, type: 'web' });
273+
await expectRefused(answer, before);
274+
expect(answer.warnings).toEqual([]);
275+
});
276+
277+
it.each([LOOPBACK, LOOPBACK_IP])('native + %s: accepted and stored as native, no warning', async (uri) => {
278+
const answer = await register({ name: 'Local Native', redirectURLs: uri, type: 'native' });
279+
await expectStoredAs(answer, 'native');
280+
expect(answer.warnings).toEqual([]);
281+
});
282+
283+
it('native + plain http on a non-loopback host: refused (the provider\'s body schema) — 400, nothing stored', async () => {
284+
// The provider's `SafeUrlSchema` requires https except on loopback hosts,
285+
// for every application type, before its per-type rule runs.
286+
const before = (await storedClients()).length;
287+
const answer = await register({ name: 'Remote Native', redirectURLs: PLAIN_HTTP, type: 'native' });
288+
await expectRefused(answer, before, 'body-schema');
289+
expect(answer.warnings).toEqual([]);
290+
});
291+
292+
it('native + http on a loopback-range host that is not exactly localhost / 127.0.0.1 / [::1]: refused by the provider\'s native rule', async () => {
293+
const before = (await storedClients()).length;
294+
const answer = await register({ name: 'Near Native', redirectURLs: 'http://127.0.0.2:4001/callback', type: 'native' });
295+
await expectRefused(answer, before);
296+
expect(answer.warnings).toEqual([]);
297+
});
298+
299+
it('web + https on a non-loopback host: accepted and stored as web, no warning', async () => {
300+
const answer = await register({ name: 'Production', redirectURLs: HTTPS, type: 'web' });
301+
await expectStoredAs(answer, 'web');
302+
expect(answer.warnings).toEqual([]);
303+
});
304+
305+
it('no type: the provider\'s default, web — stored as web, no warning', async () => {
306+
const answer = await register({ name: 'Untyped', redirectURLs: HTTPS });
307+
await expectStoredAs(answer, 'web');
308+
expect(answer.warnings).toEqual([]);
309+
});
310+
});
311+
312+
describe('[#24773] a type the provider has no `application_type` for keeps today\'s outcome, with one warning', () => {
313+
const UNHONOURED = ['user-agent-based', 'public', 'spa'] as const;
314+
315+
it.each(UNHONOURED)('%s + https: accepted and stored as web; exactly one warning naming the value', async (type) => {
316+
const answer = await register({ name: `Typed ${type}`, redirectURLs: HTTPS, type });
317+
const row = await expectStoredAs(answer, 'web');
318+
// The form's value is stored nowhere on the row, and no public client
319+
// (no secret) was issued for `public`.
320+
expect(Object.entries(row).filter(([, value]) => value === type)).toEqual([]);
321+
expect(row.public === true || row.public === 1, 'the door registered a public client').toBe(false);
322+
expect(row.token_endpoint_auth_method).not.toBe('none');
323+
expect(answer.warnings).toHaveLength(1);
324+
const [, meta] = answer.warnings[0] as [string, Record<string, unknown>];
325+
expect(meta).toEqual({ requestedType: type, registeredAs: 'web' });
326+
});
327+
328+
it.each(UNHONOURED)('%s + http://localhost: judged as web — refused, nothing stored; exactly one warning', async (type) => {
329+
const before = (await storedClients()).length;
330+
const answer = await register({ name: `Local ${type}`, redirectURLs: LOOPBACK, type });
331+
await expectRefused(answer, before);
332+
expect(answer.warnings).toHaveLength(1);
333+
});
334+
});

0 commit comments

Comments
 (0)