From 86b61ef1d82559cbe2065935ef22545e85747228 Mon Sep 17 00:00:00 2001
From: Jaayden Halko <jaayden.halko@gmail.com>
Date: Mon, 10 Mar 2025 22:43:09 +0000
Subject: [PATCH] fix: use correct permissions for CRUD of custom roles
 (#16854)

resolves coder/internal#428

The goal of the PR is to start using updateOrgRoles and deleteOrgRoles
permissions to gate custom roles functionality

```
		updateOrgRoles: {
			object: {
				resource_type: "assign_org_role",
				organization_id: organizationId,
			},
			action: "update",
		},
		deleteOrgRoles: {
			object: {
				resource_type: "assign_org_role",
				organization_id: organizationId,
			},
			action: "delete",
		}
```
---
 site/src/modules/permissions/index.ts         |  3 +
 site/src/modules/permissions/organizations.ts | 14 +++++
 .../CustomRolesPage/CreateEditRolePage.tsx    |  6 +-
 .../CreateEditRolePageView.stories.tsx        |  2 -
 .../CreateEditRolePageView.tsx                | 60 +++++++++----------
 .../CustomRolesPage/CustomRolesPage.tsx       |  3 +-
 .../CustomRolesPageView.stories.tsx           |  4 +-
 .../CustomRolesPage/CustomRolesPageView.tsx   | 59 +++++++++++-------
 site/src/testHelpers/entities.ts              |  4 ++
 9 files changed, 93 insertions(+), 62 deletions(-)

diff --git a/site/src/modules/permissions/index.ts b/site/src/modules/permissions/index.ts
index 300edec9e5..98356aa34b 100644
--- a/site/src/modules/permissions/index.ts
+++ b/site/src/modules/permissions/index.ts
@@ -6,6 +6,9 @@ export type Permissions = {
 
 export type PermissionName = keyof typeof permissionChecks;
 
+/**
+ * Site-wide permission checks
+ */
 export const permissionChecks = {
 	viewAllUsers: {
 		object: {
diff --git a/site/src/modules/permissions/organizations.ts b/site/src/modules/permissions/organizations.ts
index 1b79e11e68..0a7cb505c2 100644
--- a/site/src/modules/permissions/organizations.ts
+++ b/site/src/modules/permissions/organizations.ts
@@ -73,6 +73,20 @@ export const organizationPermissionChecks = (organizationId: string) =>
 			},
 			action: "create",
 		},
+		updateOrgRoles: {
+			object: {
+				resource_type: "assign_org_role",
+				organization_id: organizationId,
+			},
+			action: "update",
+		},
+		deleteOrgRoles: {
+			object: {
+				resource_type: "assign_org_role",
+				organization_id: organizationId,
+			},
+			action: "delete",
+		},
 		viewProvisioners: {
 			object: {
 				resource_type: "provisioner_daemon",
diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePage.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePage.tsx
index 0d702b400e..271018da7e 100644
--- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePage.tsx
+++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePage.tsx
@@ -48,8 +48,9 @@ export const CreateEditRolePage: FC = () => {
 	return (
 		<RequirePermission
 			isFeatureVisible={
-				organizationPermissions.assignOrgRoles ||
-				organizationPermissions.createOrgRoles
+				role
+					? organizationPermissions.updateOrgRoles
+					: organizationPermissions.createOrgRoles
 			}
 		>
 			<Helmet>
@@ -87,7 +88,6 @@ export const CreateEditRolePage: FC = () => {
 						: createOrganizationRoleMutation.isLoading
 				}
 				organizationName={organizationName}
-				canAssignOrgRole={organizationPermissions.assignOrgRoles}
 			/>
 		</RequirePermission>
 	);
diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx
index c374aa33d5..9318238555 100644
--- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx
+++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx
@@ -23,7 +23,6 @@ export const Default: Story = {
 		error: undefined,
 		isLoading: false,
 		organizationName: "my-org",
-		canAssignOrgRole: true,
 	},
 };
 
@@ -81,7 +80,6 @@ export const InvalidCharsError: Story = {
 export const CannotEditRoleName: Story = {
 	args: {
 		...Default.args,
-		canAssignOrgRole: false,
 	},
 };
 
diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx
index 9e9d7f4e41..717904b4bd 100644
--- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx
+++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx
@@ -43,7 +43,6 @@ export type CreateEditRolePageViewProps = {
 	error?: unknown;
 	isLoading: boolean;
 	organizationName: string;
-	canAssignOrgRole: boolean;
 	allResources?: boolean;
 };
 
@@ -53,7 +52,6 @@ export const CreateEditRolePageView: FC<CreateEditRolePageViewProps> = ({
 	error,
 	isLoading,
 	organizationName,
-	canAssignOrgRole,
 	allResources = false,
 }) => {
 	const navigate = useNavigate();
@@ -84,26 +82,24 @@ export const CreateEditRolePageView: FC<CreateEditRolePageViewProps> = ({
 					title={`${role ? "Edit" : "Create"} Custom Role`}
 					description="Set a name and permissions for this role."
 				/>
-				{canAssignOrgRole && (
-					<div className="flex space-x-2 items-center">
-						<Button
-							variant="outline"
-							onClick={() => {
-								navigate(`/organizations/${organizationName}/roles`);
-							}}
-						>
-							Cancel
-						</Button>
-						<Button
-							onClick={() => {
-								form.handleSubmit();
-							}}
-						>
-							<Spinner loading={isLoading} />
-							{role !== undefined ? "Save" : "Create Role"}
-						</Button>
-					</div>
-				)}
+				<div className="flex space-x-2 items-center">
+					<Button
+						variant="outline"
+						onClick={() => {
+							navigate(`/organizations/${organizationName}/roles`);
+						}}
+					>
+						Cancel
+					</Button>
+					<Button
+						onClick={() => {
+							form.handleSubmit();
+						}}
+					>
+						<Spinner loading={isLoading} />
+						{role !== undefined ? "Save" : "Create Role"}
+					</Button>
+				</div>
 			</Stack>
 
 			<VerticalForm onSubmit={form.handleSubmit}>
@@ -135,18 +131,16 @@ export const CreateEditRolePageView: FC<CreateEditRolePageViewProps> = ({
 						allResources={allResources}
 					/>
 				</FormFields>
-				{canAssignOrgRole && (
-					<FormFooter>
-						<Button onClick={onCancel} variant="outline">
-							Cancel
-						</Button>
+				<FormFooter>
+					<Button onClick={onCancel} variant="outline">
+						Cancel
+					</Button>
 
-						<Button type="submit" disabled={isLoading}>
-							<Spinner loading={isLoading} />
-							{role ? "Save role" : "Create Role"}
-						</Button>
-					</FormFooter>
-				)}
+					<Button type="submit" disabled={isLoading}>
+						<Spinner loading={isLoading} />
+						{role ? "Save role" : "Create Role"}
+					</Button>
+				</FormFooter>
 			</VerticalForm>
 		</>
 	);
diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPage.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPage.tsx
index 67d511c066..fc5ec83e12 100644
--- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPage.tsx
+++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPage.tsx
@@ -81,8 +81,9 @@ export const CustomRolesPage: FC = () => {
 					builtInRoles={builtInRoles}
 					customRoles={customRoles}
 					onDeleteRole={setRoleToDelete}
-					canAssignOrgRole={organizationPermissions?.assignOrgRoles ?? false}
 					canCreateOrgRole={organizationPermissions?.createOrgRoles ?? false}
+					canUpdateOrgRole={organizationPermissions?.updateOrgRoles ?? false}
+					canDeleteOrgRole={organizationPermissions?.deleteOrgRoles ?? false}
 					isCustomRolesEnabled={isCustomRolesEnabled}
 				/>
 
diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.stories.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.stories.tsx
index 79319c8886..14ffbfa85b 100644
--- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.stories.tsx
+++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.stories.tsx
@@ -11,7 +11,6 @@ const meta: Meta<typeof CustomRolesPageView> = {
 	args: {
 		builtInRoles: [MockRoleWithOrgPermissions],
 		customRoles: [MockRoleWithOrgPermissions],
-		canAssignOrgRole: true,
 		canCreateOrgRole: true,
 		isCustomRolesEnabled: true,
 	},
@@ -31,7 +30,7 @@ export const NotEnabled: Story = {
 export const NotEnabledEmptyTable: Story = {
 	args: {
 		customRoles: [],
-		canAssignOrgRole: true,
+		canCreateOrgRole: true,
 		isCustomRolesEnabled: false,
 	},
 };
@@ -58,7 +57,6 @@ export const EmptyDisplayName: Story = {
 export const EmptyTableUserWithoutPermission: Story = {
 	args: {
 		customRoles: [],
-		canAssignOrgRole: false,
 		canCreateOrgRole: false,
 	},
 };
diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.tsx
index c770d73966..d2eebac62e 100644
--- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.tsx
+++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.tsx
@@ -34,8 +34,9 @@ interface CustomRolesPageViewProps {
 	builtInRoles: AssignableRoles[] | undefined;
 	customRoles: AssignableRoles[] | undefined;
 	onDeleteRole: (role: Role) => void;
-	canAssignOrgRole: boolean;
 	canCreateOrgRole: boolean;
+	canUpdateOrgRole: boolean;
+	canDeleteOrgRole: boolean;
 	isCustomRolesEnabled: boolean;
 }
 
@@ -43,8 +44,9 @@ export const CustomRolesPageView: FC<CustomRolesPageViewProps> = ({
 	builtInRoles,
 	customRoles,
 	onDeleteRole,
-	canAssignOrgRole,
 	canCreateOrgRole,
+	canUpdateOrgRole,
+	canDeleteOrgRole,
 	isCustomRolesEnabled,
 }) => {
 	return (
@@ -77,7 +79,9 @@ export const CustomRolesPageView: FC<CustomRolesPageViewProps> = ({
 			<RoleTable
 				roles={customRoles}
 				isCustomRolesEnabled={isCustomRolesEnabled}
-				canAssignOrgRole={canAssignOrgRole}
+				canCreateOrgRole={canCreateOrgRole}
+				canUpdateOrgRole={canUpdateOrgRole}
+				canDeleteOrgRole={canDeleteOrgRole}
 				onDeleteRole={onDeleteRole}
 			/>
 			<span>
@@ -90,7 +94,9 @@ export const CustomRolesPageView: FC<CustomRolesPageViewProps> = ({
 			<RoleTable
 				roles={builtInRoles}
 				isCustomRolesEnabled={isCustomRolesEnabled}
-				canAssignOrgRole={canAssignOrgRole}
+				canCreateOrgRole={canCreateOrgRole}
+				canUpdateOrgRole={canUpdateOrgRole}
+				canDeleteOrgRole={canDeleteOrgRole}
 				onDeleteRole={onDeleteRole}
 			/>
 		</Stack>
@@ -100,15 +106,19 @@ export const CustomRolesPageView: FC<CustomRolesPageViewProps> = ({
 interface RoleTableProps {
 	roles: AssignableRoles[] | undefined;
 	isCustomRolesEnabled: boolean;
-	canAssignOrgRole: boolean;
+	canCreateOrgRole: boolean;
+	canUpdateOrgRole: boolean;
+	canDeleteOrgRole: boolean;
 	onDeleteRole: (role: Role) => void;
 }
 
 const RoleTable: FC<RoleTableProps> = ({
 	roles,
 	isCustomRolesEnabled,
+	canCreateOrgRole,
+	canUpdateOrgRole,
+	canDeleteOrgRole,
 	onDeleteRole,
-	canAssignOrgRole,
 }) => {
 	const isLoading = roles === undefined;
 	const isEmpty = Boolean(roles && roles.length === 0);
@@ -134,14 +144,14 @@ const RoleTable: FC<RoleTableProps> = ({
 									<EmptyState
 										message="No custom roles yet"
 										description={
-											canAssignOrgRole && isCustomRolesEnabled
+											canCreateOrgRole && isCustomRolesEnabled
 												? "Create your first custom role"
 												: !isCustomRolesEnabled
 													? "Upgrade to a premium license to create a custom role"
 													: "You don't have permission to create a custom role"
 										}
 										cta={
-											canAssignOrgRole &&
+											canCreateOrgRole &&
 											isCustomRolesEnabled && (
 												<Button
 													component={RouterLink}
@@ -165,7 +175,8 @@ const RoleTable: FC<RoleTableProps> = ({
 									<RoleRow
 										key={role.name}
 										role={role}
-										canAssignOrgRole={canAssignOrgRole}
+										canUpdateOrgRole={canUpdateOrgRole}
+										canDeleteOrgRole={canDeleteOrgRole}
 										onDelete={() => onDeleteRole(role)}
 									/>
 								))}
@@ -179,11 +190,17 @@ const RoleTable: FC<RoleTableProps> = ({
 
 interface RoleRowProps {
 	role: AssignableRoles;
+	canUpdateOrgRole: boolean;
+	canDeleteOrgRole: boolean;
 	onDelete: () => void;
-	canAssignOrgRole: boolean;
 }
 
-const RoleRow: FC<RoleRowProps> = ({ role, onDelete, canAssignOrgRole }) => {
+const RoleRow: FC<RoleRowProps> = ({
+	role,
+	onDelete,
+	canUpdateOrgRole,
+	canDeleteOrgRole,
+}) => {
 	const navigate = useNavigate();
 
 	return (
@@ -195,20 +212,22 @@ const RoleRow: FC<RoleRowProps> = ({ role, onDelete, canAssignOrgRole }) => {
 			</TableCell>
 
 			<TableCell>
-				{!role.built_in && (
+				{!role.built_in && (canUpdateOrgRole || canDeleteOrgRole) && (
 					<MoreMenu>
 						<MoreMenuTrigger>
 							<ThreeDotsButton />
 						</MoreMenuTrigger>
 						<MoreMenuContent>
-							<MoreMenuItem
-								onClick={() => {
-									navigate(role.name);
-								}}
-							>
-								Edit
-							</MoreMenuItem>
-							{canAssignOrgRole && (
+							{canUpdateOrgRole && (
+								<MoreMenuItem
+									onClick={() => {
+										navigate(role.name);
+									}}
+								>
+									Edit
+								</MoreMenuItem>
+							)}
+							{canDeleteOrgRole && (
 								<MoreMenuItem danger onClick={onDelete}>
 									Delete&hellip;
 								</MoreMenuItem>
diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts
index 69f2544192..d2125baab3 100644
--- a/site/src/testHelpers/entities.ts
+++ b/site/src/testHelpers/entities.ts
@@ -2900,6 +2900,8 @@ export const MockOrganizationPermissions: OrganizationPermissions = {
 	viewOrgRoles: true,
 	createOrgRoles: true,
 	assignOrgRoles: true,
+	updateOrgRoles: true,
+	deleteOrgRoles: true,
 	viewProvisioners: true,
 	viewProvisionerJobs: true,
 	viewIdpSyncSettings: true,
@@ -2916,6 +2918,8 @@ export const MockNoOrganizationPermissions: OrganizationPermissions = {
 	viewOrgRoles: false,
 	createOrgRoles: false,
 	assignOrgRoles: false,
+	updateOrgRoles: false,
+	deleteOrgRoles: false,
 	viewProvisioners: false,
 	viewProvisionerJobs: false,
 	viewIdpSyncSettings: false,