From f791b866dd66a8e9d5865a0d20bfd168b025b5aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Tue, 18 Jun 2024 08:57:00 +0100 Subject: [PATCH 1/3] fix: check for permission in group access assignment --- .../ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx index ce7f408d39..bcd0c57ab1 100644 --- a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx @@ -38,6 +38,7 @@ 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'; const StyledForm = styled('form')(() => ({ display: 'flex', @@ -119,6 +120,8 @@ export const ProjectAccessAssign = ({ useProjectApi(); const edit = Boolean(selected); + const checkPermissions = useCheckProjectPermissions(projectId); + const { setToastData, setToastApiError } = useToast(); const navigate = useNavigate(); @@ -323,6 +326,7 @@ export const ProjectAccessAssign = ({ const isValid = selectedOptions.length > 0 && selectedRoles.length > 0; const displayAllRoles = + checkPermissions('ADMIN') || userRoles.length === 0 || userRoles.some( (userRole) => From afbb1b6192e70a09a7dbc149a008d1d3a149572f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Tue, 18 Jun 2024 09:03:20 +0100 Subject: [PATCH 2/3] refactor: remove redundant ADMIN role check --- .../ProjectAccessAssign/ProjectAccessAssign.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx index bcd0c57ab1..2fb5d03b14 100644 --- a/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessAssign/ProjectAccessAssign.tsx @@ -39,6 +39,7 @@ 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', @@ -326,12 +327,10 @@ export const ProjectAccessAssign = ({ const isValid = selectedOptions.length > 0 && selectedRoles.length > 0; const displayAllRoles = - checkPermissions('ADMIN') || + 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; From dc33f3a7fcda54489386e75bf80e6107f07764af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Tue, 18 Jun 2024 10:16:26 +0100 Subject: [PATCH 3/3] fix: backend check on the service layer --- src/lib/features/project/project-service.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index 0a9ce5721b..cca524d571 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, @@ -838,16 +839,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; @@ -864,7 +870,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, @@ -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, );