Skip to content

Commit

Permalink
feat: custom root roles (#3975)
Browse files Browse the repository at this point in the history
## About the changes
Implements custom root roles, encompassing a lot of different areas of
the project, and slightly refactoring the current roles logic. It
includes quite a clean up.

This feature itself is behind a flag: `customRootRoles`

This feature covers root roles in:
 - Users;
 - Service Accounts;
 - Groups;

Apologies in advance. I may have gotten a bit carried away 🙈 

### Roles

We now have a new admin tab called "Roles" where we can see all root
roles and manage custom ones. We are not allowed to edit or remove
*predefined* roles.

![image](https://github.com/Unleash/unleash/assets/14320932/1ad8695c-8c3f-440d-ac32-39746720d588)
This meant slightly pushing away the existing roles to `project-roles`
instead. One idea we want to explore in the future is to unify both
types of roles in the UI instead of having 2 separate tabs. This
includes modernizing project roles to fit more into our current design
and decisions.

Hovering the permissions cell expands detailed information about the
role:

![image](https://github.com/Unleash/unleash/assets/14320932/81c4aae7-8b4d-4cb4-92d1-8f1bc3ef1f2a)

### Create and edit role

Here's how the role form looks like (create / edit):

![image](https://github.com/Unleash/unleash/assets/14320932/85baec29-bb10-48c5-a207-b3e9a8de838a)
Here I categorized permissions so it's easier to visualize and manage
from a UX perspective.

I'm using the same endpoint as before. I tried to unify the logic and
get rid of the `projectRole` specific hooks. What distinguishes custom
root roles from custom project roles is the extra `root-custom` type we
see on the payload. By default we assume `custom` (custom project role)
instead, which should help in terms of backwards compatibility.

### Delete role

When we delete a custom role we try to help the end user make an
informed decision by listing all the entities which currently use this
custom root role:

![image](https://github.com/Unleash/unleash/assets/14320932/352ed529-76be-47a8-88da-5e924fb191d4)
~~As mentioned in the screenshot, when deleting a custom role, we demote
all entities associated with it to the predefined `Viewer` role.~~
**EDIT**: Apparently we currently block this from the API
(access-service deleteRole) with a message:

![image](https://github.com/Unleash/unleash/assets/14320932/82a8e50f-8dc5-4c18-a2ba-54e2ae91b91c)
What should the correct behavior be?

### Role selector

I added a new easy-to-use role selector component that is present in:
 - Users 

![image](https://github.com/Unleash/unleash/assets/14320932/76953139-7fb6-437e-b3fa-ace1d9187674)
 - Service Accounts

![image](https://github.com/Unleash/unleash/assets/14320932/2b80bd55-9abb-4883-b715-15650ae752ea)
- Groups

![image](https://github.com/Unleash/unleash/assets/14320932/ab438f7c-2245-4779-b157-2da1689fe402)

### Role description

I also added a new role description component that you can see below the
dropdown in the selector component, but it's also used to better
describe each role in the respective tables:

![image](https://github.com/Unleash/unleash/assets/14320932/a3eecac1-2a34-4500-a68c-e3f62ebfa782)

I'm not listing all the permissions of predefined roles. Those simply
show the description in the tooltip:

![image](https://github.com/Unleash/unleash/assets/14320932/7e5b2948-45f0-4472-8311-bf533409ba6c)

### Role badge

Groups is a bit different, since it uses a list of cards, so I added yet
another component - Role badge:

![image](https://github.com/Unleash/unleash/assets/14320932/1d62c3db-072a-4c97-b86f-1d8ebdd3523e)

I'm using this same component on the profile tab:

![image](https://github.com/Unleash/unleash/assets/14320932/214272db-a828-444e-8846-4f39b9456bc6)

## Discussion points
- Are we being defensive enough with the use of the flag? Should we
cover more?
 - Are we breaking backwards compatibility in any way?
 - What should we do when removing a role? Block or demote?
- Maybe some existing permission-related issues will surface with this
change: Are we being specific enough with our permissions? A lot of
places are simply checking for `ADMIN`;
- We may want to get rid of the API roles coupling we have with the
users and SAs and instead use the new hooks (e.g. `useRoles`)
explicitly;
 - We should update the docs;
- Maybe we could allow the user to add a custom role directly from the
role selector component;

---------

Co-authored-by: Gastón Fournier <gaston@getunleash.io>
  • Loading branch information
nunogois and gastonfournier committed Jun 14, 2023
1 parent 1bd182d commit bb026c0
Show file tree
Hide file tree
Showing 70 changed files with 2,036 additions and 490 deletions.
11 changes: 8 additions & 3 deletions frontend/src/component/admin/Admin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import AdminMenu from './menu/AdminMenu';
import { Network } from './network/Network';
import CreateProjectRole from './projectRoles/CreateProjectRole/CreateProjectRole';
import EditProjectRole from './projectRoles/EditProjectRole/EditProjectRole';
import { Roles } from './roles/Roles';
import ProjectRoles from './projectRoles/ProjectRoles/ProjectRoles';
import { ServiceAccounts } from './serviceAccounts/ServiceAccounts';
import CreateUser from './users/CreateUser/CreateUser';
Expand All @@ -27,8 +28,11 @@ export const Admin = () => (
<AdminMenu />
<Routes>
<Route path="users" element={<UsersAdmin />} />
<Route path="create-project-role" element={<CreateProjectRole />} />
<Route path="roles/:id/edit" element={<EditProjectRole />} />
<Route path="project-roles/new" element={<CreateProjectRole />} />
<Route
path="project-roles/:id/edit"
element={<EditProjectRole />}
/>
<Route path="api" element={<ApiTokenPage />} />
<Route path="api/create-token" element={<CreateApiToken />} />
<Route path="users/:id/edit" element={<EditUser />} />
Expand All @@ -42,7 +46,8 @@ export const Admin = () => (
element={<EditGroupContainer />}
/>
<Route path="groups/:groupId" element={<Group />} />
<Route path="roles" element={<ProjectRoles />} />
<Route path="roles" element={<Roles />} />
<Route path="project-roles" element={<ProjectRoles />} />
<Route path="instance" element={<InstanceAdmin />} />
<Route path="network/*" element={<Network />} />
<Route path="maintenance" element={<MaintenanceAdmin />} />
Expand Down
49 changes: 8 additions & 41 deletions frontend/src/component/admin/groups/GroupForm/GroupForm.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { FC } from 'react';
import { Autocomplete, Box, Button, styled, TextField } from '@mui/material';
import { Box, Button, styled } from '@mui/material';
import { UG_DESC_ID, UG_NAME_ID } from 'utils/testIds';
import Input from 'component/common/Input/Input';
import { IGroupUser } from 'interfaces/group';
Expand All @@ -10,9 +10,10 @@ import { ItemList } from 'component/common/ItemList/ItemList';
import useAuthSettings from 'hooks/api/getters/useAuthSettings/useAuthSettings';
import { Link } from 'react-router-dom';
import { HelpIcon } from 'component/common/HelpIcon/HelpIcon';
import { IProjectRole } from 'interfaces/role';
import IRole from 'interfaces/role';
import { useUsers } from 'hooks/api/getters/useUsers/useUsers';
import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
import { RoleSelect } from 'component/common/RoleSelect/RoleSelect';

const StyledForm = styled('form')(() => ({
display: 'flex',
Expand Down Expand Up @@ -74,15 +75,6 @@ const StyledAutocompleteWrapper = styled('div')(({ theme }) => ({
},
}));

const StyledRoleOption = styled('div')(({ theme }) => ({
display: 'flex',
flexDirection: 'column',
'& > span:last-of-type': {
fontSize: theme.fontSizes.smallerBody,
color: theme.palette.text.secondary,
},
}));

interface IGroupForm {
name: string;
description: string;
Expand Down Expand Up @@ -128,24 +120,10 @@ export const GroupForm: FC<IGroupForm> = ({

const groupRootRolesEnabled = Boolean(uiConfig.flags.groupRootRoles);

const roleIdToRole = (rootRoleId: number | null): IProjectRole | null => {
return (
roles.find((role: IProjectRole) => role.id === rootRoleId) || null
);
const roleIdToRole = (rootRoleId: number | null): IRole | null => {
return roles.find((role: IRole) => role.id === rootRoleId) || null;
};

const renderRoleOption = (
props: React.HTMLAttributes<HTMLLIElement>,
option: IProjectRole
) => (
<li {...props}>
<StyledRoleOption>
<span>{option.name}</span>
<span>{option.description}</span>
</StyledRoleOption>
</li>
);

return (
<StyledForm onSubmit={handleSubmit}>
<div>
Expand Down Expand Up @@ -214,23 +192,12 @@ export const GroupForm: FC<IGroupForm> = ({
</Box>
</StyledInputDescription>
<StyledAutocompleteWrapper>
<Autocomplete
<RoleSelect
data-testid="GROUP_ROOT_ROLE"
size="small"
openOnFocus
value={roleIdToRole(rootRole)}
onChange={(_, newValue) =>
setRootRole(newValue?.id || null)
setValue={role =>
setRootRole(role?.id || null)
}
options={roles.filter(
(role: IProjectRole) =>
role.name !== 'Viewer'
)}
renderOption={renderRoleOption}
getOptionLabel={option => option.name}
renderInput={params => (
<TextField {...params} label="Role" />
)}
/>
</StyledAutocompleteWrapper>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { GroupCardAvatars } from './GroupCardAvatars/GroupCardAvatars';
import { Badge } from 'component/common/Badge/Badge';
import { GroupCardActions } from './GroupCardActions/GroupCardActions';
import TopicOutlinedIcon from '@mui/icons-material/TopicOutlined';
import { IProjectRole } from 'interfaces/role';
import { RoleBadge } from 'component/common/RoleBadge/RoleBadge';

const StyledLink = styled(Link)(({ theme }) => ({
textDecoration: 'none',
Expand Down Expand Up @@ -86,14 +86,12 @@ const InfoBadgeDescription = styled('span')(({ theme }) => ({

interface IGroupCardProps {
group: IGroup;
rootRoles: IProjectRole[];
onEditUsers: (group: IGroup) => void;
onRemoveGroup: (group: IGroup) => void;
}

export const GroupCard = ({
group,
rootRoles,
onEditUsers,
onRemoveGroup,
}: IGroupCardProps) => {
Expand All @@ -117,17 +115,7 @@ export const GroupCard = ({
show={
<InfoBadgeDescription>
<p>Root role:</p>
<Badge
color="success"
icon={<TopicOutlinedIcon />}
>
{
rootRoles.find(
(role: IProjectRole) =>
role.id === group.rootRole
)?.name
}
</Badge>
<RoleBadge roleId={group.rootRole!} />
</InfoBadgeDescription>
}
/>
Expand Down
8 changes: 0 additions & 8 deletions frontend/src/component/admin/groups/GroupsList/GroupsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import { Add } from '@mui/icons-material';
import { NAVIGATE_TO_CREATE_GROUP } from 'utils/testIds';
import { EditGroupUsers } from '../Group/EditGroupUsers/EditGroupUsers';
import { RemoveGroup } from '../RemoveGroup/RemoveGroup';
import { useUsers } from 'hooks/api/getters/useUsers/useUsers';
import { IProjectRole } from 'interfaces/role';

type PageQueryType = Partial<Record<'search', string>>;

Expand Down Expand Up @@ -51,7 +49,6 @@ export const GroupsList: VFC = () => {
const [searchValue, setSearchValue] = useState(
searchParams.get('search') || ''
);
const { roles } = useUsers();

const isSmallScreen = useMediaQuery(theme.breakpoints.down('md'));

Expand Down Expand Up @@ -85,10 +82,6 @@ export const GroupsList: VFC = () => {
setRemoveOpen(true);
};

const getBindableRootRoles = () => {
return roles.filter((role: IProjectRole) => role.type === 'root');
};

return (
<PageContent
isLoading={loading}
Expand Down Expand Up @@ -141,7 +134,6 @@ export const GroupsList: VFC = () => {
<Grid key={group.id} item xs={12} md={6}>
<GroupCard
group={group}
rootRoles={getBindableRootRoles()}
onEditUsers={onEditUsers}
onRemoveGroup={onRemoveGroup}
/>
Expand Down
12 changes: 11 additions & 1 deletion frontend/src/component/admin/menu/AdminMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,21 @@ function AdminMenu() {
}
/>
)}
{flags.RE && (
{flags.customRootRoles && (
<Tab
value="roles"
label={
<CenteredNavLink to="/admin/roles">
<span>Roles</span>
</CenteredNavLink>
}
/>
)}
{flags.RE && (
<Tab
value="project-roles"
label={
<CenteredNavLink to="/admin/project-roles">
<span>Project roles</span>
</CenteredNavLink>
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import FormTemplate from 'component/common/FormTemplate/FormTemplate';
import useProjectRolesApi from 'hooks/api/actions/useProjectRolesApi/useProjectRolesApi';
import { useRolesApi } from 'hooks/api/actions/useRolesApi/useRolesApi';
import { useNavigate } from 'react-router-dom';
import ProjectRoleForm from '../ProjectRoleForm/ProjectRoleForm';
import useProjectRoleForm from '../hooks/useProjectRoleForm';
Expand Down Expand Up @@ -33,7 +33,7 @@ const CreateProjectRole = () => {
getRoleKey,
} = useProjectRoleForm();

const { createRole, loading } = useProjectRolesApi();
const { addRole, loading } = useRolesApi();

const onSubmit = async (e: Event) => {
e.preventDefault();
Expand All @@ -44,8 +44,8 @@ const CreateProjectRole = () => {
if (validName && validPermissions) {
const payload = getProjectRolePayload();
try {
await createRole(payload);
navigate('/admin/roles');
await addRole(payload);
navigate('/admin/project-roles');
setToastData({
title: 'Project role created',
text: 'Now you can start assigning your project roles to project members.',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import FormTemplate from 'component/common/FormTemplate/FormTemplate';
import { UpdateButton } from 'component/common/UpdateButton/UpdateButton';
import { ADMIN } from 'component/providers/AccessProvider/permissions';
import useProjectRolesApi from 'hooks/api/actions/useProjectRolesApi/useProjectRolesApi';
import useProjectRole from 'hooks/api/getters/useProjectRole/useProjectRole';
import { useRolesApi } from 'hooks/api/actions/useRolesApi/useRolesApi';
import { useRole } from 'hooks/api/getters/useRole/useRole';
import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
import useToast from 'hooks/useToast';
import { useNavigate } from 'react-router-dom';
Expand All @@ -15,8 +15,8 @@ import { GO_BACK } from 'constants/navigate';
const EditProjectRole = () => {
const { uiConfig } = useUiConfig();
const { setToastData, setToastApiError } = useToast();
const projectId = useRequiredPathParam('id');
const { role } = useProjectRole(projectId);
const roleId = useRequiredPathParam('id');
const { role, refetch } = useRole(roleId);

const navigate = useNavigate();
const {
Expand All @@ -35,19 +35,18 @@ const EditProjectRole = () => {
validateName,
clearErrors,
getRoleKey,
} = useProjectRoleForm(role.name, role.description, role?.permissions);
} = useProjectRoleForm(role?.name, role?.description, role?.permissions);

const formatApiCode = () => {
return `curl --location --request PUT '${
uiConfig.unleashUrl
}/api/admin/roles/${role.id}' \\
}/api/admin/roles/${role?.id}' \\
--header 'Authorization: INSERT_API_KEY' \\
--header 'Content-Type: application/json' \\
--data-raw '${JSON.stringify(getProjectRolePayload(), undefined, 2)}'`;
};

const { refetch } = useProjectRole(projectId);
const { editRole, loading } = useProjectRolesApi();
const { updateRole, loading } = useRolesApi();

const onSubmit = async (e: Event) => {
e.preventDefault();
Expand All @@ -58,9 +57,9 @@ const EditProjectRole = () => {

if (validName && validPermissions) {
try {
await editRole(projectId, payload);
await updateRole(+roleId, payload);
refetch();
navigate('/admin/roles');
navigate('/admin/project-roles');
setToastData({
type: 'success',
title: 'Project role updated',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
Typography,
} from '@mui/material';
import { ExpandMore } from '@mui/icons-material';
import { IPermission } from 'interfaces/project';
import { IPermission } from 'interfaces/permissions';
import StringTruncator from 'component/common/StringTruncator/StringTruncator';
import { ICheckedPermission } from 'component/admin/projectRoles/hooks/useProjectRoleForm';

Expand All @@ -23,10 +23,10 @@ interface IEnvironmentPermissionAccordionProps {
title: string;
Icon: ReactNode;
isInitiallyExpanded?: boolean;
context: 'project' | 'environment';
context: string;
onPermissionChange: (permission: IPermission) => void;
onCheckAll: () => void;
getRoleKey: (permission: { id: number; environment?: string }) => string;
getRoleKey?: (permission: { id: number; environment?: string }) => string;
}

const AccordionHeader = styled(Box)(({ theme }) => ({
Expand All @@ -52,7 +52,7 @@ export const PermissionAccordion: VFC<IEnvironmentPermissionAccordionProps> = ({
context,
onPermissionChange,
onCheckAll,
getRoleKey,
getRoleKey = permission => permission.id.toString(),
}) => {
const [expanded, setExpanded] = useState(isInitiallyExpanded);
const permissionMap = useMemo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { ConditionallyRender } from 'component/common/ConditionallyRender/Condit
import {
IPermission,
IProjectEnvironmentPermissions,
IProjectRolePermissions,
} from 'interfaces/project';
IPermissions,
} from 'interfaces/permissions';
import { ICheckedPermission } from '../hooks/useProjectRoleForm';

interface IProjectRoleForm {
Expand All @@ -21,7 +21,7 @@ interface IProjectRoleForm {
errors: { [key: string]: string };
children: ReactNode;
permissions:
| IProjectRolePermissions
| IPermissions
| {
project: IPermission[];
environments: IProjectEnvironmentPermissions[];
Expand Down
Loading

0 comments on commit bb026c0

Please sign in to comment.