feat(oauth-provider): pass client ID to privilege checks (#11658)

.changeset/oauth-provider-client-privileges-client-id.md

@@ -0,0 +1,5 @@
+---
+"@better-auth/oauth-provider": patch
+---
+
+If you configure `clientPrivileges`, the callback now receives `clientId` when a user reads, updates, deletes, rotates the secret of, or configures scopes on an existing client. You can use it to allow or deny actions per client among users who share a `clientReference`. Before, the callback could not tell which client the request targeted. `clientId` is absent for creating and listing clients, and the callback still cannot grant access to a client owned by someone else, apart from the existing scope-only updates through `adminUpdateOAuthClient`. Existing callbacks keep working unchanged.

docs/content/docs/plugins/oauth-provider.mdx

@@ -1706,6 +1706,8 @@ To set user-specific permissions and roles on tokens see [Claims](#claims).
 
 To determine whether a logged in user has the ability to perform specific actions in client creation, you can utilize the `clientPrivileges` configuration setting. By default, CRUD actions are allowed for users with matching `userId` or `clientReference`.
 
+The callback can only deny an action. Client CRUD still checks ownership after the callback, so returning `true` cannot give a user access to a client owned by another user or another `clientReference`. The one exception is scope configuration on another owner's client through `adminUpdateOAuthClient`: when the update changes only `client_credentials_scopes`, the callback's `update` and `configure-client-credentials-scopes` decisions authorize it without an ownership check.
+
 The following is a basic example that allows all OAuth Client CRUD actions for organization owners assuming ordinary users cannot create clients:
 
 ```ts title="auth.ts"
@@ -1720,6 +1722,28 @@ oauthProvider({
 })
 ```
 
+For actions on an existing client (`read`, `update`, `delete`, `rotate`, and `configure-client-credentials-scopes`), the callback also receives `clientId`. It is absent for `create`, `list`, and scope configuration during creation. `clientId` is the `client_id` from the request, so the client may not exist and the value proves nothing about ownership.
+
+Use `clientId` to restrict individual clients among users who share a `clientReference`, such as the members of an organization. In this example, every member can create, list, and read the organization's clients, but only a client's maintainers can change it or configure its scopes. `getClientMaintainers` stands for your own lookup:
+
+```ts title="auth.ts"
+oauthProvider({
+  clientReference: ({ session }) => {
+    return (session?.activeOrganizationId as string | undefined) ?? undefined;
+  },
+  clientPrivileges: async ({ action, user, clientId }) => {
+    if (!user) return false;
+    if (action === "create" || action === "list" || action === "read") {
+      return true;
+    }
+    // Scope configuration during creation has no client ID, so deny it here.
+    if (!clientId) return false;
+    const maintainers = await getClientMaintainers(clientId);
+    return maintainers.includes(user.id);
+  },
+})
+```
+
 ### Storage
 
 By default all secrets are `hashed` by default on the database. This helps protect the `client_secret` in case of a database leak.

packages/oauth-provider/src/oauthClient/endpoints-privileges.test.ts

@@ -5,8 +5,13 @@ import { getTestInstance } from "better-auth/test";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { oauthProviderClient } from "../client";
 import { oauthProvider } from "../oauth";
+import type { OAuthOptions } from "../types";
 import type { OAuthClient } from "../types/oauth";
 
+type ClientPrivilegeContext = Parameters<
+	NonNullable<OAuthOptions["clientPrivileges"]>
+>[0];
+
 type TestOAuthClientUiMetadata = Pick<
 	OAuthClient,
 	| "client_name"
@@ -32,7 +37,7 @@ describe("oauthClient", async () => {
 		password: "test123456",
 		name: "forbidden user",
 	};
-	const clientPrivileges = vi.fn(({ user }) => {
+	const clientPrivileges = vi.fn(({ user }: ClientPrivilegeContext) => {
 		if (user?.email === allowedUser.email) {
 			return true;
 		}
@@ -144,6 +149,7 @@ describe("oauthClient", async () => {
 		expect(client?.data?.client_id_issued_at).toBeDefined();
 
 		expect(clientPrivileges).toHaveBeenCalledTimes(1);
+		expect(clientPrivileges.mock.calls[0]?.[0]?.clientId).toBeUndefined();
 	});
 
 	it("should not create client with forbidden user via admin api", async () => {
@@ -219,6 +225,30 @@ describe("oauthClient", async () => {
 			}),
 		);
 		expect(clientPrivileges).toHaveBeenCalledTimes(2);
+		expect(clientPrivileges.mock.calls[1]?.[0]?.clientId).toBeUndefined();
+
+		clientPrivileges.mockClear();
+		await auth.api.adminUpdateOAuthClient({
+			headers: allowedUserHeaders,
+			body: {
+				client_id: adminClient.client_id,
+				update: { client_credentials_scopes: ["m2m:read"] },
+			},
+		});
+		expect(clientPrivileges).toHaveBeenNthCalledWith(
+			1,
+			expect.objectContaining({
+				action: "update",
+				clientId: adminClient.client_id,
+			}),
+		);
+		expect(clientPrivileges).toHaveBeenNthCalledWith(
+			2,
+			expect.objectContaining({
+				action: "configure-client-credentials-scopes",
+				clientId: adminClient.client_id,
+			}),
+		);
 		await allowedAuthClient.oauth2.deleteClient({
 			client_id: adminClient.client_id,
 		});
@@ -309,6 +339,12 @@ describe("oauthClient", async () => {
 		expect(check).toMatchObject(expected);
 
 		expect(clientPrivileges).toHaveBeenCalledTimes(1);
+		expect(clientPrivileges).toHaveBeenCalledWith(
+			expect.objectContaining({
+				action: "read",
+				clientId: oauthClient.client_id,
+			}),
+		);
 	});
 
 	it("should get public-only information about a client with any user", async () => {
@@ -356,6 +392,7 @@ describe("oauthClient", async () => {
 		expect(check).toMatchObject(expected);
 
 		expect(clientPrivileges).toHaveBeenCalledTimes(1);
+		expect(clientPrivileges.mock.calls[0]?.[0]?.clientId).toBeUndefined();
 	});
 
 	it("should not update the client with forbidden user", async () => {
@@ -393,6 +430,12 @@ describe("oauthClient", async () => {
 		oauthClient = client.data!;
 
 		expect(clientPrivileges).toHaveBeenCalledTimes(1);
+		expect(clientPrivileges).toHaveBeenCalledWith(
+			expect.objectContaining({
+				action: "update",
+				clientId: oauthClient.client_id,
+			}),
+		);
 	});
 
 	it("should not rotate the client secret with forbidden user", async () => {
@@ -420,6 +463,12 @@ describe("oauthClient", async () => {
 		oauthClient = client.data!;
 
 		expect(clientPrivileges).toHaveBeenCalledTimes(1);
+		expect(clientPrivileges).toHaveBeenCalledWith(
+			expect.objectContaining({
+				action: "rotate",
+				clientId: oauthClient.client_id,
+			}),
+		);
 	});
 
 	it("should not delete the client with forbidden user", async () => {
@@ -442,6 +491,12 @@ describe("oauthClient", async () => {
 		expect(client.data).toBeNull();
 
 		expect(clientPrivileges).toHaveBeenCalledTimes(1);
+		expect(clientPrivileges).toHaveBeenCalledWith(
+			expect.objectContaining({
+				action: "delete",
+				clientId: oauthClient.client_id,
+			}),
+		);
 	});
 
 	it("should not create client via admin api without a session", async () => {
@@ -583,3 +638,192 @@ describe("oauthClient dynamic registration privileges", async () => {
 		expect(clientPrivileges).not.toHaveBeenCalled();
 	});
 });
+
+/**
+ * @see https://github.com/better-auth/better-auth/issues/11589
+ */
+describe("clientPrivileges client ID", async () => {
+	const redirectUri = "https://rp.example.com/callback";
+	const owner = {
+		email: "owner@test.com",
+		password: "test123456",
+		name: "owner",
+	};
+	const collaborator = {
+		email: "collaborator@test.com",
+		password: "test123456",
+		name: "collaborator",
+	};
+	let restrictedClientId = "";
+	const clientPrivileges = vi.fn(
+		({ user, action, clientId }: ClientPrivilegeContext) =>
+			user?.email !== collaborator.email ||
+			action === "read" ||
+			clientId !== restrictedClientId,
+	);
+	const { auth, signInWithUser } = await getTestInstance({
+		plugins: [
+			oauthProvider({
+				loginPage: "/login",
+				consentPage: "/consent",
+				clientReference: () => "shared-organization",
+				clientPrivileges,
+			}),
+			jwt(),
+		],
+	});
+	await auth.api.signUpEmail({ body: owner });
+	await auth.api.signUpEmail({ body: collaborator });
+	const { headers: ownerHeaders } = await signInWithUser(
+		owner.email,
+		owner.password,
+	);
+	const { headers: collaboratorHeaders } = await signInWithUser(
+		collaborator.email,
+		collaborator.password,
+	);
+	const restrictedClient = await auth.api.adminCreateOAuthClient({
+		headers: ownerHeaders,
+		body: { redirect_uris: [redirectUri] },
+	});
+	restrictedClientId = restrictedClient.client_id;
+
+	beforeEach(() => {
+		clientPrivileges.mockClear();
+	});
+
+	it("should let a collaborator read but not change a restricted shared client", async () => {
+		const client = await auth.api.getOAuthClient({
+			headers: collaboratorHeaders,
+			query: { client_id: restrictedClientId },
+		});
+		expect(client.client_id).toBe(restrictedClientId);
+
+		await expect(
+			auth.api.updateOAuthClient({
+				headers: collaboratorHeaders,
+				body: {
+					client_id: restrictedClientId,
+					update: { client_name: "renamed" },
+				},
+			}),
+		).rejects.toMatchObject({ statusCode: 401 });
+		await expect(
+			auth.api.rotateClientSecret({
+				headers: collaboratorHeaders,
+				body: { client_id: restrictedClientId },
+			}),
+		).rejects.toMatchObject({ statusCode: 401 });
+		await expect(
+			auth.api.deleteOAuthClient({
+				headers: collaboratorHeaders,
+				body: { client_id: restrictedClientId },
+			}),
+		).rejects.toMatchObject({ statusCode: 401 });
+
+		for (const action of ["read", "update", "rotate", "delete"]) {
+			expect(clientPrivileges).toHaveBeenCalledWith(
+				expect.objectContaining({ action, clientId: restrictedClientId }),
+			);
+		}
+		const stored = await auth.api.getOAuthClient({
+			headers: ownerHeaders,
+			query: { client_id: restrictedClientId },
+		});
+		expect(stored.client_name).toBeUndefined();
+	});
+
+	it("should let a collaborator change another shared client", async () => {
+		const otherClient = await auth.api.adminCreateOAuthClient({
+			headers: ownerHeaders,
+			body: { redirect_uris: [redirectUri] },
+		});
+
+		const updated = await auth.api.updateOAuthClient({
+			headers: collaboratorHeaders,
+			body: {
+				client_id: otherClient.client_id,
+				update: { client_name: "renamed" },
+			},
+		});
+
+		expect(updated.client_name).toBe("renamed");
+		expect(clientPrivileges).toHaveBeenCalledWith(
+			expect.objectContaining({
+				action: "update",
+				clientId: otherClient.client_id,
+			}),
+		);
+	});
+
+	it("should pass an unknown client ID to the callback before the lookup", async () => {
+		await expect(
+			auth.api.getOAuthClient({
+				headers: ownerHeaders,
+				query: { client_id: "unknown-client" },
+			}),
+		).rejects.toMatchObject({ statusCode: 404 });
+		expect(clientPrivileges).toHaveBeenCalledWith(
+			expect.objectContaining({ action: "read", clientId: "unknown-client" }),
+		);
+	});
+});
+
+describe("clientPrivileges client ID across owners", async () => {
+	const clientPrivileges = vi.fn((_context: ClientPrivilegeContext) => true);
+	const { auth, signInWithUser } = await getTestInstance({
+		plugins: [
+			oauthProvider({
+				loginPage: "/login",
+				consentPage: "/consent",
+				scopes: ["m2m:read"],
+				clientPrivileges,
+			}),
+			jwt(),
+		],
+	});
+	const owner = {
+		email: "client-owner@test.com",
+		password: "test123456",
+		name: "client owner",
+	};
+	const administrator = {
+		email: "administrator@test.com",
+		password: "test123456",
+		name: "administrator",
+	};
+	await auth.api.signUpEmail({ body: owner });
+	await auth.api.signUpEmail({ body: administrator });
+	const { headers: ownerHeaders } = await signInWithUser(
+		owner.email,
+		owner.password,
+	);
+	const { headers: administratorHeaders } = await signInWithUser(
+		administrator.email,
+		administrator.password,
+	);
+
+	it("should pass the client ID when configuring scopes on another owner's client", async () => {
+		const client = await auth.api.adminCreateOAuthClient({
+			headers: ownerHeaders,
+			body: { grant_types: ["client_credentials"] },
+		});
+		clientPrivileges.mockClear();
+
+		const updated = await auth.api.adminUpdateOAuthClient({
+			headers: administratorHeaders,
+			body: {
+				client_id: client.client_id,
+				update: { client_credentials_scopes: ["m2m:read"] },
+			},
+		});
+
+		expect(updated.client_credentials_scopes).toEqual(["m2m:read"]);
+		expect(clientPrivileges).toHaveBeenCalledWith(
+			expect.objectContaining({
+				action: "configure-client-credentials-scopes",
+				clientId: client.client_id,
+			}),
+		);
+	});
+});

packages/oauth-provider/src/oauthClient/endpoints.ts

@@ -31,7 +31,7 @@ export async function getClientEndpoint(
 	opts: OAuthOptions<Scope[]>,
 ) {
 	const session = await getSessionFromCtx(ctx);
-	await assertClientPrivileges(ctx, session, opts, "read");
+	await assertClientPrivileges(ctx, session, opts, "read", ctx.query.client_id);
 	if (!session) throw new APIError("UNAUTHORIZED");
 
 	const client = await getClient(ctx, opts, ctx.query.client_id);
@@ -143,7 +143,13 @@ export async function deleteClientEndpoint(
 	opts: OAuthOptions<Scope[]>,
 ) {
 	const session = await getSessionFromCtx(ctx);
-	await assertClientPrivileges(ctx, session, opts, "delete");
+	await assertClientPrivileges(
+		ctx,
+		session,
+		opts,
+		"delete",
+		ctx.body.client_id,
+	);
 	if (!session) throw new APIError("UNAUTHORIZED");
 
 	const clientId = ctx.body.client_id;
@@ -216,7 +222,13 @@ export async function updateClientEndpoint(
 	OAuthClientAdministrativeResponse | OAuthClientRegistrationResponse
 > {
 	const session = await getSessionFromCtx(ctx);
-	await assertClientPrivileges(ctx, session, opts, "update");
+	await assertClientPrivileges(
+		ctx,
+		session,
+		opts,
+		"update",
+		ctx.body.client_id,
+	);
 	if (!session) throw new APIError("UNAUTHORIZED");
 
 	const clientId = ctx.body.client_id;
@@ -257,6 +269,7 @@ export async function updateClientEndpoint(
 			session,
 			opts,
 			"configure-client-credentials-scopes",
+			clientId,
 		);
 	}
 
@@ -298,6 +311,7 @@ export async function updateClientEndpoint(
 				session,
 				opts,
 				"configure-client-credentials-scopes",
+				clientId,
 			);
 		}
 	}
@@ -386,7 +400,13 @@ export async function rotateClientSecretEndpoint(
 	opts: OAuthOptions<Scope[]>,
 ) {
 	const session = await getSessionFromCtx(ctx);
-	await assertClientPrivileges(ctx, session, opts, "rotate");
+	await assertClientPrivileges(
+		ctx,
+		session,
+		opts,
+		"rotate",
+		ctx.body.client_id,
+	);
 	if (!session) throw new APIError("UNAUTHORIZED");
 
 	const clientId = ctx.body.client_id;

packages/oauth-provider/src/oauthClient/privileges.ts

@@ -25,6 +25,7 @@ export async function assertClientPrivileges(
 		| "list"
 		| "rotate"
 		| "configure-client-credentials-scopes",
+	clientId?: string,
 ) {
 	if (!session) throw new APIError("UNAUTHORIZED");
 	if (!ctx.headers) throw new APIError("BAD_REQUEST");
@@ -41,6 +42,7 @@ export async function assertClientPrivileges(
 			action,
 			session: session.session,
 			user: session.user,
+			clientId,
 		}))
 	) {
 		throw new APIError("UNAUTHORIZED");

packages/oauth-provider/src/public-types.test.ts

@@ -15,6 +15,9 @@ import { describe, expectTypeOf, it } from "vitest";
 type ClientPrivilegeAction = Parameters<
 	NonNullable<OAuthOptions["clientPrivileges"]>
 >[0]["action"];
+type ClientPrivilegeContext = Parameters<
+	NonNullable<OAuthOptions["clientPrivileges"]>
+>[0];
 
 describe("public oauth-provider types", () => {
 	it("exports the signed-query verifier with a server-side secret", () => {
@@ -75,5 +78,8 @@ describe("public oauth-provider types", () => {
 		expectTypeOf<SchemaClient>().not.toHaveProperty("type");
 		expectTypeOf<"configure-client-credentials-scopes">().toMatchTypeOf<ClientPrivilegeAction>();
 		expectTypeOf<"configure-client-credentials">().not.toMatchTypeOf<ClientPrivilegeAction>();
+		expectTypeOf<ClientPrivilegeContext["clientId"]>().toEqualTypeOf<
+			string | undefined
+		>();
 	});
 });

packages/oauth-provider/src/types/index.ts

@@ -943,6 +943,16 @@ export interface OAuthOptions<
 			| "configure-client-credentials-scopes";
 		user?: User & Record<string, unknown>;
 		session?: Session & Record<string, unknown>;
+		/**
+		 * The `client_id` from the request, for actions on an existing client.
+		 * Absent for `create`, `list`, and scope configuration during creation.
+		 * The client may not exist, and this value does not establish ownership.
+		 * Client CRUD enforces ownership except when `adminUpdateOAuthClient`
+		 * changes only `client_credentials_scopes`: the `update` and
+		 * `configure-client-credentials-scopes` decisions can authorize access
+		 * to another owner's client.
+		 */
+		clientId?: string;
 	}) => Awaitable<boolean | undefined>;
 	/**
 	 * Grant types supported by the token endpoint