Skip to content

Commit 30bcd4f

Browse files
committed
fix(client)!: oauth.applications.register declares only members /oauth2/create-client accepts
`name`, `scopes` and `metadata` were never honoured by the route this method posts to. The vendor body schema (`@better-auth/oauth-provider@1.7.2`) has no `catchall`, so zod strips them: the caller got HTTP 201 and a client that quietly did not have the value. Driven on the card — absent from the response, from `applications.get`, from `applications.list`, and `null` in the `sys_oauth_application` row. The two near-misses were the vendor's RECORD vocabulary, not typos: `client_name` writes the DB column named `name`, and `scope` writes the column named `scopes`. `scopes` -> `scope` is therefore not a rename — the array form is refused (`400 [body.scope] Invalid input: expected string, received array`) and only a space-joined string passes. Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 Co-authored-by: Claude <noreply@anthropic.com>
1 parent d61139f commit 30bcd4f

2 files changed

Lines changed: 277 additions & 3 deletions

File tree

packages/client/src/index.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3842,22 +3842,57 @@ export class ObjectStackClient {
38423842
*
38433843
* Returns the freshly-issued `client_id` and `client_secret`.
38443844
* The secret is only returned at creation time — store it securely.
3845+
*
3846+
* ## Why `name`, `scopes` and `metadata` are NOT declared here (#15447)
3847+
*
3848+
* They used to be, and the route silently dropped all three. Its body
3849+
* schema is `@better-auth/oauth-provider@1.7.2`'s, a zod object with no
3850+
* `catchall` — so zod's default `strip` — and none of the three is among
3851+
* its 21 members. A caller who set one got **HTTP 201 and a client that
3852+
* quietly did not have it**: no error, no receipt, nothing to notice.
3853+
* Driven end to end (real `betterAuth` + real `oauthProvider` over the
3854+
* real ObjectQL engine on a real socket, through this very client): each
3855+
* member came back absent from the response, absent from
3856+
* `applications.get`, absent from `applications.list`, and `null` in the
3857+
* `sys_oauth_application` row.
3858+
*
3859+
* A second, independent barrier stands behind the strip, so widening the
3860+
* SDK alone could never have made them arrive: the handler funnels the
3861+
* rest of the parsed body into the opaque-metadata envelope, and all
3862+
* three names are in `OPAQUE_METADATA_RESERVED_FIELDS`.
3863+
*
3864+
* ## ⚠️ They were the vendor's RECORD vocabulary, not typos
3865+
*
3866+
* The two near-misses look like misspellings of `client_name` and
3867+
* `scope` and are not — they are the names of the DB columns those two
3868+
* wire members write. Measured: `client_name: 'CTRL-…'` lands in the
3869+
* column literally named **`name`**, and `scope: 'openid profile email'`
3870+
* lands in the column literally named **`scopes`**, as a JSON array. So
3871+
* this type used to offer the record spelling and the wire spelling side
3872+
* by side, and only the wire one worked. The right prescription is the
3873+
* wire member, and for `scopes` it is not a rename: `scope` is a single
3874+
* space-delimited `string`, and posting an array is refused —
3875+
* `400 [body.scope] Invalid input: expected string, received array`.
3876+
*
3877+
* `metadata` has no reachable door at all: only the SERVER_ONLY
3878+
* `PATCH /admin/oauth2/update-client` honours it, and `better-call`'s
3879+
* router skips SERVER_ONLY endpoints, so over HTTP it answers 404 with a
3880+
* zero-byte body.
3881+
*
3882+
* Pinned by `oauth-applications-register-request-members.test.ts`.
38453883
*/
38463884
register: async (req: {
38473885
client_name?: string;
3848-
name?: string;
38493886
redirect_uris: string[];
38503887
token_endpoint_auth_method?: 'none' | 'client_secret_basic' | 'client_secret_post';
38513888
grant_types?: string[];
38523889
response_types?: string[];
38533890
client_uri?: string;
38543891
logo_uri?: string;
38553892
scope?: string;
3856-
scopes?: string[];
38573893
contacts?: string[];
38583894
tos_uri?: string;
38593895
policy_uri?: string;
3860-
metadata?: Record<string, unknown>;
38613896
}): Promise<OAuthApplicationRegistration> => {
38623897
const route = this.getRoute('auth');
38633898
// The new oauth-provider package exposes `/oauth2/create-client`
Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#15447] `oauth.applications.register` must declare only members the route
5+
* it posts to actually accepts — and must still send the ones it does, byte
6+
* for byte.
7+
*
8+
* ## Two halves, two mechanisms, and neither can do the other's job
9+
*
10+
* 1. **The removal is type-level only.** `POST /oauth2/create-client` answers
11+
* **201 either way** — that is the entire reason the defect was invisible
12+
* for as long as it was. A runtime assertion on the status, on the response
13+
* body, or on a read-back would have been green before the fix and green
14+
* after it. Only a compile-time assertion can observe a member leaving a
15+
* declared type, so `registerRequestMemberPins15447` below is compiled and
16+
* never invoked, exactly like `return-type-precision.test.ts`'s pins.
17+
* 2. **The members that survive are runtime-pinned**, because those the SDK
18+
* can still break: it serialises the caller's object straight into the
19+
* request body, so a later "helpful" translation layer (mapping `name` onto
20+
* `client_name`, joining a `scopes` array) would change what reaches the
21+
* server without changing a single type. The `it()` blocks hold the request
22+
* bytes to FULL-STRING equality for that reason — never `toContain`, which
23+
* a body carrying extra members would satisfy.
24+
*
25+
* ## What was measured, and where
26+
*
27+
* Driven on the card (issue #15447, comment 5559384773) against real
28+
* `betterAuth` + real `@better-auth/oauth-provider@1.7.2` over the real
29+
* ObjectQL engine on a real TCP socket, through the real `ObjectStackClient`.
30+
* ⛔ Nothing here re-drives that rig; these fixtures encode its verdict.
31+
*
32+
* | posted member | response | `get` | `list` | `sys_oauth_application` row |
33+
* |---|---|---|---|---|
34+
* | `client_name: 'CTRL-…'` *(control)* | present | present | present | column `name` = `'CTRL-…'` |
35+
* | `scope: 'openid profile email'` *(control)* | present | present | present | column `scopes` = `["openid","profile","email"]` |
36+
* | `name: 'PROBE-…'` | absent | absent | absent | column `name` = **null** |
37+
* | `scopes: ['openid',…]` | absent | absent | absent | column `scopes` = **null** |
38+
* | `metadata: {…}` | absent | absent | absent | column `metadata` = **null** |
39+
*
40+
* The vendor body schema has no `catchall`, so it is zod's default **strip**:
41+
* parsing a body carrying all three reports `ok: true` with
42+
* `droppedKeys: ["name","scopes","metadata"]`. A second, independent barrier
43+
* stands behind that strip — the handler funnels the parsed rest into the
44+
* opaque-metadata envelope and all three names sit in
45+
* `OPAQUE_METADATA_RESERVED_FIELDS` — so loosening the SDK alone could never
46+
* have made them arrive.
47+
*
48+
* ## ⚠️ The two near-misses are the RECORD vocabulary, not typos
49+
*
50+
* `client_name` writes the DB column literally named `name`; `scope` writes
51+
* the column literally named `scopes`. The removed members were the column
52+
* names, offered next to the wire names in the same type. That is also why
53+
* `scopes` → `scope` is not a rename: `scope` is one space-delimited string,
54+
* and the array form is driven-refused with
55+
* `400 [body.scope] Invalid input: expected string, received array`. The
56+
* space-joined case below is that prescription, pinned.
57+
*
58+
* `metadata` has no reachable door at all: only the SERVER_ONLY
59+
* `PATCH /admin/oauth2/update-client` honours it, and `better-call`'s router
60+
* skips SERVER_ONLY endpoints (driven over HTTP: 404, zero bytes).
61+
*/
62+
63+
import { describe, it, expect, expectTypeOf, vi } from 'vitest';
64+
import { ObjectStackClient } from './index';
65+
66+
const BASE = 'http://localhost:3000';
67+
const CREATE_CLIENT_URL = `${BASE}/api/v1/auth/oauth2/create-client`;
68+
69+
/** The 201 the route answers — identical before and after this card. */
70+
const REGISTERED = JSON.stringify({
71+
client_id: 'GshJvINrsShauIzjLxjKpcCtxsYjoOxP',
72+
client_secret: 'aLPpOONfpGeJymAExjukigaMaxNLvpjt',
73+
client_secret_expires_at: 0,
74+
client_id_issued_at: 1788699309,
75+
});
76+
77+
function clientCapturingRequest() {
78+
const fetchMock = vi.fn(
79+
async () =>
80+
new Response(REGISTERED, { status: 201, headers: { 'content-type': 'application/json' } }),
81+
);
82+
const client = new ObjectStackClient({ baseUrl: BASE, fetch: fetchMock as never });
83+
return { client, fetchMock };
84+
}
85+
86+
/** The one request the method under test is allowed to make. */
87+
function soleRequest(fetchMock: ReturnType<typeof clientCapturingRequest>['fetchMock']) {
88+
expect(fetchMock).toHaveBeenCalledTimes(1);
89+
return fetchMock.mock.calls[0] as unknown as [string, RequestInit];
90+
}
91+
92+
// ─────────────────────────────────────────────────────────────────────────
93+
// ① The pin — type-level, and RED on the defect rather than on the fix
94+
// ─────────────────────────────────────────────────────────────────────────
95+
96+
declare const client: ObjectStackClient;
97+
98+
/** The declared request type of the method this card narrows. */
99+
type RegisterRequest = Parameters<ObjectStackClient['oauth']['applications']['register']>[0];
100+
101+
/**
102+
* Compiled by `packages/client/tsconfig.test.json` (which includes `src/**` and
103+
* is named by `package.json`'s `typecheck` script through
104+
* `check:test-typecheck`), never invoked — every statement is an assertion tsc
105+
* evaluates, and none of them may perform a request.
106+
*
107+
* ⚠️ Both directions below are red while the defect stands, which is what
108+
* makes this a pin on the DEFECT and not on the fix:
109+
*
110+
* - the key-set equality fails because the union carried three more members;
111+
* - each `@ts-expect-error` goes UNUSED (TS2578, "Unused '@ts-expect-error'
112+
* directive") because the literal was accepted while the member was
113+
* declared.
114+
*
115+
* The key set is asserted as an EQUALITY, not as three absences, so it is also
116+
* the guard against the opposite move: re-adding any of the three under a new
117+
* spelling, or slipping in a compatibility alias member, reddens it too.
118+
*/
119+
export async function registerRequestMemberPins15447(): Promise<void> {
120+
// ── direction 1: the surviving key set, exactly ──────────────────────
121+
expectTypeOf<keyof RegisterRequest>().toEqualTypeOf<
122+
| 'client_name'
123+
| 'redirect_uris'
124+
| 'token_endpoint_auth_method'
125+
| 'grant_types'
126+
| 'response_types'
127+
| 'client_uri'
128+
| 'logo_uri'
129+
| 'scope'
130+
| 'contacts'
131+
| 'tos_uri'
132+
| 'policy_uri'
133+
>();
134+
135+
// The two wire members the route DOES honour are still declared, and still
136+
// carry the types the wire uses — `scope` a single space-delimited string,
137+
// never the array the removed `scopes` invited.
138+
expectTypeOf<RegisterRequest['client_name']>().toEqualTypeOf<string | undefined>();
139+
expectTypeOf<RegisterRequest['scope']>().toEqualTypeOf<string | undefined>();
140+
141+
// ── direction 2: the three stripped members are now REFUSED ──────────
142+
// Each suppression sits on the property line, because that is where the
143+
// excess-property check reports, and each is UNUSED while the member is
144+
// still declared.
145+
void (await client.oauth.applications.register({
146+
redirect_uris: ['https://app.example.com/cb'],
147+
// @ts-expect-error [#15447] `name` is the DB COLUMN `client_name` writes, not a wire member; the route strips it and answers 201
148+
name: 'PROBE-NAME-15447',
149+
}));
150+
void (await client.oauth.applications.register({
151+
redirect_uris: ['https://app.example.com/cb'],
152+
// @ts-expect-error [#15447] `scopes` is the DB COLUMN `scope` writes; it is stripped everywhere and is NOT a rename of `scope` — that one takes a space-joined string
153+
scopes: ['openid', 'profile', 'email'],
154+
}));
155+
void (await client.oauth.applications.register({
156+
redirect_uris: ['https://app.example.com/cb'],
157+
// @ts-expect-error [#15447] `metadata` is honoured only by the SERVER_ONLY admin update, which is not an HTTP route at all
158+
metadata: { tenant: 'acme', tier: 7 },
159+
}));
160+
}
161+
162+
// ─────────────────────────────────────────────────────────────────────────
163+
// ② The negative control — what the route DOES honour still arrives verbatim
164+
// ─────────────────────────────────────────────────────────────────────────
165+
166+
describe('#15447 oauth.applications.register — the honoured members still reach the wire', () => {
167+
it('sends `client_name` and a space-joined `scope` byte for byte', async () => {
168+
const { client: c, fetchMock } = clientCapturingRequest();
169+
await c.oauth.applications.register({
170+
client_name: 'CTRL-CLIENT-NAME-15447',
171+
scope: ['openid', 'profile', 'email'].join(' '),
172+
redirect_uris: ['https://app.example.com/cb'],
173+
});
174+
const [url, init] = soleRequest(fetchMock);
175+
expect(url).toBe(CREATE_CLIENT_URL);
176+
expect(init.method).toBe('POST');
177+
// ⛔ Full-string equality, never `toContain`: a body that also carried a
178+
// re-introduced `name`/`scopes`/`metadata`, or one the SDK had started
179+
// translating, would satisfy a containment check.
180+
expect(init.body).toBe(
181+
JSON.stringify({
182+
client_name: 'CTRL-CLIENT-NAME-15447',
183+
scope: 'openid profile email',
184+
redirect_uris: ['https://app.example.com/cb'],
185+
}),
186+
);
187+
});
188+
189+
it('is a pass-through: every surviving member arrives unchanged and nothing is added', async () => {
190+
const { client: c, fetchMock } = clientCapturingRequest();
191+
const req = {
192+
client_name: 'CTRL-CLIENT-NAME-15447',
193+
redirect_uris: ['https://app.example.com/cb'],
194+
token_endpoint_auth_method: 'client_secret_basic' as const,
195+
grant_types: ['authorization_code', 'refresh_token'],
196+
response_types: ['code'],
197+
client_uri: 'https://app.example.com',
198+
logo_uri: 'https://app.example.com/logo.png',
199+
scope: 'openid profile email',
200+
contacts: ['ops@example.com'],
201+
tos_uri: 'https://app.example.com/tos',
202+
policy_uri: 'https://app.example.com/privacy',
203+
};
204+
await c.oauth.applications.register(req);
205+
const [, init] = soleRequest(fetchMock);
206+
// The method's whole body-building step is `JSON.stringify(req)`. Holding
207+
// it to that exactly is what makes a future mapping layer — the shape
208+
// triage called "keep and honour" — a red test rather than a silent
209+
// divergence from the vendor's wire vocabulary.
210+
expect(init.body).toBe(JSON.stringify(req));
211+
});
212+
213+
it('the space-joined `scope` is what the route accepts — the array form is refused on the wire', async () => {
214+
// Driven, not invented: posting `scope: ['openid','profile']` answered
215+
// `400 [body.scope] Invalid input: expected string, received array`, while
216+
// `['openid','profile'].join(' ')` answered 201 with
217+
// `"scope":"openid profile"`. This case pins that the SDK surfaces the
218+
// refusal rather than papering over it — the reason a caller's array must
219+
// be joined at the CALL SITE and not by a translation layer here.
220+
const fetchMock = vi.fn(
221+
async () =>
222+
new Response(
223+
JSON.stringify({
224+
message: '[body.scope] Invalid input: expected string, received array',
225+
code: 'VALIDATION_ERROR',
226+
}),
227+
{ status: 400, headers: { 'content-type': 'application/json' } },
228+
),
229+
);
230+
const c = new ObjectStackClient({ baseUrl: BASE, fetch: fetchMock as never });
231+
await expect(
232+
c.oauth.applications.register({
233+
client_name: 'CTRL-CLIENT-NAME-15447',
234+
redirect_uris: ['https://app.example.com/cb'],
235+
scope: 'openid profile email',
236+
}),
237+
).rejects.toThrow(/expected string, received array/);
238+
});
239+
});

0 commit comments

Comments
 (0)