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

Cherry pick fix --------- Co-authored-by: Nuno Góis <[email protected]>
Unleash · Jun 19, 2024 · 28e3747 · 28e3747
1 parent 3e3235e
commit 28e3747
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 33 deletions.
diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx b/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(
-            (userRole) =>
-                userRole.name === 'Admin' || userRole.name === 'Owner',
-        );
+        userRoles.some((userRole) => userRole.name === 'Owner');
+
     let filteredRoles: IRole[];
     if (displayAllRoles) {
         filteredRoles = roles;

diff --git a/package.json b/package.json
@@ -82,9 +82,7 @@
     "testTimeout": 10000,
     "globalSetup": "./scripts/jest-setup.js",
     "transform": {
-      "^.+\\.tsx?$": [
-        "@swc/jest"
-      ]
+      "^.+\\.tsx?$": ["@swc/jest"]
     },
     "testRegex": "(/__tests__/.*|(\\.|/)(test|spec))\\.(jsx?|tsx?)$",
     "testPathIgnorePatterns": [
@@ -93,13 +91,7 @@
       "/frontend/",
       "/website/"
     ],
-    "moduleFileExtensions": [
-      "ts",
-      "tsx",
-      "js",
-      "jsx",
-      "json"
-    ],
+    "moduleFileExtensions": ["ts", "tsx", "js", "jsx", "json"],
     "coveragePathIgnorePatterns": [
       "/node_modules/",
       "/dist/",
@@ -240,14 +232,8 @@
     "tough-cookie": "4.1.4"
   },
   "lint-staged": {
-    "*.{js,ts}": [
-      "biome check --write --no-errors-on-unmatched"
-    ],
-    "*.{jsx,tsx}": [
-      "biome check --write --no-errors-on-unmatched"
-    ],
-    "*.json": [
-      "biome format --write --no-errors-on-unmatched"
-    ]
+    "*.{js,ts}": ["biome check --write --no-errors-on-unmatched"],
+    "*.{jsx,tsx}": ["biome check --write --no-errors-on-unmatched"],
+    "*.json": ["biome format --write --no-errors-on-unmatched"]
   }
 }
diff --git a/src/lib/features/project/project-service.e2e.test.ts b/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: '[email protected]',
+        });
+        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 = {

diff --git a/src/lib/features/project/project-service.ts b/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,
@@ -832,16 +833,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;
@@ -858,7 +864,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,
@@ -909,7 +915,7 @@ export default class ProjectService {
             await this.validateAtLeastOneOwner(projectId, ownerRole);
         }
         const isAllowedToAssignRoles = await this.isAllowedToAddAccess(
-            auditUser.id,
+            auditUser,
             projectId,
             newRoles,
         );
@@ -960,7 +966,7 @@ export default class ProjectService {
             await this.validateAtLeastOneOwner(projectId, ownerRole);
         }
         const isAllowedToAssignRoles = await this.isAllowedToAddAccess(
-            auditUser.id,
+            auditUser,
             projectId,
             newRoles,
         );