From 28e37472fa4c4554642b0f2c0681c6a3958e707c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Wed, 19 Jun 2024 12:15:05 +0200 Subject: [PATCH] fix: check for permission in group access assignment (#7408) (#7413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry pick fix --------- Co-authored-by: Nuno Góis --- .../ProjectAccessAssign.tsx | 11 ++-- package.json | 24 ++------- .../project/project-service.e2e.test.ts | 53 +++++++++++++++++-- src/lib/features/project/project-service.ts | 18 ++++--- 4 files changed, 73 insertions(+), 33 deletions(-) diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx index ce7f408d3956..2fb5d03b14bb 100644 --- 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 index fa53990c33dd..7bae24a8dacc 100644 --- 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 index 88b4ed461b0c..b9e6c704ff35 100644 --- 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: '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 = { diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index efb06a0e11c2..7b6333f0cfa6 100644 --- 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 { + 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 { - 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, );