From 8364c3b396bf01b15b606484e268f446dfbf86f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Mon, 21 Feb 2022 14:39:59 +0100 Subject: [PATCH] fix: add method to change role for project memeber --- src/lib/db/access-store.ts | 13 +++ src/lib/services/access-service.ts | 8 ++ src/lib/services/project-service.ts | 91 +++++++++++++++---- src/lib/types/events.ts | 22 +++++ src/lib/types/stores/access-store.ts | 6 ++ .../e2e/services/project-service.e2e.test.ts | 61 +++++++++++++ src/test/fixtures/fake-access-store.ts | 8 ++ 7 files changed, 191 insertions(+), 18 deletions(-) diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts index b60ed3982f..1649c82d2c 100644 --- a/src/lib/db/access-store.ts +++ b/src/lib/db/access-store.ts @@ -247,6 +247,19 @@ export class AccessStore implements IAccessStore { .delete(); } + async updateUserProjectRole( + userId: number, + roleId: number, + projectId: string, + ): Promise { + return this.db(T.ROLE_USER) + .where({ + user_id: userId, + project: projectId, + }) + .update('role_id', roleId); + } + async removeRolesOfTypeForUser( userId: number, roleType: string, diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts index 7be15b9677..065c2b34cc 100644 --- a/src/lib/services/access-service.ts +++ b/src/lib/services/access-service.ts @@ -220,6 +220,14 @@ export class AccessService { return this.store.removeUserFromRole(userId, roleId, projectId); } + async updateUserProjectRole( + userId: number, + roleId: number, + projectId: string, + ): Promise { + return this.store.updateUserProjectRole(userId, roleId, projectId); + } + //This actually only exists for testing purposes async addPermissionToRole( roleId: number, diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 01605f1e61..8281a0fb2e 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -8,6 +8,7 @@ import NotFoundError from '../error/notfound-error'; import { ProjectUserAddedEvent, ProjectUserRemovedEvent, + ProjectUserUpdateRoleEvent, PROJECT_CREATED, PROJECT_DELETED, PROJECT_UPDATED, @@ -309,23 +310,9 @@ export default class ProjectService { userId: number, createdBy?: string, ): Promise { - const roles = await this.accessService.getRolesForProject(projectId); - const role = roles.find((r) => r.id === roleId); - if (!role) { - throw new NotFoundError( - `Couldn't find roleId=${roleId} on project=${projectId}`, - ); - } + const role = await this.findProjectRole(projectId, roleId); - if (role.name === RoleName.OWNER) { - const users = await this.accessService.getProjectUsersForRole( - role.id, - projectId, - ); - if (users.length < 2) { - throw new Error('A project must have at least one owner'); - } - } + await this.validateAtLeastOneOwner(projectId, role); await this.accessService.removeUserFromRole(userId, role.id, projectId); @@ -338,6 +325,76 @@ export default class ProjectService { ); } + async findProjectRole( + projectId: string, + roleId: number, + ): Promise { + const roles = await this.accessService.getRolesForProject(projectId); + const role = roles.find((r) => r.id === roleId); + if (!role) { + throw new NotFoundError( + `Couldn't find roleId=${roleId} on project=${projectId}`, + ); + } + return role; + } + + async validateAtLeastOneOwner( + projectId: string, + currentRole: IRoleDescriptor, + ): Promise { + if (currentRole.name === RoleName.OWNER) { + const users = await this.accessService.getProjectUsersForRole( + currentRole.id, + projectId, + ); + if (users.length < 2) { + throw new Error('A project must have at least one owner'); + } + } + } + + async changeRole( + projectId: string, + roleId: number, + userId: number, + createdBy: string, + ): Promise { + const role = await this.findProjectRole(projectId, roleId); + + const usersWithRoles = await this.getUsersWithAccess(projectId); + const user = usersWithRoles.users.find((u) => u.id === userId); + const currentRole = usersWithRoles.roles.find( + (r) => r.id === user.roleId, + ); + + if (currentRole.id === roleId) { + // Nothing to do.... + return; + } + + await this.validateAtLeastOneOwner(projectId, currentRole); + + await this.accessService.updateUserProjectRole( + userId, + roleId, + projectId, + ); + + await this.eventStore.store( + new ProjectUserUpdateRoleEvent({ + project: projectId, + createdBy, + preData: { + userId, + roleId: currentRole.id, + roleName: currentRole.name, + }, + data: { userId, roleId, roleName: role.name }, + }), + ); + } + async getMembers(projectId: string): Promise { return this.store.getMembers(projectId); } @@ -366,5 +423,3 @@ export default class ProjectService { }; } } - -module.exports = ProjectService; diff --git a/src/lib/types/events.ts b/src/lib/types/events.ts index bdf8b37bc1..2f5b32b6d3 100644 --- a/src/lib/types/events.ts +++ b/src/lib/types/events.ts @@ -41,6 +41,7 @@ export const PROJECT_DELETED = 'project-deleted'; export const PROJECT_IMPORT = 'project-import'; export const PROJECT_USER_ADDED = 'project-user-added'; export const PROJECT_USER_REMOVED = 'project-user-removed'; +export const PROJECT_USER_ROLE_CHANGED = 'project-user-role-changed'; export const DROP_PROJECTS = 'drop-projects'; export const TAG_CREATED = 'tag-created'; export const TAG_DELETED = 'tag-deleted'; @@ -412,3 +413,24 @@ export class ProjectUserRemovedEvent extends BaseEvent { this.preData = preData; } } + +export class ProjectUserUpdateRoleEvent extends BaseEvent { + readonly project: string; + + readonly data: any; + + readonly preData: any; + + constructor(p: { + project: string; + createdBy: string; + data: any; + preData: any; + }) { + super(PROJECT_USER_REMOVED, p.createdBy); + const { project, data, preData } = p; + this.project = project; + this.data = data; + this.preData = preData; + } +} diff --git a/src/lib/types/stores/access-store.ts b/src/lib/types/stores/access-store.ts index bac68b07c0..dd63603f7a 100644 --- a/src/lib/types/stores/access-store.ts +++ b/src/lib/types/stores/access-store.ts @@ -19,6 +19,7 @@ export interface IRoleWithPermissions extends IRole { } export interface IRoleDescriptor { + id: number; name: string; description?: string; type: string; @@ -51,6 +52,11 @@ export interface IAccessStore extends Store { roleId: number, projectId?: string, ): Promise; + updateUserProjectRole( + userId: number, + roleId: number, + projectId: string, + ): Promise; removeRolesOfTypeForUser(userId: number, roleType: string): Promise; addPermissionsToRole( role_id: number, diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index fac909c19e..2f72fc3e1b 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -652,3 +652,64 @@ test('should change a users role in the project', async () => { expect(customUser[0].id).toBe(projectUser.id); expect(customUser[0].name).toBe(projectUser.name); }); + +test('should update role for user on project', async () => { + const project = { + id: 'update-users', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, user); + + const projectMember1 = await stores.userStore.insert({ + name: 'Some Member', + email: 'update99@getunleash.io', + }); + + const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); + const ownerRole = await stores.roleStore.getRoleByName(RoleName.OWNER); + + await projectService.addUser(project.id, memberRole.id, projectMember1.id); + await projectService.changeRole( + project.id, + ownerRole.id, + projectMember1.id, + 'test', + ); + + const { users } = await projectService.getUsersWithAccess(project.id, user); + const memberUsers = users.filter((u) => u.roleId === memberRole.id); + const ownerUsers = users.filter((u) => u.roleId === ownerRole.id); + + expect(memberUsers).toHaveLength(0); + expect(ownerUsers).toHaveLength(2); +}); + +test('should not update role for user on project when she is the owner', async () => { + const project = { + id: 'update-users-not-allowed', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, user); + + const projectMember1 = await stores.userStore.insert({ + name: 'Some Member', + email: 'update991@getunleash.io', + }); + + const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER); + + await projectService.addUser(project.id, memberRole.id, projectMember1.id); + + await expect(async () => { + await projectService.changeRole( + project.id, + memberRole.id, + user.id, + 'test', + ); + }).rejects.toThrowError( + new Error('A project must have at least one owner'), + ); +}); diff --git a/src/test/fixtures/fake-access-store.ts b/src/test/fixtures/fake-access-store.ts index cc50d1dd18..0194ed18ae 100644 --- a/src/test/fixtures/fake-access-store.ts +++ b/src/test/fixtures/fake-access-store.ts @@ -9,6 +9,14 @@ import { import { IAvailablePermissions, IPermission } from 'lib/types/model'; class AccessStoreMock implements IAccessStore { + updateUserProjectRole( + userId: number, + roleId: number, + projectId: string, + ): Promise { + throw new Error('Method not implemented.'); + } + removeUserFromRole( userId: number, roleId: number,