feat: prevent privilege escalation

controlplane/src/core/bufservices/api-key/createAPIKey.ts

@@ -9,6 +9,7 @@ import { ApiKeyGenerator } from '../../services/ApiGenerator.js';
 import { enrichLogger, getLogger, handleError } from '../../util.js';
 import { OrganizationGroupRepository } from '../../repositories/OrganizationGroupRepository.js';
 import { UnauthorizedError } from '../../errors/errors.js';
+import { RBACEvaluator } from '../../services/RBACEvaluator.js';
 
 export function createAPIKey(
   opts: RouterOptions,
@@ -70,8 +71,18 @@ export function createAPIKey(
       };
     }
 
-    const generatedAPIKey = ApiKeyGenerator.generate();
+    const rbac = new RBACEvaluator([orgGroup]);
+    if (rbac.isOrganizationAdmin && !authContext.rbac.isOrganizationAdmin) {
+      return {
+        response: {
+          code: EnumStatusCode.ERR,
+          details: `You don't have access to create an API key with the group "${orgGroup.name}"`,
+        },
+        apiKey: '',
+      };
+    }
 
+    const generatedAPIKey = ApiKeyGenerator.generate();
     await apiKeyRepo.addAPIKey({
       name: keyName,
       organizationID: authContext.organizationId,

controlplane/src/core/bufservices/api-key/deleteAPIKey.ts

@@ -7,6 +7,8 @@ import { AuditLogRepository } from '../../repositories/AuditLogRepository.js';
 import type { RouterOptions } from '../../routes.js';
 import { enrichLogger, getLogger, handleError } from '../../util.js';
 import { UnauthorizedError } from '../../errors/errors.js';
+import { OrganizationGroupRepository } from '../../repositories/OrganizationGroupRepository.js';
+import { RBACEvaluator } from '../../services/RBACEvaluator.js';
 
 export function deleteAPIKey(
   opts: RouterOptions,
@@ -21,6 +23,7 @@ export function deleteAPIKey(
 
     const apiKeyRepo = new ApiKeyRepository(opts.db);
     const auditLogRepo = new AuditLogRepository(opts.db);
+    const groupRepo = new OrganizationGroupRepository(opts.db);
 
     if (authContext.organizationDeactivated) {
       throw new UnauthorizedError();
@@ -40,6 +43,26 @@ export function deleteAPIKey(
       throw new UnauthorizedError();
     }
 
+    if (apiKey.group?.id) {
+      const group = await groupRepo.byId({
+        organizationId: authContext.organizationId,
+        groupId: apiKey.group.id,
+      });
+
+      if (group) {
+        const rbac = new RBACEvaluator([group]);
+        if (rbac.isOrganizationAdmin && !authContext.rbac.isOrganizationAdmin) {
+          return {
+            response: {
+              code: EnumStatusCode.ERR,
+              details: `You don't have access to remove the API key "${apiKey.name}"`,
+            },
+            apiKey: '',
+          };
+        }
+      }
+    }
+
     await apiKeyRepo.removeAPIKey({
       name: req.name,
       organizationID: authContext.organizationId,

controlplane/src/core/bufservices/api-key/updateAPIKey.ts

@@ -8,6 +8,7 @@ import type { RouterOptions } from '../../routes.js';
 import { enrichLogger, getLogger, handleError } from '../../util.js';
 import { OrganizationGroupRepository } from '../../repositories/OrganizationGroupRepository.js';
 import { UnauthorizedError } from '../../errors/errors.js';
+import { RBACEvaluator } from '../../services/RBACEvaluator.js';
 
 export function updateAPIKey(
   opts: RouterOptions,
@@ -56,8 +57,18 @@ export function updateAPIKey(
       };
     }
 
-    await apiKeyRepo.updateAPIKeyGroup({ apiKeyId: apiKey.id, groupId: orgGroup.groupId });
+    const rbac = new RBACEvaluator([orgGroup]);
+    if (rbac.isOrganizationAdmin && !authContext.rbac.isOrganizationAdmin) {
+      return {
+        response: {
+          code: EnumStatusCode.ERR,
+          details: `You don't have access to update the API key group to "${orgGroup.name}"`,
+        },
+        apiKey: '',
+      };
+    }
 
+    await apiKeyRepo.updateAPIKeyGroup({ apiKeyId: apiKey.id, groupId: orgGroup.groupId });
     await auditLogRepo.addAuditLog({
       organizationId: authContext.organizationId,
       organizationSlug: authContext.organizationSlug,

controlplane/src/core/repositories/ApiKeyRepository.ts

@@ -87,6 +87,7 @@ export class ApiKeyRepository {
         expiresAt: apiKeys.expiresAt,
         createdBy: users.email,
         creatorUserID: users.id,
+        groupId: apiKeys.groupId,
       })
       .from(apiKeys)
       .innerJoin(users, eq(users.id, apiKeys.userId))
@@ -105,6 +106,7 @@ export class ApiKeyRepository {
       expiresAt: key[0].expiresAt?.toISOString() ?? '',
       createdBy: key[0].createdBy,
       creatorUserID: key[0].creatorUserID,
+      group: { id: key[0].groupId },
     } as APIKeyDTO;
   }
 

controlplane/test/api-keys.test.ts

@@ -110,20 +110,20 @@ describe('API Keys', (ctx) => {
     const orgGroupsResponse = await client.getOrganizationGroups({});
     expect(orgGroupsResponse.response?.code).toBe(EnumStatusCode.OK);
 
-    const adminGroup = orgGroupsResponse.groups.find((g) => g.name === 'admin')!;
     const developerGroup = orgGroupsResponse.groups.find((g) => g.name === 'developer')!;
+    const viewerGroup = orgGroupsResponse.groups.find((g) => g.name === 'viewer')!;
 
     authenticator.changeUserWithSuppliedContext({
       ...users[TestUser.adminAliceCompanyA],
       rbac: createTestRBACEvaluator(createTestGroup({ role: role as OrganizationRole })),
     });
 
-    // Create the API key with the `admin` group
+    // Create the API key with the `viewer` group
     const apiKeyName = uid();
     const createApiKeyResponse = await client.createAPIKey({
       name: apiKeyName,
       expires: ExpiresAt.NEVER,
-      groupId: adminGroup.groupId,
+      groupId: viewerGroup.groupId,
     });
 
     expect(createApiKeyResponse.response?.code).toBe(EnumStatusCode.OK);
@@ -136,7 +136,7 @@ describe('API Keys', (ctx) => {
 
     expect(updateApiKeyResponse.response?.code).toBe(EnumStatusCode.OK);
 
-    // Ensure that the API key have the correct group
+    // Ensure that the API key has the correct group
     let getApiKeysResponse = await client.getAPIKeys({});
     let apiKey = getApiKeysResponse.apiKeys?.find((k) => k.name === apiKeyName);
 
@@ -150,7 +150,7 @@ describe('API Keys', (ctx) => {
 
     expect(deleteApiKeyResponse.response?.code).toBe(EnumStatusCode.OK);
 
-    // Ensure the API key have been deleted
+    // Ensure the API key has been deleted
     getApiKeysResponse = await client.getAPIKeys({});
     apiKey = getApiKeysResponse.apiKeys?.find((k) => k.name === apiKeyName);
 
@@ -160,6 +160,104 @@ describe('API Keys', (ctx) => {
     await server.close();
   });
 
+  test('that an "organization-apikey-manager" cannot create API keys with admin role', async () => {
+    const { client, server, users, authenticator } = await SetupTest({ dbname, enableMultiUsers: true });
+
+    const orgGroupsResponse = await client.getOrganizationGroups({});
+    expect(orgGroupsResponse.response?.code).toBe(EnumStatusCode.OK);
+
+    const adminGroup = orgGroupsResponse.groups.find((g) => g.name === 'admin')!;
+
+    authenticator.changeUserWithSuppliedContext({
+      ...users[TestUser.adminAliceCompanyA],
+      rbac: createTestRBACEvaluator(createTestGroup({ role: 'organization-apikey-manager' })),
+    });
+
+    // Create the API key with the `admin` group
+    const apiKeyName = uid();
+    const createApiKeyResponse = await client.createAPIKey({
+      name: apiKeyName,
+      expires: ExpiresAt.NEVER,
+      groupId: adminGroup.groupId,
+    });
+
+    expect(createApiKeyResponse.response?.code).toBe(EnumStatusCode.ERR);
+    expect(createApiKeyResponse.response?.details).toBe(`You don't have access to create an API key with the group "admin"`);
+
+    await server.close();
+  });
+
+  test('that an "organization-apikey-manager" cannot update API keys with admin role', async () => {
+    const { client, server, users, authenticator } = await SetupTest({ dbname, enableMultiUsers: true });
+
+    const orgGroupsResponse = await client.getOrganizationGroups({});
+    expect(orgGroupsResponse.response?.code).toBe(EnumStatusCode.OK);
+
+    const adminGroup = orgGroupsResponse.groups.find((g) => g.name === 'admin')!;
+    const viewerGroup = orgGroupsResponse.groups.find((g) => g.name === 'viewer')!;
+
+    authenticator.changeUserWithSuppliedContext({
+      ...users[TestUser.adminAliceCompanyA],
+      rbac: createTestRBACEvaluator(createTestGroup({ role: 'organization-apikey-manager' })),
+    });
+
+    // Create the API key with the `viewer` group
+    const apiKeyName = uid();
+    const createApiKeyResponse = await client.createAPIKey({
+      name: apiKeyName,
+      expires: ExpiresAt.NEVER,
+      groupId: viewerGroup.groupId,
+    });
+
+    expect(createApiKeyResponse.response?.code).toBe(EnumStatusCode.OK);
+
+    // Update the API key to the `admin` group
+    const updateApiKeyResponse = await client.updateAPIKey({
+      name: apiKeyName,
+      groupId: adminGroup.groupId,
+    });
+
+    expect(updateApiKeyResponse.response?.code).toBe(EnumStatusCode.ERR);
+    expect(updateApiKeyResponse.response?.details).toBe(`You don't have access to update the API key group to "admin"`);
+
+    await server.close();
+  });
+
+  test('that an "organization-apikey-manager" cannot delete an API key with admin role', async () => {
+    const { client, server, users, authenticator } = await SetupTest({ dbname, enableMultiUsers: true });
+
+    const orgGroupsResponse = await client.getOrganizationGroups({});
+    expect(orgGroupsResponse.response?.code).toBe(EnumStatusCode.OK);
+
+    const adminGroup = orgGroupsResponse.groups.find((g) => g.name === 'admin')!;
+    authenticator.changeUserWithSuppliedContext({
+      ...users[TestUser.adminAliceCompanyA],
+    });
+
+    // Create the API key with the `admin` group
+    const apiKeyName = uid();
+    const createApiKeyResponse = await client.createAPIKey({
+      name: apiKeyName,
+      expires: ExpiresAt.NEVER,
+      groupId: adminGroup.groupId,
+    });
+
+    expect(createApiKeyResponse.response?.code).toBe(EnumStatusCode.OK);
+
+    // Try to delete the API key
+    authenticator.changeUserWithSuppliedContext({
+      ...users[TestUser.adminAliceCompanyA],
+      rbac: createTestRBACEvaluator(createTestGroup({ role: 'organization-apikey-manager' })),
+    });
+
+    const deleteApiKeyResponse = await client.deleteAPIKey({ name: apiKeyName });
+
+    expect(deleteApiKeyResponse.response?.code).toBe(EnumStatusCode.ERR);
+    expect(deleteApiKeyResponse.response?.details).toBe(`You don't have access to remove the API key "${apiKeyName}"`);
+
+    await server.close();
+  });
+
   test.each([
     'organization-developer',
     'organization-viewer',

studio/src/components/group-select.tsx

@@ -4,6 +4,7 @@ import { useQuery } from "@connectrpc/connect-query";
 import { getOrganizationGroups } from "@wundergraph/cosmo-connect/dist/platform/v1/platform-PlatformService_connectquery";
 import { EnumStatusCode } from "@wundergraph/cosmo-connect/dist/common/common_pb";
 import { Button } from "@/components/ui/button";
+import { useIsAdmin } from "@/hooks/use-is-admin";
 
 export function GroupSelect({ id, value, disabled = false, groups, onValueChange }: {
   id?: string;
@@ -12,6 +13,7 @@ export function GroupSelect({ id, value, disabled = false, groups, onValueChange
   groups?: OrganizationGroup[];
   onValueChange(group: OrganizationGroup): void;
 }) {
+  const isAdmin = useIsAdmin();
   const { data, isPending, error, refetch } = useQuery(getOrganizationGroups, {}, { enabled: groups === undefined });
   if (isPending) {
     return (
@@ -60,6 +62,7 @@ export function GroupSelect({ id, value, disabled = false, groups, onValueChange
           <SelectItem
             key={`group-${group.groupId}`}
             value={group.groupId}
+            disabled={!isAdmin && group.rules.some((rule) => rule.role === 'organization-admin')}
           >
             {group.name}
           </SelectItem>

studio/src/pages/[organizationSlug]/members.tsx

@@ -132,7 +132,7 @@ const InviteForm = ({ onSuccess }: { onSuccess: () => void }) => {
       </div>
 
       <div className="space-y-2">
-        <span>What group should the member be added to?</span>
+        <span>What group(s) should the member be added to?</span>
         <MultiGroupSelect
           disabled={isPending}
           value={selectedGroups}