From fe62de2b3b20aeddebf64bf6fe52ca1051fdf6ac Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Mon, 12 Oct 2026 14:17:21 +0100 Subject: [PATCH] fix(oauth-provider): refuse trusted-client changes with 400, not 500 Updating, deleting, or rotating the secret of a client configured in code through `cachedTrustedClients` is a deliberate refusal, but the three management endpoints reported it as a 500 server failure. They now share one guard with client key rotation and return 400 `invalid_client`. --- .../oauth-provider-trusted-client-status.md | 5 ++ .../oauth-provider/src/client-key-rotation.ts | 8 +--- .../src/oauthClient/endpoints.test.ts | 48 +++++++++++++++++++ .../src/oauthClient/endpoints.ts | 26 ++-------- packages/oauth-provider/src/utils/index.ts | 16 +++++++ 5 files changed, 75 insertions(+), 28 deletions(-) create mode 100644 .changeset/oauth-provider-trusted-client-status.md diff --git a/.changeset/oauth-provider-trusted-client-status.md b/.changeset/oauth-provider-trusted-client-status.md new file mode 100644 index 0000000000..1eee35640d --- /dev/null +++ b/.changeset/oauth-provider-trusted-client-status.md @@ -0,0 +1,5 @@ +--- +"@better-auth/oauth-provider": patch +--- + +If you configure `cachedTrustedClients`, attempts to update, delete, or rotate the secret of one of those clients through the client management endpoints now fail with HTTP 400 instead of 500. The response body is unchanged (`error: "invalid_client"`), so only code that branches on the status code is affected. A 500 previously made a deliberate refusal look like a server failure. diff --git a/packages/oauth-provider/src/client-key-rotation.ts b/packages/oauth-provider/src/client-key-rotation.ts index f3328159c8..4506dacfe6 100644 --- a/packages/oauth-provider/src/client-key-rotation.ts +++ b/packages/oauth-provider/src/client-key-rotation.ts @@ -2,6 +2,7 @@ import type { GenericEndpointContext } from "@better-auth/core"; import { APIError, NO_STORE_HEADERS } from "better-auth/api"; import { assertValidClientJwks } from "./register"; import type { OAuthClient, OAuthOptions, SchemaClient, Scope } from "./types"; +import { assertNotTrustedClient } from "./utils"; import { verifyClientAssertion } from "./utils/client-assertion"; type ClientJwks = NonNullable; @@ -85,12 +86,7 @@ function assertRotatable( opts: OAuthOptions, client: SchemaClient, ) { - if (opts.cachedTrustedClients?.has(client.clientId)) { - throw new APIError("BAD_REQUEST", { - error: "invalid_client", - error_description: "trusted clients must be updated manually", - }); - } + assertNotTrustedClient(opts, client.clientId); if (client.clientDiscoveryId || client.jwksUri || !client.jwks) { throw new APIError("BAD_REQUEST", { error: "invalid_client", diff --git a/packages/oauth-provider/src/oauthClient/endpoints.test.ts b/packages/oauth-provider/src/oauthClient/endpoints.test.ts index 729cd7b60d..bb93d35214 100644 --- a/packages/oauth-provider/src/oauthClient/endpoints.test.ts +++ b/packages/oauth-provider/src/oauthClient/endpoints.test.ts @@ -541,3 +541,51 @@ describe("oauthClient private_key_jwt clients", async () => { expect(result.error?.status).toBeLessThan(500); }); }); + +describe("oauthClient - trusted clients", async () => { + const baseURL = "http://localhost:3000"; + const trustedClients = new Set(); + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + cachedTrustedClients: trustedClients, + }), + jwt(), + ], + }); + const { headers } = await signInWithTestUser(); + + it("refuses to change a trusted client as a client error, not a server failure", async () => { + const created = await auth.api.createOAuthClient({ + headers, + body: { redirect_uris: ["https://rp.example.com/callback"] }, + }); + const clientId = created.client_id; + trustedClients.add(clientId); + + for (const [path, body] of [ + [ + "/oauth2/update-client", + { client_id: clientId, update: { client_name: "x" } }, + ], + ["/oauth2/client/rotate-secret", { client_id: clientId }], + ["/oauth2/delete-client", { client_id: clientId }], + ] as const) { + const response = await customFetchImpl(`${baseURL}/api/auth${path}`, { + method: "POST", + headers: { + ...Object.fromEntries(headers), + "content-type": "application/json", + }, + body: JSON.stringify(body), + }); + expect(response.status, path).toBe(400); + expect(await response.json(), path).toMatchObject({ + error: "invalid_client", + }); + } + }); +}); diff --git a/packages/oauth-provider/src/oauthClient/endpoints.ts b/packages/oauth-provider/src/oauthClient/endpoints.ts index 505b97cf82..2f86e999d4 100644 --- a/packages/oauth-provider/src/oauthClient/endpoints.ts +++ b/packages/oauth-provider/src/oauthClient/endpoints.ts @@ -11,7 +11,7 @@ import type { Scope, } from "../types"; import type { GrantType } from "../types/oauth"; -import { getClient, storeClientSecret } from "../utils"; +import { assertNotTrustedClient, getClient, storeClientSecret } from "../utils"; import { normalizeClientCredentialsScopes, validateClientCredentialsScopes, @@ -153,13 +153,7 @@ export async function deleteClientEndpoint( if (!session) throw new APIError("UNAUTHORIZED"); const clientId = ctx.body.client_id; - const trustedClient = opts.cachedTrustedClients?.has(clientId); - if (trustedClient) { - throw new APIError("INTERNAL_SERVER_ERROR", { - error_description: "trusted clients must be updated manually", - error: "invalid_client", - }); - } + assertNotTrustedClient(opts, clientId); const client = await getClient(ctx, opts, clientId); if (!client) { @@ -232,13 +226,7 @@ export async function updateClientEndpoint( if (!session) throw new APIError("UNAUTHORIZED"); const clientId = ctx.body.client_id; - const trustedClient = opts.cachedTrustedClients?.has(clientId); - if (trustedClient) { - throw new APIError("INTERNAL_SERVER_ERROR", { - error_description: "trusted clients must be updated manually", - error: "invalid_client", - }); - } + assertNotTrustedClient(opts, clientId); const client = await getClient(ctx, opts, clientId); if (!client) { @@ -410,13 +398,7 @@ export async function rotateClientSecretEndpoint( if (!session) throw new APIError("UNAUTHORIZED"); const clientId = ctx.body.client_id; - const trustedClient = opts.cachedTrustedClients?.has(clientId); - if (trustedClient) { - throw new APIError("INTERNAL_SERVER_ERROR", { - error_description: "trusted clients must be updated manually", - error: "invalid_client", - }); - } + assertNotTrustedClient(opts, clientId); const client = await getClient(ctx, opts, clientId); if (!client) { diff --git a/packages/oauth-provider/src/utils/index.ts b/packages/oauth-provider/src/utils/index.ts index c2f1390cee..09e4271c38 100644 --- a/packages/oauth-provider/src/utils/index.ts +++ b/packages/oauth-provider/src/utils/index.ts @@ -247,6 +247,22 @@ export async function verifyOAuthQueryParams( ); } +/** + * Rejects changes to a client configured in code through `cachedTrustedClients`; + * those clients are updated in code, not through client management endpoints. + */ +export function assertNotTrustedClient( + opts: OAuthOptions, + clientId: string, +) { + if (opts.cachedTrustedClients?.has(clientId)) { + throw new APIError("BAD_REQUEST", { + error: "invalid_client", + error_description: "trusted clients must be updated manually", + }); + } +} + /** * Get a client by ID, checking trusted clients first, then database */