From 472a5f22ff686a4e5c3baad7954e2ecc44868670 Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Mon, 12 Oct 2026 14:18:42 +0100 Subject: [PATCH] refactor(oauth-provider): refuse unrotatable clients before verifying the assertion verifyClientAssertion takes an `assertClient` precondition that runs on the looked-up client before any key is fetched or the jti is consumed. Key rotation uses it, so a jwks_uri or trusted client is refused without a JWKS fetch, a signature check, or spending the assertion. --- .../oauth-provider-rotation-precheck.md | 5 ++++ .../src/client-key-rotation.test.ts | 30 +++++++++++++++++++ .../oauth-provider/src/client-key-rotation.ts | 6 ++-- .../src/utils/client-assertion.ts | 10 ++++++- 4 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 .changeset/oauth-provider-rotation-precheck.md diff --git a/.changeset/oauth-provider-rotation-precheck.md b/.changeset/oauth-provider-rotation-precheck.md new file mode 100644 index 0000000000..3a11605ba6 --- /dev/null +++ b/.changeset/oauth-provider-rotation-precheck.md @@ -0,0 +1,5 @@ +--- +"@better-auth/oauth-provider": patch +--- + +`POST /oauth2/client/rotate-keys` now refuses a client it cannot rotate, such as one that publishes a `jwks_uri` or a trusted client configured in code, before it fetches the client's keys or uses up the client assertion. diff --git a/packages/oauth-provider/src/client-key-rotation.test.ts b/packages/oauth-provider/src/client-key-rotation.test.ts index 69b451e616..c81a08a832 100644 --- a/packages/oauth-provider/src/client-key-rotation.test.ts +++ b/packages/oauth-provider/src/client-key-rotation.test.ts @@ -428,6 +428,36 @@ describe.each(adapters)("client key rotation ($name)", async ({ trustedClients.delete(clientId); }); + // The memory adapter does not enforce the primary key the jti tombstone relies on. + it.skipIf(name === "memory adapter")( + "refuses a client it cannot rotate before spending the assertion", + async () => { + const oldKey = await createKey("old"); + const newKey = await createKey("new"); + const clientId = await createClient([oldKey]); + const assertion = await sign(clientId, oldKey, rotateURL); + + trustedClients.add(clientId); + const refused = await rotate({ + clientId, + signer: oldKey, + next: [newKey], + assertion, + }); + trustedClients.delete(clientId); + expect(refused.status).toBe(400); + + // The refusal came before verification, so the jti was never consumed. + const accepted = await rotate({ + clientId, + signer: oldKey, + next: [newKey], + assertion, + }); + expect(accepted.status).toBe(200); + }, + ); + it("rejects a disabled client", async () => { const oldKey = await createKey("old"); const clientId = await createClient([oldKey]); diff --git a/packages/oauth-provider/src/client-key-rotation.ts b/packages/oauth-provider/src/client-key-rotation.ts index 4506dacfe6..f26e25b866 100644 --- a/packages/oauth-provider/src/client-key-rotation.ts +++ b/packages/oauth-provider/src/client-key-rotation.ts @@ -49,9 +49,11 @@ export async function rotateClientKeysEndpoint( body.client_assertion_type, body.client_id, `${ctx.context.baseURL}/oauth2/client/rotate-keys`, - { endpointAudienceOnly: true }, + { + endpointAudienceOnly: true, + assertClient: (candidate) => assertRotatable(opts, candidate), + }, ); - assertRotatable(opts, client); const updated = await ctx.context.adapter.incrementOne>( { diff --git a/packages/oauth-provider/src/utils/client-assertion.ts b/packages/oauth-provider/src/utils/client-assertion.ts index 0eeabfc75e..46d8ae5494 100644 --- a/packages/oauth-provider/src/utils/client-assertion.ts +++ b/packages/oauth-provider/src/utils/client-assertion.ts @@ -460,6 +460,10 @@ export async function consumeClientAssertion( * an assertion addressed to `expectedAudience` itself verifies. Endpoints that * change client credentials use it, so an assertion minted for the token * endpoint or the issuer cannot be redirected to them. + * + * `assertClient` runs on the looked-up client before any key is fetched or the + * jti is consumed, so a caller can refuse a client it does not serve without + * spending the assertion or reaching out to a `jwks_uri`. */ export async function verifyClientAssertion( ctx: GenericEndpointContext, @@ -468,7 +472,10 @@ export async function verifyClientAssertion( clientAssertionType: string, clientIdHint?: string, expectedAudience?: string, - options?: { endpointAudienceOnly?: boolean }, + options?: { + endpointAudienceOnly?: boolean; + assertClient?: (client: SchemaClient) => void; + }, ): Promise<{ clientId: string; client: SchemaClient }> { if (clientAssertionType !== CLIENT_ASSERTION_TYPE) { throw new APIError("BAD_REQUEST", { @@ -542,6 +549,7 @@ export async function verifyClientAssertion( error: "invalid_client", }); } + options?.assertClient?.(client); // Fetch JWKS and verify signature + claims const jwks = await fetchClientJwks(ctx, opts, client);