From 36a6c2ea03085dffa4676c9934b3a54c014517d3 Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Thu, 15 Oct 2026 16:19:09 +0100 Subject: [PATCH] fix(oauth-provider): approve pending clients with serial ids and Prisma on MongoDB The approval guard filtered userId on null, which serial ids turn into 0, so every approval returned 409. Pending clients come only from anonymous registration and never have a userId, so the guard now checks referenceId alone. Pending clients also store referenceId as null, because Prisma on MongoDB does not match unset fields with a null filter. A pending private_key_jwt client can rotate its inline keys again: the pending check in client assertion verification only guarded key rotation, and token endpoints already refuse pending clients. --- docs/content/docs/plugins/oauth-provider.mdx | 2 +- .../src/client-approval.test.ts | 36 +++++++++++++------ .../oauth-provider/src/client-approval.ts | 3 +- packages/oauth-provider/src/register.ts | 4 ++- packages/oauth-provider/src/types/index.ts | 8 ++--- .../src/utils/client-assertion.ts | 6 ---- 6 files changed, 35 insertions(+), 24 deletions(-) diff --git a/docs/content/docs/plugins/oauth-provider.mdx b/docs/content/docs/plugins/oauth-provider.mdx index b846a26ced..c776e81a83 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -1856,7 +1856,7 @@ With the callback defined: * A pending client cannot authenticate at the token, introspection, revocation, or device authorization endpoints. The authorization endpoint always sends its users to the consent page, even when `skipConsent` is set, and answers `prompt=none` with `consent_required`. * The registration response and the [public client endpoint](#get-public-client) return `pending: true`. Show it on your consent page: accepting consent for this client also hands it to an owner. -When a user accepts consent for a pending client, the callback runs before anything is stored. Return `false` to refuse: the user is redirected to the client with an `access_denied` error, as when they deny consent. Throw an `APIError` instead to show your own message on the consent page. To approve, return `clientReference`, the owner, stored as the client's `reference_id`. Use the value your [`clientReference`](#organizations) option returns for that owner, so the owner can manage the client afterwards. +When a user accepts consent for a pending client, the callback runs before anything is stored. Return `false` to refuse: the user is redirected to the client with an `access_denied` error, as when they deny consent. Throw an `APIError` instead to show your own message on the consent page. To approve, return `clientReference`, the owner, stored as the client's `reference_id`. Use the same kind of identifier your [`clientReference`](#organizations) option returns, such as an organization ID, so the owner's members can manage the client. In the example above, both are organization IDs. The owner and the consent are written in one transaction. The client is only updated while it is still pending and has no owner, so of two concurrent approvals only one succeeds (the other receives a `409`), and a client that already has an owner is never reassigned. Consent by other users for an approved client is ordinary consent and does not call the callback. diff --git a/packages/oauth-provider/src/client-approval.test.ts b/packages/oauth-provider/src/client-approval.test.ts index 4a1f492147..1db76c7baa 100644 --- a/packages/oauth-provider/src/client-approval.test.ts +++ b/packages/oauth-provider/src/client-approval.test.ts @@ -53,6 +53,7 @@ const adapters = (process.env.OAUTH_PROVIDER_TEST_ADAPTERS ?? "sqlite").split( async function createInstance( testWith: (typeof adapters)[number], options: Partial, + advanced?: { database: { generateId: "serial" } }, ) { const { auth, @@ -63,6 +64,7 @@ async function createInstance( } = await getTestInstance( { baseURL: baseUrl, + advanced, plugins: [ jwt({ jwt: { issuer: baseUrl } }), oauthProvider({ @@ -384,6 +386,22 @@ describe.each(adapters)("approvePendingClient on %s", (testWith) => { expect(await t.consents(clientId)).toHaveLength(0); }); + it("approves a client when ids are serial", async () => { + const t = await createInstance( + testWith, + { approvePendingClient: () => ({ clientReference: "business_1" }) }, + { database: { generateId: "serial" } }, + ); + const { clientId } = await t.registerPending(); + const { headers } = await t.signInWithTestUser(); + const { location } = await t.authorize(clientId, headers); + const result = await t.consent(location, headers); + expect(result.status).toBe(200); + const stored = await t.row(clientId); + expect(stored?.pending).toBe(false); + expect(stored?.referenceId).toBe("business_1"); + }); + it("redirects a refusal to the client with access_denied and stores nothing", async () => { const t = await createInstance(testWith, { approvePendingClient: () => false, @@ -526,17 +544,13 @@ describe.each(adapters)("approvePendingClient on %s", (testWith) => { it("keeps a pending client unusable when the callback is no longer configured", async () => { const t = await createInstance(testWith, {}); - const { clientId, clientSecret } = await t - .registerPending() - .catch(async () => { - const registered = await t.register({ - grant_types: ["authorization_code"], - }); - return { - clientId: registered.body.client_id, - clientSecret: registered.body.client_secret!, - }; - }); + // Without the callback, open registration cannot request client_credentials. + const registered = await t.register({ + grant_types: ["authorization_code"], + }); + expect(registered.status).toBe(201); + const clientId = registered.body.client_id; + const clientSecret = registered.body.client_secret!; await t.context.adapter.update({ model: "oauthClient", where: [{ field: "clientId", value: clientId }], diff --git a/packages/oauth-provider/src/client-approval.ts b/packages/oauth-provider/src/client-approval.ts index 8ba37022db..0cb1b25ab5 100644 --- a/packages/oauth-provider/src/client-approval.ts +++ b/packages/oauth-provider/src/client-approval.ts @@ -79,10 +79,11 @@ export async function storeClientApproval( ): Promise { const approved = await adapter.updateMany({ model: opts.schema?.oauthClient?.modelName ?? "oauthClient", + // Pending clients come only from anonymous registration, so `userId` is + // never set; filtering it on null would break serial ids. where: [ { field: "clientId", value: clientId }, { field: "pending", value: true }, - { field: "userId", value: null }, { field: "referenceId", value: null }, ], update: { diff --git a/packages/oauth-provider/src/register.ts b/packages/oauth-provider/src/register.ts index 5b71423a87..79d7952a00 100644 --- a/packages/oauth-provider/src/register.ts +++ b/packages/oauth-provider/src/register.ts @@ -1176,7 +1176,9 @@ async function persistOAuthClientRegistration( model: clientModel, data: { ...schema, - ...(startsPending ? { pending: true } : {}), + // Store the missing owner as null: Prisma on MongoDB + // skips unset fields when the approval filters on null. + ...(startsPending ? { pending: true, referenceId: null } : {}), createdAt: new Date(iat * 1000), updatedAt: new Date(iat * 1000), }, diff --git a/packages/oauth-provider/src/types/index.ts b/packages/oauth-provider/src/types/index.ts index 640a1221b8..c56d1fed50 100644 --- a/packages/oauth-provider/src/types/index.ts +++ b/packages/oauth-provider/src/types/index.ts @@ -727,9 +727,9 @@ export interface PendingClientApprovalContext< export interface PendingClientApproval { /** * Owner of the approved client, stored as its `reference_id`, such as the - * organization the consenting user administers. Use the value - * {@link OAuthOptions.clientReference} returns for that owner, so the - * owner can manage the client afterwards. + * organization the consenting user administers. Use the same kind of + * identifier {@link OAuthOptions.clientReference} returns, such as an + * organization ID, so the owner's members can manage the client. */ clientReference: string; } @@ -2215,7 +2215,7 @@ export interface SchemaClient< */ sectorIdentifier?: string | null; /** Reference to the owner of this client. Eg. Organization, Team, Profile */ - referenceId?: string; + referenceId?: string | null; /** * Additional metadata about the client. */ diff --git a/packages/oauth-provider/src/utils/client-assertion.ts b/packages/oauth-provider/src/utils/client-assertion.ts index 110e586c51..a2b9170937 100644 --- a/packages/oauth-provider/src/utils/client-assertion.ts +++ b/packages/oauth-provider/src/utils/client-assertion.ts @@ -542,12 +542,6 @@ export async function verifyClientAssertion( error: "invalid_client", }); } - if (client.pending) { - throw new APIError("BAD_REQUEST", { - error_description: "client is pending approval", - error: "invalid_client", - }); - } if (client.tokenEndpointAuthMethod !== "private_key_jwt") { throw new APIError("BAD_REQUEST", { error_description: