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
36 changes: 36 additions & 0 deletions .changeset/17215-oauth-register-redirect-uris-optional.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
"@objectstack/client": minor
---

fix(client): `oauth.applications.register` declares `redirect_uris` optional, matching the body schema of the route it posts to (#17215)

`ObjectStackClient.oauth.applications.register` declared `redirect_uris` **required**. `POST /api/v1/auth/oauth2/create-client` is mounted verbatim from `@better-auth/oauth-provider`, and that route's body schema declares the member **optional** — so a request the route accepts had no spelling through this SDK. The caller never got a wrong answer; they got a call they could not write.

## What changes for a caller

Nothing they have to do. Every existing call still compiles — this only *adds* spellings:

```ts
// now expressible, and accepted by the route:
await client.oauth.applications.register({ client_name: 'My App' });

// unchanged, and still the right call when you have redirect URIs:
await client.oauth.applications.register({
client_name: 'My App',
redirect_uris: ['https://app.example.com/cb'],
});
```

⛔ Not breaking in this direction — relaxing a required member to optional keeps every existing call valid. Tightening it back later would be breaking, which is why the parity is now pinned.

## Measured at runtime, not read off a `.d.ts`

The vendor body schema was re-introspected the way the card's original measurement was taken: instantiate `oauthProvider()`, walk `endpoints`, find the endpoint whose `path` is `/oauth2/create-client`, read `options.body`. At the installed **1.7.3** (the card measured 1.7.2; the package has since moved) the object still declares **21 members and every one of them is optional**, and `body.safeParse({ client_name: '…' })` succeeds with `redirect_uris` absent.

⚠️ Optional does **not** mean an empty array will do: the vendor refuses `[]`, so when the member is present it must be non-empty. Omitting it and passing `[]` are different requests and only the first is legal. Nor does it mean a client registered without redirect URIs is *usable* — it cannot complete an `authorization_code` flow. The type states what the route accepts, never that every accepted call yields a client fit for every grant; the docblock now says both.

## Why it was required, for the record

Not as a guard. It is residue from the method's first commit, which declared `client_name` required too; the same-day follow-up relaxed `client_name` and left this one behind. No comment, test, ADR or review thread ever asserted a reason for it — which is exactly why it read as a defect to the next auditor.

Nothing else on the signature moves: the other ten members are byte-identical.
23 changes: 22 additions & 1 deletion packages/client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4090,11 +4090,32 @@ export class ObjectStackClient {
* performs no such split — `redirect_uris` must arrive **pre-split**,
* one entry per URL, which is what an SDK caller holds anyway.
*
* ## ⚠️ `redirect_uris` is OPTIONAL here, and that is measured parity (#17215)
*
* It used to be the one required member on this type. It was never a
* deliberate guard — it is residue from the method's first commit, which
* declared `client_name` required too; the same-day follow-up relaxed
* `client_name` and left this one behind, and no comment, test, ADR or
* review thread ever asserted a reason for it.
*
* Re-introspected at runtime against `@better-auth/oauth-provider@1.7.3`
* — instantiate `oauthProvider()`, walk `endpoints`, read `options.body`
* — the member is `optional`, and a body omitting it entirely parses
* `ok`. All 21 members of that schema are optional.
*
* ⚠️ Optional does NOT mean `[]` will do. The vendor refuses an empty
* array, so when the member is present it must be non-empty: omitting it
* and passing `[]` are different requests, and only the first is legal.
* ⚠️ Nor does it mean a client registered without redirect URIs is
* usable — it cannot complete an `authorization_code` flow. This type
* states what the route accepts, never that every accepted call yields a
* client fit for every grant.
*
* Pinned by `oauth-applications-register-request-members.test.ts`.
*/
register: async (req: {
client_name?: string;
redirect_uris: string[];
redirect_uris?: string[];
token_endpoint_auth_method?: 'none' | 'client_secret_basic' | 'client_secret_post';
grant_types?: string[];
response_types?: string[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,55 @@ export async function registerRequestMemberPins15447(): Promise<void> {
}));
}

// ─────────────────────────────────────────────────────────────────────────
// ①b [#17215] `redirect_uris` is OPTIONAL — parity with the vendor schema
// ─────────────────────────────────────────────────────────────────────────

/**
* [#17215] The eleventh member was the one required member on this type, and
* the route it posts to declares it **optional**. Re-introspected at runtime
* against `@better-auth/oauth-provider@1.7.3` — instantiate `oauthProvider()`,
* walk `endpoints`, read `options.body` — every one of that schema's 21
* members is optional, and `body.safeParse({ client_name })` succeeds with
* `redirect_uris` absent. So a request the route accepts had no spelling here.
*
* ⚠️ This pin is type-level for the same reason `registerRequestMemberPins15447`
* is: the route answers the same way either way, and only a compile-time
* assertion can observe a member's OPTIONALITY changing. A runtime assertion
* on the request bytes cannot — `JSON.stringify` omits an absent member
* whether the type required it or not, so the body is byte-identical in both
* states and any such test is green before the fix and green after it.
*
* ⛔ The key-set equality above is deliberately NOT the guard for this:
* `keyof` is blind to optionality, so it reads the same eleven names in both
* states. That is why it keeps holding across this change, and why this needs
* its own assertion rather than relying on the one already there.
*/
export async function registerRedirectUrisOptionalPin17215(): Promise<void> {
// ── the parity assertion — red if anyone re-tightens it ──────────────
expectTypeOf<RegisterRequest['redirect_uris']>().toEqualTypeOf<string[] | undefined>();

// ── the call that was previously INEXPRESSIBLE ───────────────────────
// Before this card `redirect_uris` was required, so this did not compile at
// all. It is the whole point of the change: the vendor accepts this body.
void (await client.oauth.applications.register({ client_name: 'PROBE-17215-OMITTED' }));

// The emptiest legal call: every member of the vendor schema is optional.
void (await client.oauth.applications.register({}));

// ── and the call that always worked still does ───────────────────────
void (await client.oauth.applications.register({
client_name: 'CTRL-17215-SUPPLIED',
redirect_uris: ['https://app.example.com/cb'],
}));

// ⚠️ Optional is not "any array will do": the vendor refuses `[]`
// (`safeParse([])` fails at runtime). The TYPE cannot express non-empty, so
// this still compiles — recorded here so the next reader does not mistake
// the compiling call for a legal one.
void (await client.oauth.applications.register({ redirect_uris: [] }));
}

// ─────────────────────────────────────────────────────────────────────────
// ② The negative control — what the route DOES honour still arrives verbatim
// ─────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -211,6 +260,19 @@ describe('#15447 oauth.applications.register — the honoured members still reac
expect(init.body).toBe(JSON.stringify(req));
});

it('[#17215] a call omitting `redirect_uris` sends a body without the key at all', async () => {
// The type-level pin above cannot witness this half: it proves the call
// COMPILES, never what reaches the wire. This proves the SDK adds no
// default — no `redirect_uris: []` synthesised on the caller's behalf,
// which the vendor would refuse outright.
const { client: c, fetchMock } = clientCapturingRequest();
await c.oauth.applications.register({ client_name: 'PROBE-17215-OMITTED' });
const [url, init] = soleRequest(fetchMock);
expect(url).toBe(CREATE_CLIENT_URL);
expect(init.body).toBe(JSON.stringify({ client_name: 'PROBE-17215-OMITTED' }));
expect(JSON.parse(init.body as string)).not.toHaveProperty('redirect_uris');
});

it("surfaces the route's refusal of an array-form `scope` rather than swallowing it", async () => {
// ⚠️ The 400 below is a RECORDED response, replayed — never one this test
// produces. It is the verbatim answer the driven run got (issue #15447,
Expand Down
Loading