fix: check for permission in group access assignment (#7408)

Fix project role assignment for users with `ADMIN` permission, even if
they don't have the Admin root role. This happens when e.g. users
inherit the `ADMIN` permission from a group root role, but are not
Admins themselves.

---------

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

frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx

@@ -38,6 +38,8 @@ import { caseInsensitiveSearch } from 'utils/search';
 import type { IServiceAccount } from 'interfaces/service-account';
 import { MultipleRoleSelect } from 'component/common/MultipleRoleSelect/MultipleRoleSelect';
 import type { IUserProjectRole } from '../../../../interfaces/userProjectRoles';
+import { useCheckProjectPermissions } from 'hooks/useHasAccess';
+import { ADMIN } from 'component/providers/AccessProvider/permissions';
 
 const StyledForm = styled('form')(() => ({
     display: 'flex',
@@ -119,6 +121,8 @@ export const ProjectAccessAssign = ({
         useProjectApi();
     const edit = Boolean(selected);
 
+    const checkPermissions = useCheckProjectPermissions(projectId);
+
     const { setToastData, setToastApiError } = useToast();
     const navigate = useNavigate();
 
@@ -323,11 +327,10 @@ export const ProjectAccessAssign = ({
 
     const isValid = selectedOptions.length > 0 && selectedRoles.length > 0;
     const displayAllRoles =
+        checkPermissions(ADMIN) ||
         userRoles.length === 0 ||
-        userRoles.some(
+        userRoles.some((userRole) => userRole.name === 'Owner');
-            (userRole) =>
+
-                userRole.name === 'Admin' || userRole.name === 'Owner',
-        );
     let filteredRoles: IRole[];
     if (displayAllRoles) {
         filteredRoles = roles;

src/lib/features/project/project-service.e2e.test.ts

@@ -440,6 +440,51 @@ describe('Managing Project access', () => {
             ),
         ).resolves.not.toThrow();
     });
+
+    test('Admin group members should be allowed to add any project role', async () => {
+        const viewerUser = await stores.userStore.insert({
+            name: 'Some project admin',
+            email: 'some_project_admin@example.com',
+        });
+        await accessService.setUserRootRole(viewerUser.id, RoleName.VIEWER);
+
+        const adminRole = await stores.roleStore.getRoleByName(RoleName.ADMIN);
+        const adminGroup = await stores.groupStore.create({
+            name: 'admin_group',
+            rootRole: adminRole.id,
+        });
+        await stores.groupStore.addUsersToGroup(
+            adminGroup.id,
+            [{ user: { id: viewerUser.id } }],
+            opsUser.username!,
+        );
+
+        const project = {
+            id: 'some-project',
+            name: 'sp',
+            description: '',
+            mode: 'open' as const,
+            defaultStickiness: 'clientId',
+        };
+        await projectService.createProject(project, user, auditUser);
+        const customRole = await stores.roleStore.create({
+            name: 'my_custom_project_role_admin_user',
+            roleType: 'custom',
+            description:
+                'Used to prove that you can assign a role when you are admin',
+        });
+
+        await expect(
+            projectService.addAccess(
+                project.id,
+                [customRole.id], // roles
+                [], // groups
+                [opsUser.id], // users
+                extractAuditInfoFromUser(viewerUser),
+            ),
+        ).resolves.not.toThrow();
+    });
+
     test('Users with project owner should be allowed to add any project role', async () => {
         const project = {
             id: 'project-owner',
@@ -451,11 +496,11 @@ describe('Managing Project access', () => {
         await projectService.createProject(project, user, auditUser);
         const projectAdmin = await stores.userStore.insert({
             name: 'Some project admin',
-            email: 'admin@example.com',
+            email: 'some_other_project_admin@example.com',
         });
         const projectCustomer = await stores.userStore.insert({
             name: 'Some project customer',
-            email: 'customer@example.com',
+            email: 'some_project_customer@example.com',
         });
         const ownerRole = await stores.roleStore.getRoleByName(RoleName.OWNER);
         await accessService.addUserToRole(
@@ -464,7 +509,7 @@ describe('Managing Project access', () => {
             project.id,
         );
         const customRole = await stores.roleStore.create({
-            name: 'my_custom_role',
+            name: 'my_custom_project_role',
             roleType: 'custom',
             description:
                 'Used to prove that you can assign a role the project owner does not have',
@@ -477,7 +522,7 @@ describe('Managing Project access', () => {
                 [projectCustomer.id],
                 auditUser,
             ),
-        ).resolves;
+        ).resolves.not.toThrow();
     });
     test('Users with project role should only be allowed to grant same role to others', async () => {
         const project = {

src/lib/features/project/project-service.ts

@@ -52,6 +52,7 @@ import {
     SYSTEM_USER_ID,
     type ProjectCreated,
     type IProjectOwnersReadModel,
+    ADMIN,
 } from '../../types';
 import type {
     IProjectAccessModel,
@@ -838,16 +839,21 @@ export default class ProjectService {
     }
 
     private async isAllowedToAddAccess(
-        userAddingAccess: number,
+        userAddingAccess: IAuditUser,
         projectId: string,
         rolesBeingAdded: number[],
     ): Promise<boolean> {
+        const userPermissions =
+            await this.accessService.getPermissionsForUser(userAddingAccess);
+        if (userPermissions.some(({ permission }) => permission === ADMIN)) {
+            return true;
+        }
         const userRoles = await this.accessService.getAllProjectRolesForUser(
-            userAddingAccess,
+            userAddingAccess.id,
             projectId,
         );
         if (
-            this.isAdmin(userAddingAccess, userRoles) ||
+            this.isAdmin(userAddingAccess.id, userRoles) ||
             this.isProjectOwner(userRoles, projectId)
         ) {
             return true;
@@ -864,7 +870,7 @@ export default class ProjectService {
         users: number[],
         auditUser: IAuditUser,
     ): Promise<void> {
-        if (await this.isAllowedToAddAccess(auditUser.id, projectId, roles)) {
+        if (await this.isAllowedToAddAccess(auditUser, projectId, roles)) {
             await this.accessService.addAccessToProject(
                 roles,
                 groups,
@@ -915,7 +921,7 @@ export default class ProjectService {
             await this.validateAtLeastOneOwner(projectId, ownerRole);
         }
         const isAllowedToAssignRoles = await this.isAllowedToAddAccess(
-            auditUser.id,
+            auditUser,
             projectId,
             newRoles,
         );
@@ -966,7 +972,7 @@ export default class ProjectService {
             await this.validateAtLeastOneOwner(projectId, ownerRole);
         }
         const isAllowedToAssignRoles = await this.isAllowedToAddAccess(
-            auditUser.id,
+            auditUser,
             projectId,
             newRoles,
         );