diff --git a/.changeset/oauth-provider-client-key-rotation.md b/.changeset/oauth-provider-client-key-rotation.md new file mode 100644 index 0000000000..5dc9f57b69 --- /dev/null +++ b/.changeset/oauth-provider-client-key-rotation.md @@ -0,0 +1,7 @@ +--- +"@better-auth/oauth-provider": patch +--- + +Confidential `private_key_jwt` clients that register inline `jwks` can now replace their own signing keys without a user session. Until now those keys were fixed at registration. The feature is off by default; set `allowClientKeyRotation: true` to enable it. No database migration is needed. + +The client calls `POST /oauth2/client/rotate-keys` with its complete new `jwks` and a client assertion signed by one of its current keys, with `aud` set to the rotation endpoint's URL. An assertion addressed to the token endpoint or the issuer is rejected. Keys left out of the new set stop working at once, so roll a key in two steps: send the old and new keys together, then drop the old key once every instance signs with the new one. diff --git a/docs/content/docs/plugins/oauth-provider.mdx b/docs/content/docs/plugins/oauth-provider.mdx index 5764021ca0..99e5ec2398 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -632,6 +632,53 @@ The assertion JWT must contain: Supported signing algorithms: `RS256`, `RS384`, `RS512`, `PS256`, `PS384`, `PS512`, `ES256`, `ES384`, `ES512`, `EdDSA`. +##### Rotating client keys + +A client with inline `jwks` can replace its own keys without a user session. This is how a service rotates its signing key on a schedule. Enable it on the provider: + +```ts title="auth.ts" +oauthProvider({ + allowClientKeyRotation: true, +}) +``` + +The client generates a new key pair, then calls the endpoint with a client assertion signed by one of its current keys: + +``` +POST /api/auth/oauth2/client/rotate-keys +Content-Type: application/json + +{ + "client_assertion_type": "urn:ietf:params:oauth:client-assertion-type:jwt-bearer", + "client_assertion": "eyJhbGciOiJFUzI1NiIs...", + "jwks": { "keys": [{ "kty": "EC", "crv": "P-256", "kid": "key-2", "x": "...", "y": "..." }] } +} +``` + +The assertion follows the claims in the table above, with one difference: `aud` must be the URL of the rotation endpoint itself. An assertion issued for the token endpoint or the issuer is rejected here, so it cannot be redirected to take over a client's keys. The assertion is also single-use. + +`jwks` is the client's complete new key set. A key that is not in it stops verifying at once, and a key that is in both sets stays active. The response returns the new set: + +```json +{ + "client_id": "CLIENT_ID", + "jwks": { "keys": [{ "kid": "key-2" }] } +} +``` + +To replace a key without breaking instances that still sign with the old one, roll it in two steps, the same way a client that publishes a `jwks_uri` does: + +1. Send a set with both the old key and the new key, signed by the old key. Nothing stops working. +2. Once every instance signs with the new key, send a set with only the new key, signed by the new key. The old key stops working. + +To retire a leaked key, send a set without it. It stops working with that request. + +If the client's keys change while a rotation is in flight, for example through two simultaneous rotations, the losing request fails with `409`, or with `401` when the key that signed it has already been removed. Sign a fresh assertion with a current key and retry. + + + Only clients that register inline `jwks` can use this endpoint. A client that publishes a `jwks_uri` rotates its keys at that URI, and trusted clients configured in code are updated in code. Rotation is not available to `client_secret_*` clients; use `/oauth2/client/rotate-secret`. + + #### Authorization code grant The authorization code grant enables clients to obtain access user access tokens and optionally refresh tokens (with the "offline\_access" scope). @@ -1793,14 +1840,15 @@ The OAuth Provider includes built-in rate limiting for all OAuth endpoints to pr **Default limits:** -| Endpoint | Window | Max Requests | -| -------------------- | ------ | ------------ | -| `/oauth2/token` | 60s | 20 | -| `/oauth2/authorize` | 60s | 30 | -| `/oauth2/introspect` | 60s | 100 | -| `/oauth2/revoke` | 60s | 30 | -| `/oauth2/register` | 60s | 5 | -| `/oauth2/userinfo` | 60s | 60 | +| Endpoint | Window | Max Requests | +| ---------------------------- | ------ | ------------ | +| `/oauth2/token` | 60s | 20 | +| `/oauth2/authorize` | 60s | 30 | +| `/oauth2/introspect` | 60s | 100 | +| `/oauth2/revoke` | 60s | 30 | +| `/oauth2/register` | 60s | 5 | +| `/oauth2/client/rotate-keys` | 60s | 5 | +| `/oauth2/userinfo` | 60s | 60 | You can customize the rate limits for each endpoint: @@ -1812,6 +1860,7 @@ oauthProvider({ introspect: { window: 60, max: 100 }, // 100 requests per minute revoke: { window: 60, max: 30 }, // 30 requests per minute register: { window: 60, max: 5 }, // 5 requests per minute + rotateClientKeys: { window: 60, max: 5 }, // 5 requests per minute userinfo: { window: 60, max: 60 }, // 60 requests per minute }, }) @@ -2205,6 +2254,20 @@ export const oauthClientTableFields = [ "OIDC application type used to classify redirect URI policy. Supports: ['web', 'native']", isOptional: true, }, + { + name: "jwks", + type: "string", + description: + "The client's public JWK Set as JSON, for `private_key_jwt` authentication. Mutually exclusive with `jwksUri`", + isOptional: true, + }, + { + name: "jwksUri", + type: "string", + description: + "HTTPS URI of the client's public JWK Set. Mutually exclusive with `jwks`", + isOptional: true, + }, { name: "clientDiscoveryId", type: "string", diff --git a/packages/oauth-provider/src/client-key-rotation.test.ts b/packages/oauth-provider/src/client-key-rotation.test.ts new file mode 100644 index 0000000000..69b451e616 --- /dev/null +++ b/packages/oauth-provider/src/client-key-rotation.test.ts @@ -0,0 +1,507 @@ +import { memoryAdapter } from "@better-auth/memory-adapter"; +import { jwt } from "better-auth/plugins/jwt"; +import { getTestInstance } from "better-auth/test"; +import type { JWK } from "jose"; +import { exportJWK, generateKeyPair, SignJWT } from "jose"; +import { describe, expect, it, vi } from "vitest"; +import { oauthProvider } from "./oauth"; +import type { OAuthOptions, SchemaClient, Scope } from "./types"; + +const ASSERTION_TYPE = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"; + +type TestKey = { + kid: string; + privateKey: CryptoKey; + publicJwk: JWK & Record; +}; + +async function createKey(kid: string): Promise { + const { privateKey, publicKey } = await generateKeyPair("ES256", { + extractable: true, + }); + return { + kid, + privateKey: privateKey as CryptoKey, + publicJwk: { ...(await exportJWK(publicKey)), kid, alg: "ES256" }, + }; +} + +function createMemoryDatabase() { + return { + account: [], + jwks: [], + oauthAccessToken: [], + oauthClient: [], + oauthClientAssertion: [], + oauthClientResource: [], + oauthConsent: [], + oauthRefreshToken: [], + oauthResource: [], + session: [], + user: [], + verification: [], + }; +} + +const adapters = [ + { + name: "sqlite", + config: { testWith: "sqlite" as const }, + database: () => undefined, + }, + { + name: "memory adapter", + config: { testWith: "sqlite" as const }, + database: () => memoryAdapter(createMemoryDatabase()), + }, + { + name: "postgres", + config: { testWith: "postgres" as const }, + database: () => undefined, + }, +]; + +describe.each(adapters)("client key rotation ($name)", async ({ + name, + config, + database, +}) => { + const baseURL = "http://localhost:3000"; + const apiURL = `${baseURL}/api/auth`; + const rotateURL = `${apiURL}/oauth2/client/rotate-keys`; + const introspectURL = `${apiURL}/oauth2/introspect`; + const trustedClients = new Set(); + const providerOptions = { + loginPage: "/login", + consentPage: "/consent", + cachedTrustedClients: trustedClients, + allowClientKeyRotation: true, + } satisfies OAuthOptions; + + const instance = await getTestInstance( + { + baseURL, + trustedOrigins: ["https://trusted.example.com"], + ...(database() ? { database: database()! } : {}), + plugins: [jwt(), oauthProvider(providerOptions)], + }, + config, + ); + const { auth, signInWithTestUser, customFetchImpl } = instance; + const { headers } = await signInWithTestUser(); + const context = await auth.$context; + + async function createClient(keys: TestKey[]) { + const client = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: ["http://localhost:5000/callback"], + application_type: "native", + skip_consent: true, + token_endpoint_auth_method: "private_key_jwt", + jwks: { keys: keys.map((key) => key.publicJwk) }, + }, + }); + return client!.client_id; + } + + async function sign( + clientId: string, + key: TestKey, + audience: string, + jti: string = crypto.randomUUID(), + ) { + const now = Math.floor(Date.now() / 1000); + return new SignJWT({}) + .setProtectedHeader({ alg: "ES256", kid: key.kid }) + .setIssuer(clientId) + .setSubject(clientId) + .setAudience(audience) + .setIssuedAt(now) + .setExpirationTime(now + 120) + .setJti(jti) + .sign(key.privateKey); + } + + /** POSTs the rotation request the way a client would. */ + async function rotate(input: { + clientId: string; + signer: TestKey; + next: TestKey[]; + audience?: string; + assertion?: string; + }) { + const response = await customFetchImpl(rotateURL, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + client_assertion_type: ASSERTION_TYPE, + client_assertion: + input.assertion ?? + (await sign( + input.clientId, + input.signer, + input.audience ?? rotateURL, + )), + jwks: { keys: input.next.map((key) => key.publicJwk) }, + }), + }); + return { + status: response.status, + headers: response.headers, + body: (await response.json()) as Record, + }; + } + + /** Authenticates at the introspection endpoint, which verifies the client first. */ + async function authenticate(clientId: string, key: TestKey) { + const response = await customFetchImpl(introspectURL, { + method: "POST", + headers: { "content-type": "application/x-www-form-urlencoded" }, + body: new URLSearchParams({ + token: "not-a-real-token", + client_assertion_type: ASSERTION_TYPE, + client_assertion: await sign(clientId, key, introspectURL), + }), + }); + return response.status; + } + + async function readClient(clientId: string) { + return context.adapter.findOne>({ + model: "oauthClient", + where: [{ field: "clientId", value: clientId }], + }); + } + + it("is off unless the provider opts in", async () => { + const closed = await getTestInstance({ + baseURL, + plugins: [ + jwt(), + oauthProvider({ loginPage: "/login", consentPage: "/consent" }), + ], + }); + const response = await closed.customFetchImpl(rotateURL, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + client_assertion_type: ASSERTION_TYPE, + client_assertion: "a.b.c", + jwks: { keys: [(await createKey("k")).publicJwk] }, + }), + }); + expect(response.status).toBe(403); + expect(await response.json()).toMatchObject({ error: "access_denied" }); + }); + + it("replaces the keys on the client's own authority", async () => { + const oldKey = await createKey("old"); + const newKey = await createKey("new"); + const clientId = await createClient([oldKey]); + + const rotated = await rotate({ + clientId, + signer: oldKey, + next: [newKey], + }); + + expect(rotated.status).toBe(200); + expect(rotated.headers.get("cache-control")).toContain("no-store"); + expect(rotated.body.client_id).toBe(clientId); + expect(rotated.body.jwks.keys.map((key: JWK) => key.kid)).toEqual(["new"]); + expect(await authenticate(clientId, newKey)).toBe(200); + expect(await authenticate(clientId, oldKey)).toBe(401); + }); + + it("is callable in process through auth.api with typed input and output", async () => { + const oldKey = await createKey("old"); + const newKey = await createKey("new"); + const clientId = await createClient([oldKey]); + + const rotated = await auth.api.rotateClientKeys({ + body: { + client_assertion_type: ASSERTION_TYPE, + client_assertion: await sign(clientId, oldKey, rotateURL), + jwks: { keys: [newKey.publicJwk] }, + }, + }); + + expect(rotated.client_id).toBe(clientId); + expect(rotated.jwks.keys).toHaveLength(1); + }); + + it("rolls a key in two steps without breaking the old key in between", async () => { + const keptKey = await createKey("kept"); + const addedKey = await createKey("added"); + const clientId = await createClient([keptKey]); + + const rotated = await rotate({ + clientId, + signer: keptKey, + next: [keptKey, addedKey], + }); + + expect(rotated.status).toBe(200); + expect(await authenticate(clientId, keptKey)).toBe(200); + expect(await authenticate(clientId, addedKey)).toBe(200); + + const dropped = await rotate({ + clientId, + signer: addedKey, + next: [addedKey], + }); + expect(dropped.status).toBe(200); + expect(await authenticate(clientId, keptKey)).toBe(401); + expect(await authenticate(clientId, addedKey)).toBe(200); + }); + + it("does not let a removed key rotate the client again", async () => { + const removedKey = await createKey("removed"); + const currentKey = await createKey("current"); + const attackerKey = await createKey("attacker"); + const clientId = await createClient([removedKey]); + await rotate({ clientId, signer: removedKey, next: [currentKey] }); + + const takeover = await rotate({ + clientId, + signer: removedKey, + next: [attackerKey], + }); + + expect(takeover.status).toBe(401); + expect(takeover.body.error).toBe("invalid_client"); + expect(await authenticate(clientId, currentKey)).toBe(200); + expect(await authenticate(clientId, attackerKey)).toBe(401); + }); + + it("rejects an assertion signed by a key the client never registered", async () => { + const registered = await createKey("registered"); + const stranger = await createKey("registered"); + const clientId = await createClient([registered]); + + const response = await rotate({ + clientId, + signer: stranger, + next: [await createKey("new")], + }); + + expect(response.status).toBe(401); + expect(await authenticate(clientId, registered)).toBe(200); + }); + + it("only accepts an assertion addressed to the rotation endpoint", async () => { + const oldKey = await createKey("old"); + const clientId = await createClient([oldKey]); + const next = [await createKey("new")]; + + // An assertion minted for the token endpoint, the introspection endpoint, or the + // issuer is a valid private_key_jwt credential elsewhere. It must not be + // redirectable here, where it would hand over the client's keys. + for (const audience of [ + `${apiURL}/oauth2/token`, + introspectURL, + apiURL, + baseURL, + ]) { + const response = await rotate({ + clientId, + signer: oldKey, + next, + audience, + }); + expect(response.status, audience).toBe(401); + expect(response.body.error).toBe("invalid_client"); + } + expect(await authenticate(clientId, oldKey)).toBe(200); + }); + + // The memory adapter does not enforce the primary key the jti tombstone relies on. + it.skipIf(name === "memory adapter")( + "accepts each assertion once", + async () => { + const signer = await createKey("signer"); + const clientId = await createClient([signer]); + const assertion = await sign(clientId, signer, rotateURL); + const added = await createKey("added"); + + // The signer stays in the set, so only the jti stops the replay. + const first = await rotate({ + clientId, + signer, + next: [signer, added], + assertion, + }); + const replay = await rotate({ + clientId, + signer, + next: [signer, await createKey("attacker")], + assertion, + }); + + expect(first.status).toBe(200); + expect(replay.status).toBe(400); + expect(replay.body.error_description).toContain("already been used"); + expect(await authenticate(clientId, added)).toBe(200); + }, + ); + + it("rejects a malformed or unsafe replacement key set before touching the client", async () => { + const oldKey = await createKey("old"); + const clientId = await createClient([oldKey]); + const privateJwk = await exportJWK(oldKey.privateKey); + + const post = async (jwks: unknown) => { + const response = await customFetchImpl(rotateURL, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + client_assertion_type: ASSERTION_TYPE, + client_assertion: await sign(clientId, oldKey, rotateURL), + jwks, + }), + }); + return { + status: response.status, + body: (await response.json()) as Record, + }; + }; + + for (const jwks of [ + { keys: [] }, + { keys: [{ kty: "oct", k: "c2VjcmV0" }] }, + { keys: [{ ...privateJwk, kid: "leaky", alg: "ES256" }] }, + ]) { + const response = await post(jwks); + expect(response.status).toBe(400); + expect(response.body.error).toBe("invalid_client_metadata"); + } + expect(await authenticate(clientId, oldKey)).toBe(200); + }); + + it("leaves clients that do not hold inline keys to their own rotation path", async () => { + const signer = await createKey("signer"); + const next = [await createKey("next")]; + const secretClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: ["http://localhost:5000/callback"], + application_type: "native", + skip_consent: true, + }, + }); + const jwksUriClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: ["http://localhost:5000/callback"], + application_type: "native", + skip_consent: true, + token_endpoint_auth_method: "private_key_jwt", + jwks_uri: "https://trusted.example.com/jwks.json", + }, + }); + + for (const client of [secretClient, jwksUriClient]) { + const response = await rotate({ + clientId: client!.client_id, + signer, + next, + }); + expect(response.status).toBe(400); + expect(response.body.error).toBe("invalid_client"); + } + }); + + it("leaves clients configured in code to be updated in code", async () => { + const oldKey = await createKey("old"); + const clientId = await createClient([oldKey]); + trustedClients.add(clientId); + + const response = await rotate({ + clientId, + signer: oldKey, + next: [await createKey("new")], + }); + + expect(response.status).toBe(400); + expect(response.body.error_description).toContain("updated manually"); + trustedClients.delete(clientId); + }); + + it("rejects a disabled client", async () => { + const oldKey = await createKey("old"); + const clientId = await createClient([oldKey]); + await context.adapter.update({ + model: "oauthClient", + where: [{ field: "clientId", value: clientId }], + update: { disabled: true }, + }); + + const response = await rotate({ + clientId, + signer: oldKey, + next: [await createKey("new")], + }); + + expect(response.status).toBe(400); + expect(response.body.error_description).toContain("disabled"); + }); + + it("loses to a change that lands between verification and write", async () => { + const oldKey = await createKey("old"); + const newKey = await createKey("new"); + const ownerKey = await createKey("owner"); + const clientId = await createClient([oldKey]); + const incrementOne = context.adapter.incrementOne; + vi.spyOn(context.adapter, "incrementOne").mockImplementationOnce( + async (input) => { + // The owner replaces the keys after the assertion was verified. + await context.adapter.update({ + model: "oauthClient", + where: [{ field: "clientId", value: clientId }], + update: { jwks: JSON.stringify({ keys: [ownerKey.publicJwk] }) }, + }); + return incrementOne(input); + }, + ); + + const response = await rotate({ + clientId, + signer: oldKey, + next: [newKey], + }); + + expect(response.status).toBe(409); + expect(response.body.error).toBe("invalid_request"); + expect(await authenticate(clientId, ownerKey)).toBe(200); + expect(await authenticate(clientId, newKey)).toBe(401); + expect(await authenticate(clientId, oldKey)).toBe(401); + }); + + it("never loses an update when two rotations from the same key race", async () => { + const oldKey = await createKey("old"); + const keyA = await createKey("a"); + const keyB = await createKey("b"); + const clientId = await createClient([oldKey]); + + const results = await Promise.all([ + rotate({ clientId, signer: oldKey, next: [keyA] }), + rotate({ clientId, signer: oldKey, next: [keyB] }), + ]); + + // Either the writes interleaved and the stale one lost the guarded write + // (409), or one finished first and removed the key the other was signed + // with (401). A silent overwrite must never happen. + const statuses = results.map((result) => result.status).sort(); + expect(statuses[0]).toBe(200); + expect([401, 409]).toContain(statuses[1]); + const stored = await readClient(clientId); + const active = JSON.parse(stored!.jwks!).keys as JWK[]; + expect(active).toHaveLength(1); + const activeKey = active[0]!.kid === "a" ? keyA : keyB; + const otherKey = activeKey === keyA ? keyB : keyA; + expect(await authenticate(clientId, activeKey)).toBe(200); + expect(await authenticate(clientId, otherKey)).toBe(401); + expect(await authenticate(clientId, oldKey)).toBe(401); + }); +}); diff --git a/packages/oauth-provider/src/client-key-rotation.ts b/packages/oauth-provider/src/client-key-rotation.ts new file mode 100644 index 0000000000..f3328159c8 --- /dev/null +++ b/packages/oauth-provider/src/client-key-rotation.ts @@ -0,0 +1,101 @@ +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 { verifyClientAssertion } from "./utils/client-assertion"; + +type ClientJwks = NonNullable; + +interface RotateClientKeysRequest { + client_id?: string | undefined; + client_assertion: string; + client_assertion_type: string; + jwks: ClientJwks; +} + +/** + * Replaces a confidential `private_key_jwt` client's inline `jwks` on the + * client's own authority: the request carries a client assertion signed by one + * of the client's current keys, so no user session is involved. + * + * Keys left out of the new set stop verifying at once. A client rolls a key the + * way a `jwks_uri` client does: it first sends the old and new keys together, + * then drops the old key once every instance signs with the new one. + * + * The write is guarded on the exact key set the assertion was verified against. + * A concurrent rotation changes that value, so the stale request loses instead + * of overwriting a newer key set. + * + * @see https://datatracker.ietf.org/doc/html/rfc7523#section-3 + */ +export async function rotateClientKeysEndpoint( + ctx: GenericEndpointContext, + opts: OAuthOptions, +): Promise<{ client_id: string; jwks: ClientJwks }> { + if (!opts.allowClientKeyRotation) { + throw new APIError("FORBIDDEN", { + error: "access_denied", + error_description: "Client key rotation is disabled", + }); + } + const body = ctx.body as RotateClientKeysRequest; + assertValidClientJwks(body.jwks); + + const { client } = await verifyClientAssertion( + ctx, + opts, + body.client_assertion, + body.client_assertion_type, + body.client_id, + `${ctx.context.baseURL}/oauth2/client/rotate-keys`, + { endpointAudienceOnly: true }, + ); + assertRotatable(opts, client); + + const updated = await ctx.context.adapter.incrementOne>( + { + model: "oauthClient", + where: [ + { field: "clientId", value: client.clientId }, + { field: "jwks", value: client.jwks! }, + ], + increment: {}, + set: { + jwks: JSON.stringify(body.jwks), + updatedAt: new Date(), + }, + }, + ); + if (!updated) { + throw new APIError( + "CONFLICT", + { + error: "invalid_request", + error_description: + "client keys changed while rotating; retry with a fresh client assertion", + }, + NO_STORE_HEADERS, + ); + } + + return { client_id: client.clientId, jwks: body.jwks }; +} + +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", + }); + } + if (client.clientDiscoveryId || client.jwksUri || !client.jwks) { + throw new APIError("BAD_REQUEST", { + error: "invalid_client", + error_description: + "client keys are not stored inline; rotate them where the client publishes them", + }); + } +} diff --git a/packages/oauth-provider/src/oauth.test.ts b/packages/oauth-provider/src/oauth.test.ts index 1d53b642bf..7db058c7dc 100644 --- a/packages/oauth-provider/src/oauth.test.ts +++ b/packages/oauth-provider/src/oauth.test.ts @@ -3437,7 +3437,7 @@ describe("oauth - rate limiting", () => { const plugin = auth.options.plugins?.find((p) => p.id === "oauth-provider"); expect(plugin?.rateLimit).toBeDefined(); - expect(plugin?.rateLimit?.length).toBe(6); + expect(plugin?.rateLimit?.length).toBe(7); // Check token endpoint default const tokenRule = plugin?.rateLimit?.find((r) => @@ -3446,6 +3446,13 @@ describe("oauth - rate limiting", () => { expect(tokenRule?.window).toBe(60); expect(tokenRule?.max).toBe(20); + // Credential-replacing endpoints are limited like registration + const rotateKeysRule = plugin?.rateLimit?.find((r) => + r.pathMatcher("/oauth2/client/rotate-keys"), + ); + expect(rotateKeysRule?.window).toBe(60); + expect(rotateKeysRule?.max).toBe(5); + // Check authorize endpoint default const authorizeRule = plugin?.rateLimit?.find((r) => r.pathMatcher("/oauth2/authorize"), @@ -3539,7 +3546,7 @@ describe("oauth - rate limiting", () => { const plugin = auth.options.plugins?.find((p) => p.id === "oauth-provider"); // Token and introspect should be disabled (not in array) - expect(plugin?.rateLimit?.length).toBe(4); + expect(plugin?.rateLimit?.length).toBe(5); const tokenRule = plugin?.rateLimit?.find((r) => r.pathMatcher("/oauth2/token"), diff --git a/packages/oauth-provider/src/oauth.ts b/packages/oauth-provider/src/oauth.ts index c1c7a8f1b7..02fe36b48d 100644 --- a/packages/oauth-provider/src/oauth.ts +++ b/packages/oauth-provider/src/oauth.ts @@ -1690,6 +1690,7 @@ export const oauthProvider = >(options: O) => { adminUpdateOAuthClient: oauthClientEndpoints.adminUpdateOAuthClient(opts), updateOAuthClient: oauthClientEndpoints.updateOAuthClient(opts), rotateClientSecret: oauthClientEndpoints.rotateClientSecret(opts), + rotateClientKeys: oauthClientEndpoints.rotateClientKeys(opts), deleteOAuthClient: oauthClientEndpoints.deleteOAuthClient(opts), getOAuthConsent: oauthConsentEndpoints.getOAuthConsent(opts), getOAuthConsents: oauthConsentEndpoints.getOAuthConsents(opts), @@ -1764,6 +1765,17 @@ export const oauthProvider = >(options: O) => { }, ] : []), + // Client key rotation - replaces credentials, so keep it tight + ...(opts.rateLimit?.rotateClientKeys !== false + ? [ + { + pathMatcher: (path: string) => + path === "/oauth2/client/rotate-keys", + window: opts.rateLimit?.rotateClientKeys?.window ?? 60, + max: opts.rateLimit?.rotateClientKeys?.max ?? 5, + }, + ] + : []), // UserInfo - API endpoint ...(opts.rateLimit?.userinfo !== false ? [ diff --git a/packages/oauth-provider/src/oauthClient/index.ts b/packages/oauth-provider/src/oauthClient/index.ts index d17608e79d..d0f003d652 100644 --- a/packages/oauth-provider/src/oauthClient/index.ts +++ b/packages/oauth-provider/src/oauthClient/index.ts @@ -1,6 +1,8 @@ import { createAuthEndpoint, sessionMiddleware } from "better-auth/api"; import * as z from "zod"; +import { rotateClientKeysEndpoint } from "../client-key-rotation"; import { publicSessionMiddleware } from "../middleware"; +import { createOAuthEndpoint } from "../oauth-endpoint"; import { createOAuthClientEndpoint } from "../register"; import type { OAuthOptions, Scope } from "../types"; import { @@ -604,6 +606,76 @@ export const rotateClientSecret = (opts: OAuthOptions) => }, ); +/** + * Replaces a `private_key_jwt` client's own signing keys. The client proves it + * holds a current key with a client assertion, so no session is needed. + */ +export const rotateClientKeys = (opts: OAuthOptions) => + createOAuthEndpoint( + "/oauth2/client/rotate-keys", + { + method: "POST", + body: z.object({ + client_id: z.string().optional(), + client_assertion: z.string(), + client_assertion_type: z.string(), + jwks: clientJwksSchema, + }), + errorCodesByField: { + client_assertion: "invalid_client", + client_assertion_type: "invalid_client", + jwks: "invalid_client_metadata", + }, + metadata: { + noStore: true, + openapi: { + description: + "Replaces the signing keys of a private_key_jwt client. The request is authenticated by a client assertion signed with one of the client's current keys and addressed to this endpoint. Keys left out of the new set stop verifying at once.", + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + properties: { + client_id: { + type: "string", + description: + "Optional; must match the assertion's iss and sub", + }, + client_assertion: { + type: "string", + description: + "JWT signed with a current client key; aud must be this endpoint's URL", + }, + client_assertion_type: { + type: "string", + description: + "urn:ietf:params:oauth:client-assertion-type:jwt-bearer", + }, + jwks: { + type: "object", + description: + "The client's complete new public JWK Set. Keys omitted from it stop verifying at once", + }, + }, + required: [ + "client_assertion", + "client_assertion_type", + "jwks", + ], + }, + }, + }, + }, + }, + }, + }, + async (ctx) => { + return ctx.json(await rotateClientKeysEndpoint(ctx, opts)); + }, + ); + export const deleteOAuthClient = (opts: OAuthOptions) => createAuthEndpoint( "/oauth2/delete-client", diff --git a/packages/oauth-provider/src/private-key-jwt.test.ts b/packages/oauth-provider/src/private-key-jwt.test.ts index 401326a47a..ad10a6ef63 100644 --- a/packages/oauth-provider/src/private-key-jwt.test.ts +++ b/packages/oauth-provider/src/private-key-jwt.test.ts @@ -344,7 +344,7 @@ describe("private_key_jwt authentication", async () => { clientId, audienceA, ), - ).resolves.toEqual({ clientId }); + ).resolves.toMatchObject({ clientId }); await expect( verifyClientAssertion( providerB.ctx, @@ -359,7 +359,7 @@ describe("private_key_jwt authentication", async () => { clientId, audienceB, ), - ).resolves.toEqual({ clientId }); + ).resolves.toMatchObject({ clientId }); expect(transportA).toHaveBeenCalledTimes(1); expect(transportB).toHaveBeenCalledTimes(1); @@ -532,7 +532,7 @@ describe("private_key_jwt authentication", async () => { clientId, audience, ), - ).resolves.toEqual({ clientId }); + ).resolves.toMatchObject({ clientId }); expect(transport).toHaveBeenCalledTimes(2); }); diff --git a/packages/oauth-provider/src/register.ts b/packages/oauth-provider/src/register.ts index acbf44c82a..56a799772d 100644 --- a/packages/oauth-provider/src/register.ts +++ b/packages/oauth-provider/src/register.ts @@ -264,7 +264,7 @@ function validateClientRedirectUri( } } -function assertValidRegistrationJwks(jwks: NonNullable) { +export function assertValidClientJwks(jwks: NonNullable) { const result = validatePublicClientJwks(jwks); if (!result.valid) { throw new APIError("BAD_REQUEST", { @@ -640,7 +640,7 @@ export async function checkOAuthClient( } } if (clientWithDefaults.jwks) { - assertValidRegistrationJwks(clientWithDefaults.jwks); + assertValidClientJwks(clientWithDefaults.jwks); } } // private_key_jwt requires key material; other methods may still register diff --git a/packages/oauth-provider/src/types/index.ts b/packages/oauth-provider/src/types/index.ts index 65ef15ea79..80c67c9199 100644 --- a/packages/oauth-provider/src/types/index.ts +++ b/packages/oauth-provider/src/types/index.ts @@ -751,6 +751,17 @@ export interface OAuthOptions< * @default 300 (5 minutes) */ assertionMaxLifetime?: number; + /** + * Lets a confidential `private_key_jwt` client replace its own inline `jwks` + * at `POST /oauth2/client/rotate-keys`, authenticated by a client assertion + * signed with one of its current keys. No user session is involved. + * + * Clients that publish their keys at a `jwks_uri` rotate them there, so they + * cannot use this endpoint. + * + * @default false + */ + allowClientKeyRotation?: boolean; /** * Allows /oauth2/public-client-prelogin endpoint to be * requestable prior to login via a valid oauth_query. @@ -1421,6 +1432,11 @@ export interface OAuthOptions< * @default { window: 60, max: 5 } */ register?: { window: number; max: number } | false; + /** + * Rate limit for /oauth2/client/rotate-keys endpoint + * @default { window: 60, max: 5 } + */ + rotateClientKeys?: { window: number; max: number } | false; /** * Rate limit for /oauth2/userinfo endpoint * @default { window: 60, max: 60 } diff --git a/packages/oauth-provider/src/utils/client-assertion.ts b/packages/oauth-provider/src/utils/client-assertion.ts index c83ca94ea8..0eeabfc75e 100644 --- a/packages/oauth-provider/src/utils/client-assertion.ts +++ b/packages/oauth-provider/src/utils/client-assertion.ts @@ -452,6 +452,14 @@ export async function consumeClientAssertion( * Validates: signature, iss=client_id, sub=client_id, aud=the endpoint serving * the assertion or the normalized OP issuer, exp, assertion max lifetime, * jti uniqueness (replay prevention). + * + * Returns the client record the signature was checked against, so a caller that + * mutates the client can guard its write on exactly that snapshot. + * + * With `endpointAudienceOnly`, the issuer is not an accepted audience, so only + * 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. */ export async function verifyClientAssertion( ctx: GenericEndpointContext, @@ -460,7 +468,8 @@ export async function verifyClientAssertion( clientAssertionType: string, clientIdHint?: string, expectedAudience?: string, -): Promise<{ clientId: string }> { + options?: { endpointAudienceOnly?: boolean }, +): Promise<{ clientId: string; client: SchemaClient }> { if (clientAssertionType !== CLIENT_ASSERTION_TYPE) { throw new APIError("BAD_REQUEST", { error_description: "unsupported client_assertion_type", @@ -539,7 +548,9 @@ export async function verifyClientAssertion( const endpointAudience = expectedAudience ?? `${ctx.context.baseURL}/oauth2/token`; const issuerAudience = getIssuer(ctx, opts); - const acceptedAudiences = [...new Set([endpointAudience, issuerAudience])]; + const acceptedAudiences = options?.endpointAudienceOnly + ? [endpointAudience] + : [...new Set([endpointAudience, issuerAudience])]; const verifyOpts = { issuer: clientId, subject: clientId, @@ -614,5 +625,5 @@ export async function verifyClientAssertion( expectedAudience: acceptedAudience, }); - return { clientId }; + return { clientId, client }; }