feat: custom root roles #3975

nunogois · 2023-06-13T18:22:34Z

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.

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:

Create and edit role

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

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:

~~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:

What should the correct behavior be?

Role selector

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

Users
Service Accounts
Groups

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:

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

Role badge

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

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

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;

vercel · 2023-06-13T18:22:40Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
unleash-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 14, 2023 1:31pm
unleash-monorepo-frontend	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 14, 2023 1:31pm

gastonfournier

WIP some comments

src/lib/services/access-service.ts

gastonfournier · 2023-06-14T10:24:16Z

frontend/src/interfaces/role.ts

 interface IRole {
    id: number;
    name: string;
    project: string | null;
    description: string;
    type: string;
+    permissions?: IPermission[];


Here I'd add a new interface extending IRole, something like:

interface IRoleWithPermissions extends IRole { permissions: IPermission[]; }

gastonfournier

I think this is good enough to merge and test. There's a couple of minor things but I don't think they're blockers

gastonfournier · 2023-06-14T12:32:15Z

frontend/src/component/admin/Admin.tsx

+            <Route path="project-roles/new" element={<CreateProjectRole />} />
+            <Route
+                path="project-roles/:id/edit"
+                element={<EditProjectRole />}
+            />


I wonder if we should keep the same URL as before for project roles or just add a new one for root-roles? I see pros and cons with both approaches, the main concern is that we are changing the meaning of the existing resource (but this is not on the API level, so this should be fine)

Since the goal is to eventually merge both root roles and project roles on the same tab, I don't think we should prioritize this for now.

gastonfournier · 2023-06-14T12:43:25Z

frontend/src/component/admin/roles/RoleForm/useRoleForm.ts

+        const allChecked = rootPermissions.every(
+            (permission: IPermission) => checkedPermissionsCopy[permission.id]
+        );
+
+        if (allChecked) {
+            rootPermissions.forEach((permission: IPermission) => {
+                delete checkedPermissionsCopy[permission.id];
+            });
+        } else {
+            rootPermissions.forEach((permission: IPermission) => {
+                checkedPermissionsCopy[permission.id] = {
+                    ...permission,
+                };
+            });
+        }


This looks like a refactoring opportunity as this code is repeated in a couple of places

src/lib/services/access-service.ts

gastonfournier · 2023-06-14T13:13:09Z

src/lib/services/access-service.ts

        const baseRole = {
            ...(await this.validateRole(role)),
-            roleType: CUSTOM_ROLE_TYPE,
+            roleType,


Note for the reviewers CUSTOM_PROJECT_ROLE_TYPE has currently the same value as CUSTOM_ROLE_TYPE had

Co-authored-by: Gastón Fournier <gaston@getunleash.io>

https://linear.app/unleash/issue/2-1135/address-3975-pr-comments-by-refactoring-some-of-the-new-custom-root This pull request addresses the majority of the comments raised in issue #3975 and lays the groundwork for unifying roles. The idea is for project roles to also be managed in the "Roles" tab, and several components, such as `RoleForm` and the `useRoleForm` can potentially be reused. I'll leave the further investigation and implementation of unifying roles to be addressed in a separate task. As a mostly unrelated UI fix, this also adds an arrow to the tooltip in the `RoleBadge` component.

github-actions bot assigned nunogois Jun 13, 2023

nunogois added 6 commits June 13, 2023 19:42

feat: custom root roles

e15c438

feat: basic roles skeleton

f5f13d4

wip: custom root roles

d18628b

refactor to useRoles hook, add rolemodal and roleform

ff19f62

role select, categorize permissions, ...

2896503

adapt new role logic in all the places

f3e6c18

nunogois force-pushed the feat-custom-root-roles branch from 83d59c3 to f3e6c18 Compare June 13, 2023 18:42

vercel bot deployed to Preview – unleash-monorepo-frontend June 13, 2023 18:44 View deployment

test: update snap, fix test

bcdacb8

vercel bot deployed to Preview – unleash-monorepo-frontend June 13, 2023 19:25 View deployment

fix: graceful fallback to OSS, clean up

25fb07a

vercel bot deployed to Preview – unleash-monorepo-frontend June 14, 2023 09:22 View deployment

vercel bot deployed to Preview – unleash-docs June 14, 2023 09:27 View deployment

fix: default value on hook

5e7ff9e

vercel bot deployed to Preview – unleash-monorepo-frontend June 14, 2023 09:28 View deployment

vercel bot deployed to Preview – unleash-docs June 14, 2023 09:30 View deployment

gastonfournier reviewed Jun 14, 2023

View reviewed changes

gastonfournier approved these changes Jun 14, 2023

View reviewed changes

fix: address some PR comments

4fb9eb6

vercel bot deployed to Preview – unleash-monorepo-frontend June 14, 2023 13:24 View deployment

vercel bot deployed to Preview – unleash-docs June 14, 2023 13:26 View deployment

Update src/lib/services/access-service.ts

756681a

Co-authored-by: Gastón Fournier <gaston@getunleash.io>

vercel bot deployed to Preview – unleash-monorepo-frontend June 14, 2023 13:29 View deployment

vercel bot deployed to Preview – unleash-docs June 14, 2023 13:31 View deployment

nunogois merged commit bb026c0 into main Jun 14, 2023
17 checks passed

nunogois deleted the feat-custom-root-roles branch June 14, 2023 13:40

nunogois mentioned this pull request Jun 15, 2023

refactor: address custom root roles PR comments #3994

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: custom root roles #3975

feat: custom root roles #3975

nunogois commented Jun 13, 2023 •

edited

vercel bot commented Jun 13, 2023 •

edited

gastonfournier left a comment

gastonfournier Jun 14, 2023 •

edited

gastonfournier left a comment

gastonfournier Jun 14, 2023

nunogois Jun 14, 2023

gastonfournier Jun 14, 2023

gastonfournier Jun 14, 2023

feat: custom root roles #3975

feat: custom root roles #3975

Conversation

nunogois commented Jun 13, 2023 • edited

About the changes

Roles

Create and edit role

Delete role

Role selector

Role description

Role badge

Discussion points

vercel bot commented Jun 13, 2023 • edited

gastonfournier left a comment

Choose a reason for hiding this comment

gastonfournier Jun 14, 2023 • edited

Choose a reason for hiding this comment

gastonfournier left a comment

Choose a reason for hiding this comment

gastonfournier Jun 14, 2023

Choose a reason for hiding this comment

nunogois Jun 14, 2023

Choose a reason for hiding this comment

gastonfournier Jun 14, 2023

Choose a reason for hiding this comment

gastonfournier Jun 14, 2023

Choose a reason for hiding this comment

nunogois commented Jun 13, 2023 •

edited

vercel bot commented Jun 13, 2023 •

edited

gastonfournier Jun 14, 2023 •

edited