RBAC: Fix various ui issues for role picker (#53589) (#53644)

* RolePicker: Check if user has permissions to delegate roles

* UserRolePicker: Require both UserRolsAdd and UserRolesRemove for chaning
roles

* RolePicker: Add option for controlling if roles can be updated to
RolePicker

* UserOrgs: Dont try to fetch roles with wrong permission

* RolePicker: make usage consistent with allowed actions

* RolePickerMenu: Remove unused property

* UserOrgs: Check for correct permission

* UserRolePicker: add apply to controll apply behaviour instand of
updateDisabled

* UserOrgs: Check for correct permission when trying to update roles

* RolePicker: Disable button if no roles or org role can be updated

* RolePicker: Always close on update

* RolePicker: Fix issue with updating immutable value

* ServiceAccountsListItem: Pass correct value to RolePicker

* ServiceAccountCreatePage: Pass correct value

(cherry picked from commit 7e16d5b4b4)

Co-authored-by: Karl Persson <kalle.persson@grafana.com>

public/app/core/components/RolePicker/RolePicker.tsx

@@ -18,7 +18,8 @@ export interface Props {
   showBuiltInRole?: boolean;
   onRolesChange: (newRoles: Role[]) => void;
   onBuiltinRoleChange?: (newRole: OrgRole) => void;
-  updateDisabled?: boolean;
+  canUpdateRoles?: boolean;
+  apply?: boolean;
 }
 
 export const RolePicker = ({
@@ -31,7 +32,8 @@ export const RolePicker = ({
   showBuiltInRole,
   onRolesChange,
   onBuiltinRoleChange,
-  updateDisabled,
+  canUpdateRoles = true,
+  apply = false,
 }: Props): JSX.Element | null => {
   const [isOpen, setOpen] = useState(false);
   const [selectedRoles, setSelectedRoles] = useState<Role[]>(appliedRoles);
@@ -109,16 +111,21 @@ export const RolePicker = ({
     if (onBuiltinRoleChange && newBuiltInRole && newBuiltInRole !== builtInRole) {
       onBuiltinRoleChange(newBuiltInRole);
     }
-    onRolesChange(newRoles);
-    setOpen(false);
+    if (canUpdateRoles) {
+      onRolesChange(newRoles);
+    }
     setQuery('');
+    setOpen(false);
   };
 
   const getOptions = () => {
+    // if roles cannot be updated mark every role as non delegatable
+    const options = roleOptions.map((r) => ({ ...r, delegatable: canUpdateRoles && r.delegatable }));
+
     if (query && query.trim() !== '') {
-      return roleOptions.filter((option) => option.name?.toLowerCase().includes(query.toLowerCase()));
+      return options.filter((option) => option.name?.toLowerCase().includes(query.toLowerCase()));
     }
-    return roleOptions;
+    return options;
   };
 
   if (isLoading) {
@@ -155,7 +162,8 @@ export const RolePicker = ({
             showGroups={query.length === 0 || query.trim() === ''}
             builtinRolesDisabled={builtinRolesDisabled}
             showBuiltInRole={showBuiltInRole}
-            updateDisabled={updateDisabled || false}
+            updateDisabled={builtinRolesDisabled && !canUpdateRoles}
+            apply={apply}
             offset={offset}
           />
         )}

public/app/core/components/RolePicker/RolePickerMenu.tsx

@@ -40,8 +40,8 @@ interface RolePickerMenuProps {
   onSelect: (roles: Role[]) => void;
   onBuiltInRoleSelect?: (role: OrgRole) => void;
   onUpdate: (newRoles: Role[], newBuiltInRole?: OrgRole) => void;
-  onClear?: () => void;
   updateDisabled?: boolean;
+  apply?: boolean;
   offset: { vertical: number; horizontal: number };
 }
 
@@ -55,9 +55,9 @@ export const RolePickerMenu = ({
   onSelect,
   onBuiltInRoleSelect,
   onUpdate,
-  onClear,
   updateDisabled,
   offset,
+  apply,
 }: RolePickerMenuProps): JSX.Element => {
   const [selectedOptions, setSelectedOptions] = useState<Role[]>(appliedRoles);
   const [selectedBuiltInRole, setSelectedBuiltInRole] = useState<OrgRole | undefined>(builtInRole);
@@ -153,9 +153,6 @@ export const RolePickerMenu = ({
   };
 
   const onClearInternal = async () => {
-    if (onClear) {
-      onClear();
-    }
     setSelectedOptions([]);
   };
 
@@ -272,11 +269,11 @@ export const RolePickerMenu = ({
         </CustomScrollbar>
         <div className={customStyles.menuButtonRow}>
           <HorizontalGroup justify="flex-end">
-            <Button size="sm" fill="text" onClick={onClearInternal}>
+            <Button size="sm" fill="text" onClick={onClearInternal} disabled={updateDisabled}>
               Clear all
             </Button>
-            <Button size="sm" onClick={onUpdateInternal}>
-              {updateDisabled ? `Apply` : `Update`}
+            <Button size="sm" onClick={onUpdateInternal} disabled={updateDisabled}>
+              {apply ? `Apply` : `Update`}
             </Button>
           </HorizontalGroup>
         </div>

public/app/core/components/RolePicker/TeamRolePicker.tsx

@@ -1,7 +1,8 @@
 import React, { FC, useEffect } from 'react';
 import { useAsyncFn } from 'react-use';
 
-import { Role } from 'app/types';
+import { contextSrv } from 'app/core/core';
+import { Role, AccessControlAction } from 'app/types';
 
 import { RolePicker } from './RolePicker';
 // @ts-ignore
@@ -35,6 +36,10 @@ export const TeamRolePicker: FC<Props> = ({ teamId, orgId, roleOptions, disabled
     await getTeamRoles();
   };
 
+  const canUpdateRoles =
+    contextSrv.hasPermission(AccessControlAction.ActionTeamsRolesAdd) &&
+    contextSrv.hasPermission(AccessControlAction.ActionTeamsRolesRemove);
+
   return (
     <RolePicker
       onRolesChange={onRolesChange}
@@ -43,6 +48,7 @@ export const TeamRolePicker: FC<Props> = ({ teamId, orgId, roleOptions, disabled
       isLoading={loading}
       disabled={disabled}
       builtinRolesDisabled={builtinRolesDisabled}
+      canUpdateRoles={canUpdateRoles}
     />
   );
 };

public/app/core/components/RolePicker/UserRolePicker.tsx

@@ -16,7 +16,7 @@ export interface Props {
   builtInRoles?: { [key: string]: Role[] };
   disabled?: boolean;
   builtinRolesDisabled?: boolean;
-  updateDisabled?: boolean;
+  apply?: boolean;
   onApplyRoles?: (newRoles: Role[], userId: number, orgId: number | undefined) => void;
   pendingRoles?: Role[];
 }
@@ -30,13 +30,13 @@ export const UserRolePicker: FC<Props> = ({
   builtInRoles,
   disabled,
   builtinRolesDisabled,
-  updateDisabled,
+  apply = false,
   onApplyRoles,
   pendingRoles,
 }) => {
   const [{ loading, value: appliedRoles = [] }, getUserRoles] = useAsyncFn(async () => {
     try {
-      if (updateDisabled) {
+      if (apply) {
         if (pendingRoles?.length! > 0) {
           return pendingRoles;
         }
@@ -59,16 +59,18 @@ export const UserRolePicker: FC<Props> = ({
   }, [orgId, getUserRoles, pendingRoles]);
 
   const onRolesChange = async (roles: Role[]) => {
-    if (!updateDisabled) {
+    if (!apply) {
       await updateUserRoles(roles, userId, orgId);
       await getUserRoles();
-    } else {
-      if (onApplyRoles) {
-        onApplyRoles(roles, userId, orgId);
-      }
+    } else if (onApplyRoles) {
+      onApplyRoles(roles, userId, orgId);
     }
   };
 
+  const canUpdateRoles =
+    contextSrv.hasPermission(AccessControlAction.ActionUserRolesAdd) &&
+    contextSrv.hasPermission(AccessControlAction.ActionUserRolesRemove);
+
   return (
     <RolePicker
       appliedRoles={appliedRoles}
@@ -81,7 +83,8 @@ export const UserRolePicker: FC<Props> = ({
       disabled={disabled}
       builtinRolesDisabled={builtinRolesDisabled}
       showBuiltInRole
-      updateDisabled={updateDisabled || false}
+      apply={apply}
+      canUpdateRoles={canUpdateRoles}
     />
   );
 };

public/app/features/admin/UserOrgs.tsx

@@ -154,11 +154,6 @@ class UnThemedOrgRow extends PureComponent<OrgRowProps> {
           .then((roles) => this.setState({ roleOptions: roles }))
           .catch((e) => console.error(e));
       }
-      if (contextSrv.hasPermission(AccessControlAction.ActionBuiltinRolesList)) {
-        fetchRoleOptions(this.props.org.orgId)
-          .then((roles) => this.setState({ builtInRoles: roles }))
-          .catch((e) => console.error(e));
-      }
     }
   }
 
@@ -166,7 +161,10 @@ class UnThemedOrgRow extends PureComponent<OrgRowProps> {
     const { org, user } = this.props;
     this.props.onOrgRemove(org.orgId);
     if (contextSrv.licensedAccessControlEnabled()) {
-      if (contextSrv.hasPermission(AccessControlAction.OrgUsersRemove)) {
+      if (
+        contextSrv.hasPermission(AccessControlAction.ActionUserRolesRemove) &&
+        contextSrv.hasPermission(AccessControlAction.ActionUserRolesAdd)
+      ) {
         user && (await updateUserRoles([], user.id, org.orgId));
       }
     }
@@ -333,7 +331,7 @@ export class AddToOrgModal extends PureComponent<AddToOrgModalProps, AddToOrgMod
     this.props.onOrgAdd(selectedOrg!.id, role);
     // add the stored userRoles also
     if (contextSrv.licensedAccessControlEnabled()) {
-      if (contextSrv.hasPermission(AccessControlAction.OrgUsersWrite)) {
+      if (contextSrv.hasPermission(AccessControlAction.ActionUserRolesAdd)) {
         if (this.state.pendingUserId) {
           await updateUserRoles(this.state.pendingRoles, this.state.pendingUserId!, this.state.pendingOrgId!);
           // clear pending state
@@ -391,7 +389,7 @@ export class AddToOrgModal extends PureComponent<AddToOrgModalProps, AddToOrgMod
               onBuiltinRoleChange={this.onOrgRoleChange}
               builtinRolesDisabled={false}
               roleOptions={roleOptions}
-              updateDisabled={true}
+              apply={true}
               onApplyRoles={this.onRoleUpdate}
               pendingRoles={this.state.pendingRoles}
             />

public/app/features/serviceaccounts/ServiceAccountCreatePage.tsx

@@ -128,14 +128,13 @@ export const ServiceAccountCreatePage = ({}: Props): JSX.Element => {
                   <Field label="Role">
                     {contextSrv.licensedAccessControlEnabled() ? (
                       <UserRolePicker
+                        apply
                         userId={serviceAccount.id || 0}
                         orgId={serviceAccount.orgId}
                         builtInRole={serviceAccount.role}
                         builtInRoles={builtinRoles}
                         onBuiltinRoleChange={onRoleChange}
-                        builtinRolesDisabled={false}
                         roleOptions={roleOptions}
-                        updateDisabled={true}
                         onApplyRoles={onPendingRolesUpdate}
                         pendingRoles={pendingRoles}
                       />

public/app/features/serviceaccounts/components/ServiceAccountRoleRow.tsx

@@ -23,7 +23,6 @@ export const ServiceAccountRoleRow = ({
 }: Props): JSX.Element => {
   const inputId = `${label}-input`;
   const canUpdateRole = contextSrv.hasPermissionInMetadata(AccessControlAction.ServiceAccountsWrite, serviceAccount);
-  const rolePickerDisabled = !canUpdateRole || serviceAccount.isDisabled;
 
   return (
     <tr>
@@ -39,7 +38,8 @@ export const ServiceAccountRoleRow = ({
             onBuiltinRoleChange={onRoleChange}
             roleOptions={roleOptions}
             builtInRoles={builtInRoles}
-            disabled={rolePickerDisabled}
+            builtinRolesDisabled={!canUpdateRole}
+            disabled={serviceAccount.isDisabled}
           />
         </td>
       ) : (
@@ -50,7 +50,7 @@ export const ServiceAccountRoleRow = ({
               inputId={inputId}
               aria-label="Role"
               value={serviceAccount.role}
-              disabled={rolePickerDisabled}
+              disabled={serviceAccount.isDisabled}
               onChange={onRoleChange}
             />
           </td>

public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx

@@ -40,7 +40,6 @@ const ServiceAccountListItem = memo(
     const displayRolePicker =
       contextSrv.hasPermission(AccessControlAction.ActionRolesList) &&
       contextSrv.hasPermission(AccessControlAction.ActionUserRolesList);
-    const enableRolePicker = contextSrv.hasPermission(AccessControlAction.OrgUsersWrite) && canUpdateRole;
 
     return (
       <tr key={serviceAccount.id} className={cx({ [styles.disabled]: serviceAccount.isDisabled })}>
@@ -83,7 +82,8 @@ const ServiceAccountListItem = memo(
                 onBuiltinRoleChange={(newRole) => onRoleChange(newRole, serviceAccount)}
                 roleOptions={roleOptions}
                 builtInRoles={builtInRoles}
-                disabled={!enableRolePicker || serviceAccount.isDisabled}
+                builtinRolesDisabled={!canUpdateRole}
+                disabled={serviceAccount.isDisabled}
               />
             )}
           </td>

public/app/features/teams/TeamList.tsx

@@ -73,13 +73,8 @@ export class TeamList extends PureComponent<Props, State> {
     const canDelete = contextSrv.hasAccessInMetadata(AccessControlAction.ActionTeamsDelete, team, isTeamAdmin);
     const canReadTeam = contextSrv.hasAccessInMetadata(AccessControlAction.ActionTeamsRead, team, isTeamAdmin);
     const canSeeTeamRoles = contextSrv.hasAccessInMetadata(AccessControlAction.ActionTeamsRolesList, team, false);
-    const canUpdateTeamRoles =
-      contextSrv.hasAccess(AccessControlAction.ActionTeamsRolesAdd, false) ||
-      contextSrv.hasAccess(AccessControlAction.ActionTeamsRolesRemove, false);
     const displayRolePicker =
-      contextSrv.licensedAccessControlEnabled() &&
-      contextSrv.hasPermission(AccessControlAction.ActionTeamsRolesList) &&
-      contextSrv.hasPermission(AccessControlAction.ActionRolesList);
+      contextSrv.licensedAccessControlEnabled() && contextSrv.hasPermission(AccessControlAction.ActionRolesList);
 
     return (
       <tr key={team.id}>
@@ -114,11 +109,7 @@ export class TeamList extends PureComponent<Props, State> {
           )}
         </td>
         {displayRolePicker && (
-          <td>
-            {canSeeTeamRoles && (
-              <TeamRolePicker teamId={team.id} roleOptions={this.state.roleOptions} disabled={!canUpdateTeamRoles} />
-            )}
-          </td>
+          <td>{canSeeTeamRoles && <TeamRolePicker teamId={team.id} roleOptions={this.state.roleOptions} />}</td>
         )}
         <td className="text-right">
           <DeleteButton

public/app/features/users/UsersTable.tsx

@@ -90,11 +90,13 @@ const UsersTable: FC<Props> = (props) => {
                     <UserRolePicker
                       userId={user.userId}
                       orgId={orgId}
-                      builtInRole={user.role}
-                      onBuiltinRoleChange={(newRole) => onRoleChange(newRole, user)}
                       roleOptions={roleOptions}
                       builtInRoles={builtinRoles}
-                      disabled={!contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersWrite, user)}
+                      builtInRole={user.role}
+                      onBuiltinRoleChange={(newRole) => onRoleChange(newRole, user)}
+                      builtinRolesDisabled={
+                        !contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersWrite, user)
+                      }
                     />
                   ) : (
                     <OrgRolePicker