diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts index 5dffb31e3e..f01a61a498 100644 --- a/src/lib/db/access-store.ts +++ b/src/lib/db/access-store.ts @@ -224,10 +224,10 @@ export class AccessStore implements IAccessStore { return rows.map((r) => r.user_id); } - async addUserToProjectRole( + async addUserToRole( userId: number, roleId: number, - projecId: string, + projecId?: string, ): Promise { return this.db(T.ROLE_USER).insert({ user_id: userId, @@ -236,10 +236,10 @@ export class AccessStore implements IAccessStore { }); } - async removeUserFromProjectRole( + async removeUserFromRole( userId: number, roleId: number, - projectId: string, + projectId?: string, ): Promise { return this.db(T.ROLE_USER) .where({ diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts index e865b34528..b0878c88bc 100644 --- a/src/lib/services/access-service.ts +++ b/src/lib/services/access-service.ts @@ -170,12 +170,12 @@ export class AccessService { }; } - async addUserToProjectRole( + async addUserToRole( userId: number, roleId: number, projectId: string, ): Promise { - return this.store.addUserToProjectRole(userId, roleId, projectId); + return this.store.addUserToRole(userId, roleId, projectId); } async getRoleByName(roleName: string): Promise { @@ -194,7 +194,7 @@ export class AccessService { RoleType.ROOT, ); - await this.store.addUserToProjectRole( + await this.store.addUserToRole( userId, newRootRole.id, ALL_PROJECTS, @@ -214,14 +214,15 @@ export class AccessService { return userRoles.filter((r) => r.type === RoleType.ROOT); } - async removeUserFromProjectRole( + async removeUserFromRole( userId: number, roleId: number, projectId: string, ): Promise { - return this.store.removeUserFromProjectRole(userId, roleId, projectId); + return this.store.removeUserFromRole(userId, roleId, projectId); } + //This actually only exists for testing purposes async addPermissionToRole( roleId: number, permission: string, @@ -239,6 +240,7 @@ export class AccessService { ); } + //This actually only exists for testing purposes async removePermissionFromRole( roleId: number, permission: string, @@ -324,11 +326,11 @@ export class AccessService { const users = await Promise.all( roles.map(async (role) => { - const usrs = await this.getProjectUsersForRole( + const projectUsers = await this.getProjectUsersForRole( role.id, projectId, ); - return usrs.map((u) => ({ ...u, roleId: role.id })); + return projectUsers.map((u) => ({ ...u, roleId: role.id })); }), ); return [roles, users.flat()]; @@ -349,11 +351,7 @@ export class AccessService { this.logger.info( `Making ${owner.id} admin of ${projectId} via roleId=${ownerRole.id}`, ); - await this.store.addUserToProjectRole( - owner.id, - ownerRole.id, - projectId, - ); + await this.store.addUserToRole(owner.id, ownerRole.id, projectId); } } diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index a71c644a79..85a9fcc1a0 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -300,11 +300,7 @@ export default class ProjectService { throw new Error(`User already has access to project=${projectId}`); } - await this.accessService.addUserToProjectRole( - userId, - role.id, - projectId, - ); + await this.accessService.addUserToRole(userId, role.id, projectId); } // TODO: should be an event too @@ -328,7 +324,7 @@ export default class ProjectService { } } - await this.accessService.removeUserFromProjectRole( + await this.accessService.removeUserFromRole( userId, role.id, projectId, diff --git a/src/lib/types/stores/access-store.ts b/src/lib/types/stores/access-store.ts index aa0b891d15..bac68b07c0 100644 --- a/src/lib/types/stores/access-store.ts +++ b/src/lib/types/stores/access-store.ts @@ -41,15 +41,15 @@ export interface IAccessStore extends Store { role_id: number, permissions: IPermission[], ): Promise; - addUserToProjectRole( + addUserToRole( userId: number, roleId: number, - projectId: string, + projectId?: string, ): Promise; - removeUserFromProjectRole( + removeUserFromRole( userId: number, roleId: number, - projectId: string, + projectId?: string, ): Promise; removeRolesOfTypeForUser(userId: number, roleType: string): Promise; addPermissionsToRole( diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index eb13bca915..c66e8dcf92 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -28,14 +28,14 @@ let readRole; const createUserEditorAccess = async (name, email) => { const { userStore } = stores; const user = await userStore.insert({ name, email }); - await accessService.addUserToProjectRole(user.id, editorRole.id, 'default'); + await accessService.addUserToRole(user.id, editorRole.id, 'default'); return user; }; const createUserViewerAccess = async (name, email) => { const { userStore } = stores; const user = await userStore.insert({ name, email }); - await accessService.addUserToProjectRole( + await accessService.addUserToRole( user.id, readRole.id, ALL_PROJECTS, @@ -182,7 +182,7 @@ const createSuperUser = async () => { name: 'Alice Admin', email: 'admin@getunleash.io', }); - await accessService.addUserToProjectRole( + await accessService.addUserToRole( user.id, adminRole.id, ALL_PROJECTS, @@ -380,7 +380,7 @@ test('should grant user access to project', async () => { await accessService.createDefaultProjectRoles(user, project); const projectRole = await accessService.getRoleByName(RoleName.MEMBER); - await accessService.addUserToProjectRole(sUser.id, projectRole.id, project); + await accessService.addUserToRole(sUser.id, projectRole.id, project); // // Should be able to update feature toggles inside the project hasCommonProjectAccess(sUser, project, true); @@ -405,7 +405,7 @@ test('should not get access if not specifying project', async () => { const projectRole = await accessService.getRoleByName(RoleName.MEMBER); - await accessService.addUserToProjectRole(sUser.id, projectRole.id, project); + await accessService.addUserToRole(sUser.id, projectRole.id, project); // Should not be able to update feature toggles outside project hasCommonProjectAccess(sUser, undefined, false); @@ -418,14 +418,14 @@ test('should remove user from role', async () => { email: 'random123@getunleash.io', }); - await accessService.addUserToProjectRole(user.id, editorRole.id, 'default'); + await accessService.addUserToRole(user.id, editorRole.id, 'default'); // check user has one role const userRoles = await accessService.getRolesForUser(user.id); expect(userRoles.length).toBe(1); expect(userRoles[0].name).toBe(RoleName.EDITOR); - await accessService.removeUserFromProjectRole( + await accessService.removeUserFromRole( user.id, editorRole.id, 'default', @@ -441,7 +441,7 @@ test('should return role with users', async () => { email: 'random2223@getunleash.io', }); - await accessService.addUserToProjectRole(user.id, editorRole.id, 'default'); + await accessService.addUserToRole(user.id, editorRole.id, 'default'); const roleWithUsers = await accessService.getRoleData(editorRole.id); expect(roleWithUsers.role.name).toBe(RoleName.EDITOR); @@ -459,7 +459,7 @@ test('should return role with permissions and users', async () => { email: 'random2244@getunleash.io', }); - await accessService.addUserToProjectRole(user.id, editorRole.id, 'default'); + await accessService.addUserToRole(user.id, editorRole.id, 'default'); const roleWithPermission = await accessService.getRoleData(editorRole.id); @@ -548,11 +548,7 @@ test('should support permission with "ALL" environment requirement', async () => [CREATE_FEATURE_STRATEGY], 'production', ); - await accessStore.addUserToProjectRole( - user.id, - customRole.id, - ALL_PROJECTS, - ); + await accessStore.addUserToRole(user.id, customRole.id, ALL_PROJECTS); const hasAccess = await accessService.hasPermission( user, diff --git a/src/test/fixtures/access-service-mock.ts b/src/test/fixtures/access-service-mock.ts index e2b313b3fb..d11c945b5f 100644 --- a/src/test/fixtures/access-service-mock.ts +++ b/src/test/fixtures/access-service-mock.ts @@ -35,7 +35,7 @@ class AccessServiceMock extends AccessService { throw new Error('Method not implemented.'); } - addUserToProjectRole(userId: number, roleId: number): Promise { + addUserToRole(userId: number, roleId: number): Promise { throw new Error('Method not implemented.'); } diff --git a/src/test/fixtures/fake-access-store.ts b/src/test/fixtures/fake-access-store.ts index 8603683d9c..cc50d1dd18 100644 --- a/src/test/fixtures/fake-access-store.ts +++ b/src/test/fixtures/fake-access-store.ts @@ -9,7 +9,7 @@ import { import { IAvailablePermissions, IPermission } from 'lib/types/model'; class AccessStoreMock implements IAccessStore { - removeUserFromProjectRole( + removeUserFromRole( userId: number, roleId: number, projectId: string, @@ -87,7 +87,7 @@ class AccessStoreMock implements IAccessStore { throw new Error('Method not implemented.'); } - addUserToProjectRole(userId: number, roleId: number): Promise { + addUserToRole(userId: number, roleId: number): Promise { throw new Error('Method not implemented.'); }