diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index 812f01fb50..31db616405 100644 --- a/src/lib/features/project/project-service.ts +++ b/src/lib/features/project/project-service.ts @@ -728,8 +728,8 @@ export default class ProjectService { ) { return true; } - return rolesBeingAdded.every((role) => - userRoles.some((userRole) => userRole.id === role), + return rolesBeingAdded.every((roleId) => + userRoles.some((userRole) => userRole.id === roleId), ); } async addAccess( @@ -785,7 +785,6 @@ export default class ProjectService { projectId, userId, ); - const ownerRole = await this.accessService.getRoleByName( RoleName.OWNER, ); @@ -795,27 +794,37 @@ export default class ProjectService { if (hasOwnerRole && isRemovingOwnerRole) { await this.validateAtLeastOneOwner(projectId, ownerRole); } - - await this.accessService.setProjectRolesForUser( + const isAllowedToAssignRoles = await this.isAllowedToAddAccess( + createdByUserId, projectId, - userId, newRoles, ); - await this.eventService.storeEvent( - new ProjectAccessUserRolesUpdated({ - project: projectId, - createdBy: createdByUserName, - createdByUserId, - data: { - roles: newRoles, - userId, - }, - preData: { - roles: currentRoles, - userId, - }, - }), - ); + if (isAllowedToAssignRoles) { + await this.accessService.setProjectRolesForUser( + projectId, + userId, + newRoles, + ); + await this.eventService.storeEvent( + new ProjectAccessUserRolesUpdated({ + project: projectId, + createdBy: createdByUserName, + createdByUserId, + data: { + roles: newRoles, + userId, + }, + preData: { + roles: currentRoles, + userId, + }, + }), + ); + } else { + throw new InvalidOperationError( + 'User tried to assign a role they did not have access to', + ); + } } async setRolesForGroup( @@ -838,28 +847,38 @@ export default class ProjectService { if (hasOwnerRole && isRemovingOwnerRole) { await this.validateAtLeastOneOwner(projectId, ownerRole); } - - await this.accessService.setProjectRolesForGroup( + const isAllowedToAssignRoles = await this.isAllowedToAddAccess( + createdByUserId, projectId, - groupId, newRoles, - createdBy, ); - await this.eventService.storeEvent( - new ProjectAccessGroupRolesUpdated({ - project: projectId, + if (isAllowedToAssignRoles) { + await this.accessService.setProjectRolesForGroup( + projectId, + groupId, + newRoles, createdBy, - createdByUserId, - data: { - roles: newRoles, - groupId, - }, - preData: { - roles: currentRoles, - groupId, - }, - }), - ); + ); + await this.eventService.storeEvent( + new ProjectAccessGroupRolesUpdated({ + project: projectId, + createdBy, + createdByUserId, + data: { + roles: newRoles, + groupId, + }, + preData: { + roles: currentRoles, + groupId, + }, + }), + ); + } else { + throw new InvalidOperationError( + 'User tried to assign a role they did not have access to', + ); + } } async findProjectGroupRole( diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index c42b4ae4d9..cb58a6ea2a 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -25,6 +25,7 @@ import { SYSTEM_USER_ID, } from '../../../lib/types'; import { User } from '../../../lib/server-impl'; +import { InvalidOperationError } from '../../../lib/error'; let stores: IUnleashStores; let db: ITestDb; @@ -619,7 +620,80 @@ describe('Managing Project access', () => { projectUser.username, projectUser.id, ), - ).resolves.not.toThrow(); + ).resolves.not.toThrow( + new InvalidOperationError( + 'User tried to assign a role they did not have access to', + ), + ); + }); + test('Users can not assign roles they do not have to a user through explicit roles endpoint', async () => { + const project = { + id: 'user_fail_assign_to_user', + name: 'user_fail_assign_to_user', + description: '', + mode: 'open' as const, + defaultStickiness: 'clientId', + }; + await projectService.createProject(project, user); + const projectUser = await stores.userStore.insert({ + name: 'Some project user', + email: 'fail_assign_role_to_user@example.com', + }); + const secondUser = await stores.userStore.insert({ + name: 'Some other user', + email: 'otheruser_no_roles@example.com', + }); + const customRole = await stores.roleStore.create({ + name: 'role_that_noone_has', + roleType: 'custom', + description: + 'Used to prove that you can not assign a role you do not have via setRolesForUser', + }); + expect( + projectService.setRolesForUser( + project.id, + secondUser.id, + [customRole.id], + projectUser.username, + projectUser.id, + ), + ).rejects.toThrow(); + }); + test('Users can not assign roles they do not have to a group through explicit roles endpoint', async () => { + const project = { + id: 'user_fail_assign_to_group', + name: 'user_fail_assign_to_group', + description: '', + mode: 'open' as const, + defaultStickiness: 'clientId', + }; + await projectService.createProject(project, user); + const projectUser = await stores.userStore.insert({ + name: 'Some project user', + email: 'fail_assign_role_to_group@example.com', + }); + const group = await stores.groupStore.create({ + name: 'Some group_awaiting_role', + }); + const customRole = await stores.roleStore.create({ + name: 'role_that_noone_has_fail_assign_group', + roleType: 'custom', + description: + 'Used to prove that you can not assign a role you do not have via setRolesForGroup', + }); + expect( + projectService.setRolesForGroup( + project.id, + group.id, + [customRole.id], + projectUser.username, + projectUser.id, + ), + ).rejects.toThrow( + new InvalidOperationError( + 'User tried to assign a role they did not have access to', + ), + ); }); });