diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts index 5efc573f05..e0d3a90910 100644 --- a/src/lib/db/access-store.ts +++ b/src/lib/db/access-store.ts @@ -20,6 +20,7 @@ const T = { ROLES: 'roles', ROLE_PERMISSION: 'role_permission', PERMISSIONS: 'permissions', + PERMISSION_TYPES: 'permission_types', }; interface IPermissionRow { @@ -60,6 +61,10 @@ export class AccessStore implements IAccessStore { await this.db.batchInsert(T.PERMISSIONS, rows); } + async getRoleByName(name: string): Promise { + return this.db(T.ROLES).where({ name }).first(); + } + async delete(key: number): Promise { await this.db(T.ROLES).where({ id: key }).del(); } @@ -93,8 +98,20 @@ export class AccessStore implements IAccessStore { async getAvailablePermissions(): Promise { const rows = await this.db - .select(['id', 'permission', 'environment', 'display_name']) - .from(T.PERMISSIONS); + .select([ + 'p.id', + 'p.permission', + 'p.environment', + 'pt.display_name', + ]) + .join( + `${T.PERMISSION_TYPES} AS pt`, + 'pt.permission', + 'p.permission', + ) + .where('pt.type', 'project') + .orWhere('pt.type', 'environment') + .from(`${T.PERMISSIONS} as p`); let projectPermissions: IPermission[] = []; let rawEnvironments = new Map(); @@ -174,15 +191,24 @@ export class AccessStore implements IAccessStore { return this.db .select(['id', 'name', 'type', 'description']) .from(T.ROLES) - .andWhere('type', 'project'); + .where('type', 'custom') + .orWhere('type', 'project'); } async getRolesForProject(projectId: string): Promise { return this.db - .select(['id', 'name', 'type', 'project', 'description']) - .from(T.ROLES) - .where('project', projectId) - .andWhere('type', 'project'); + .select(['r.id', 'r.name', 'r.type', 'ru.project', 'r.description']) + .from(`${T.ROLE_USER} as ru`) + .innerJoin(`${T.ROLES} as r`, 'ru.role_id', 'r.id') + .where('project', projectId); + } + + async unlinkUserRoles(userId: number): Promise { + return this.db(T.ROLE_USER) + .where({ + user_id: userId, + }) + .delete(); } async getRootRoles(): Promise { @@ -208,15 +234,25 @@ export class AccessStore implements IAccessStore { .where('ru.user_id', '=', userId); } - async getUserIdsForRole( - roleId: number, - projectId?: string, - ): Promise { + async getUserIdsForRole(roleId: number): Promise { const rows = await this.db .select(['user_id']) .from(T.ROLE_USER) - .where('role_id', roleId) - .andWhere('project', projectId); + .where('role_id', roleId); + return rows.map((r) => r.user_id); + } + + async getProjectUserIdsForRole( + roleId: number, + projectId?: string, + ): Promise { + console.log('Checking for', roleId, projectId); + const rows = await this.db + .select(['user_id']) + .from(`${T.ROLE_USER} AS ru`) + .join(`${T.ROLES} as r`, 'ru.role_id', 'id') + .where('r.id', roleId) + .andWhere('ru.project', projectId); return rows.map((r) => r.user_id); } @@ -232,11 +268,16 @@ export class AccessStore implements IAccessStore { }); } - async removeUserFromRole(userId: number, roleId: number): Promise { + async removeUserFromRole( + userId: number, + roleId: number, + projectId: string, + ): Promise { return this.db(T.ROLE_USER) .where({ user_id: userId, role_id: roleId, + project: projectId, }) .delete(); } diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts index 7efff1cbf6..9cb6c145c7 100644 --- a/src/lib/services/access-service.ts +++ b/src/lib/services/access-service.ts @@ -125,6 +125,7 @@ export class AccessService { permission: p, })); } + console.log('Checking perms for user id', user.id); return this.store.getPermissionsForUser(user.id); } @@ -171,8 +172,12 @@ export class AccessService { return userRoles.filter((r) => r.type === RoleType.ROOT); } - async removeUserFromRole(userId: number, roleId: number): Promise { - return this.store.removeUserFromRole(userId, roleId); + async removeUserFromRole( + userId: number, + roleId: number, + projectId: string, + ): Promise { + return this.store.removeUserFromRole(userId, roleId, projectId); } async addPermissionToRole( @@ -230,11 +235,23 @@ export class AccessService { return this.store.getRolesForUserId(userId); } - async getUsersForRole( + async unlinkUserRoles(userId: number): Promise { + return this.store.unlinkUserRoles(userId); + } + + async getUsersForRole(roleId: number): Promise { + const userIdList = await this.store.getUserIdsForRole(roleId); + if (userIdList.length > 0) { + return this.userStore.getAllWithId(userIdList); + } + return []; + } + + async getProjectUsersForRole( roleId: number, projectId?: string, ): Promise { - const userIdList = await this.store.getUserIdsForRole( + const userIdList = await this.store.getProjectUserIdsForRole( roleId, projectId, ); @@ -250,9 +267,13 @@ export class AccessService { ): Promise<[IRole[], IUserWithRole[]]> { const roles = await this.store.getProjectRoles(); + console.log('GOt the following roles bacl', roles); const users = await Promise.all( roles.map(async (role) => { - const usrs = await this.getUsersForRole(role.id, projectId); + const usrs = await this.getProjectUsersForRole( + role.id, + projectId, + ); return usrs.map((u) => ({ ...u, roleId: role.id })); }), ); @@ -267,11 +288,8 @@ export class AccessService { throw new Error('ProjectId cannot be empty'); } - const ownerRole = await this.store.createRole( - RoleName.OWNER, - RoleType.PROJECT, - PROJECT_DESCRIPTION.OWNER, - ); + const ownerRole = await this.store.getRoleByName(RoleName.OWNER); + await this.store.addPermissionsToRole( ownerRole.id, PROJECT_ADMIN, @@ -285,12 +303,9 @@ export class AccessService { ); await this.store.addUserToRole(owner.id, ownerRole.id, projectId); } - const memberRole = await this.store.createRole( - RoleName.MEMBER, - RoleType.PROJECT, - projectId, - PROJECT_DESCRIPTION.MEMBER, - ); + + const memberRole = await this.store.getRoleByName(RoleName.MEMBER); + await this.store.addPermissionsToRole( memberRole.id, PROJECT_REGULAR, diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index c08485ff0f..1849332c3f 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -25,7 +25,7 @@ import { IFeatureTypeStore } from '../types/stores/feature-type-store'; import { IFeatureToggleStore } from '../types/stores/feature-toggle-store'; import { IFeatureEnvironmentStore } from '../types/stores/feature-environment-store'; import { IProjectQuery, IProjectStore } from '../types/stores/project-store'; -import { IRole } from '../types/stores/access-store'; +import { IRoleDescriptor } from '../types/stores/access-store'; import { IEventStore } from '../types/stores/event-store'; import FeatureToggleService from './feature-toggle-service'; import { CREATE_FEATURE, UPDATE_FEATURE } from '../types/permissions'; @@ -38,7 +38,7 @@ const DEFAULT_PROJECT = 'default'; export interface UsersWithRoles { users: IUserWithRole[]; - roles: IRole[]; + roles: IRoleDescriptor[]; } export default class ProjectService { @@ -271,6 +271,7 @@ export default class ProjectService { const [roles, users] = await this.accessService.getProjectRoleUsers( projectId, ); + console.log('Got the following response', roles, users); return { roles, @@ -324,7 +325,7 @@ export default class ProjectService { } } - await this.accessService.removeUserFromRole(userId, role.id); + await this.accessService.removeUserFromRole(userId, role.id, projectId); } async getMembers(projectId: string): Promise { diff --git a/src/lib/services/user-service.ts b/src/lib/services/user-service.ts index 5c5cdb3508..8d22d17256 100644 --- a/src/lib/services/user-service.ts +++ b/src/lib/services/user-service.ts @@ -262,12 +262,7 @@ class UserService { async deleteUser(userId: number, updatedBy?: User): Promise { const user = await this.store.get(userId); - const roles = await this.accessService.getRolesForUser(userId); - await Promise.all( - roles.map((role) => - this.accessService.removeUserFromRole(userId, role.id), - ), - ); + await this.accessService.unlinkUserRoles(userId); await this.sessionService.deleteSessionsForUser(userId); await this.store.delete(userId); diff --git a/src/lib/types/stores/access-store.ts b/src/lib/types/stores/access-store.ts index eba9df7fa6..a6d4390dc5 100644 --- a/src/lib/types/stores/access-store.ts +++ b/src/lib/types/stores/access-store.ts @@ -14,20 +14,29 @@ export interface IRole { type: string; } +export interface IRoleDescriptor { + name: string; + description?: string; + type: string; +} + export interface IUserRole { roleId: number; userId: number; } export interface IAccessStore extends Store { + getRoleByName(name: string): Promise; getAvailablePermissions(): Promise; getPermissionsForUser(userId: number): Promise; getPermissionsForRole(roleId: number): Promise; getRoles(): Promise; getRolesForProject(projectId: string): Promise; + unlinkUserRoles(userId: number): Promise; getProjectRoles(): Promise; getRootRoles(): Promise; removeRolesForProject(projectId: string): Promise; getRolesForUserId(userId: number): Promise; + getProjectUserIdsForRole(roleId: number, projectId?: string); getUserIdsForRole(roleId: number, projectId?: string): Promise; addEnvironmentPermissionsToRole( role_id: number, @@ -42,7 +51,11 @@ export interface IAccessStore extends Store { roleId: number, projectId: string, ): Promise; - removeUserFromRole(userId: number, roleId: number): Promise; + removeUserFromRole( + userId: number, + roleId: number, + projectId: string, + ): Promise; removeRolesOfTypeForUser(userId: number, roleType: string): Promise; createRole( name: string, diff --git a/src/migrations/20211202120808-add-custom-roles.js b/src/migrations/20211202120808-add-custom-roles.js index a402bfb562..3fb44fda10 100644 --- a/src/migrations/20211202120808-add-custom-roles.js +++ b/src/migrations/20211202120808-add-custom-roles.js @@ -5,11 +5,16 @@ exports.up = function (db, cb) { ( id SERIAL PRIMARY KEY, permission VARCHAR(255) NOT NULL, environment VARCHAR(255), - display_name TEXT, created_at TIMESTAMP WITH TIME ZONE DEFAULT now() ); - INSERT INTO permissions (permission, environment, display_name) (SELECT DISTINCT permission, environment, '' from role_permission); + CREATE TABLE IF NOT EXISTS permission_types ( + permission VARCHAR(255), + display_name TEXT, + type VARCHAR(255) + ); + + INSERT INTO permissions (permission, environment) (select distinct permission,environment from role_permission); ALTER TABLE role_user ADD COLUMN project VARCHAR(255); @@ -19,6 +24,9 @@ exports.up = function (db, cb) { FROM roles WHERE role_user.role_id = roles.id; + ALTER TABLE role_user DROP CONSTRAINT role_user_pkey; + ALTER TABLE role_user ADD PRIMARY KEY (role_id, user_id, project); + ALTER TABLE roles DROP COLUMN project; ALTER TABLE roles @@ -29,7 +37,7 @@ exports.up = function (db, cb) { ADD COLUMN permission_id INTEGER; - UPDATE role_permission + UPDATE role_permission SET permission_id = permissions.id FROM permissions WHERE @@ -43,29 +51,30 @@ exports.up = function (db, cb) { DROP COLUMN permission, DROP COLUMN environment; - UPDATE permissions SET display_name = 'Admin' where permission = 'ADMIN'; - UPDATE permissions SET display_name = 'Create Strategies' where permission = 'CREATE_STRATEGY'; - UPDATE permissions SET display_name = 'Create Addons' where permission = 'CREATE_ADDON'; - UPDATE permissions SET display_name = 'Delete Addons' where permission = 'DELETE_ADDON'; - UPDATE permissions SET display_name = 'Update Addons' where permission = 'UPDATE_ADDON'; - UPDATE permissions SET display_name = 'Create Feature Toggles' where permission = 'CREATE_FEATURE'; - UPDATE permissions SET display_name = 'Update Feature Toggles' where permission = 'UPDATE_FEATURE'; - UPDATE permissions SET display_name = 'Delete Feature Toggles' where permission = 'DELETE_FEATURE'; - UPDATE permissions SET display_name = 'Update Applications' where permission = 'UPDATE_APPLICATION'; - UPDATE permissions SET display_name = 'Update Tag Types' where permission = 'UPDATE_TAG_TYPE'; - UPDATE permissions SET display_name = 'Delete Tag Types' where permission = 'DELETE_TAG_TYPE'; - UPDATE permissions SET display_name = 'Create Projects' where permission = 'CREATE_PROJECT'; - UPDATE permissions SET display_name = 'Update Projects' where permission = 'UPDATE_PROJECT'; - UPDATE permissions SET display_name = 'Delete Projects' where permission = 'DELETE_PROJECT'; - UPDATE permissions SET display_name = 'Update Strategies on Toggles' where permission = 'UPDATE_FEATURE_STRATEGY'; - UPDATE permissions SET display_name = 'Add Strategies to Toggles' where permission = 'CREATE_FEATURE_STRATEGY'; - UPDATE permissions SET display_name = 'Remove Strategies from Toggles' where permission = 'DELETE_FEATURE_STRATEGY'; - UPDATE permissions SET display_name = 'Update Strategies' where permission = 'UPDATE_STRATEGY'; - UPDATE permissions SET display_name = 'Delete Strategies' where permission = 'DELETE_STRATEGY'; - UPDATE permissions SET display_name = 'Enable/Disable Toggles for Environments' where permission = 'UPDATE_FEATURE_ENVIRONMENT'; - UPDATE permissions SET display_name = 'Update Context Fields' where permission = 'UPDATE_CONTEXT_FIELD'; - UPDATE permissions SET display_name = 'Create Context Fields' where permission = 'CREATE_CONTEXT_FIELD'; - UPDATE permissions SET display_name = 'Delete Context Fields' where permission = 'DELETE_CONTEXT_FIELD'; + INSERT INTO permission_types (permission, display_name, type) VALUES ('ADMIN', 'Admin', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('CLIENT', 'Client', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_STRATEGY','Create Strategies', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_ADDON', 'Create Addons', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_ADDON', 'Delete Addons', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_ADDON', 'Update Addons', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_FEATURE', 'Create Feature Toggles', 'project'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_FEATURE', 'Update Feature Toggles', 'project'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_FEATURE', 'Delete Feature Toggles', 'project'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_APPLICATION', 'Update Applications', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_TAG_TYPE', 'Update Tag Types', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_TAG_TYPE', 'Delete Tag Types', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_PROJECT', 'Create Projects', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_PROJECT', 'Update Projects', 'project'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_PROJECT', 'Delete Projects', 'project'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_FEATURE_STRATEGY', 'Update Strategies on Toggles', 'environment'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_FEATURE_STRATEGY', 'Add Strategies to Toggles', 'environment'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_FEATURE_STRATEGY', 'Remove Strategies from Toggles', 'environment'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_STRATEGY', 'Update Strategies', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_STRATEGY', 'Delete Strategies', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_FEATURE_ENVIRONMENT', 'Enable/Disable Toggles for Environments', 'environment'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_CONTEXT_FIELD', 'Update Context Fields', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_CONTEXT_FIELD', 'Create Context Fields', 'root'); + INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_CONTEXT_FIELD', 'Delete Context Fields', 'root'); `, cb, ); diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index 5f8f0e9fcc..485ba222d2 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -53,7 +53,12 @@ afterEach(async () => { .map(async (env) => { await stores.environmentStore.delete(env.name); }); + const users = await stores.userStore.getAll(); + const wipeUserPermissions = users.map(async (u) => { + await stores.accessStore.unlinkUserRoles(u.id); + }); await Promise.allSettled(deleteEnvs); + await Promise.allSettled(wipeUserPermissions); }); test('should have default project', async () => { @@ -240,13 +245,10 @@ test('should get list of users with access to project', async () => { description: 'Blah', }; await projectService.createProject(project, user); - const { roles, users } = await projectService.getUsersWithAccess( - project.id, - user, - ); + const { users } = await projectService.getUsersWithAccess(project.id, user); - const owner = roles.find((role) => role.name === RoleName.OWNER); - const member = roles.find((role) => role.name === RoleName.MEMBER); + const member = await stores.accessStore.getRoleByName(RoleName.MEMBER); + const owner = await stores.accessStore.getRoleByName(RoleName.OWNER); expect(users).toHaveLength(1); expect(users[0].id).toBe(user.id); @@ -272,8 +274,7 @@ test('should add a member user to the project', async () => { email: 'member2@getunleash.io', }); - const roles = await stores.accessStore.getRolesForProject(project.id); - const memberRole = roles.find((r) => r.name === RoleName.MEMBER); + const memberRole = await stores.accessStore.getRoleByName(RoleName.MEMBER); await projectService.addUser(project.id, memberRole.id, projectMember1.id); await projectService.addUser(project.id, memberRole.id, projectMember2.id); @@ -305,10 +306,7 @@ test('should add admin users to the project', async () => { email: 'admin2@getunleash.io', }); - const projectRoles = await stores.accessStore.getRolesForProject( - project.id, - ); - const ownerRole = projectRoles.find((r) => r.name === RoleName.OWNER); + const ownerRole = await stores.accessStore.getRoleByName(RoleName.OWNER); await projectService.addUser(project.id, ownerRole.id, projectAdmin1.id); await projectService.addUser(project.id, ownerRole.id, projectAdmin2.id); @@ -325,8 +323,7 @@ test('should add admin users to the project', async () => { }); test('add user only accept to add users to project roles', async () => { - const roles = await accessService.getRoles(); - const memberRole = roles.find((r) => r.name === RoleName.MEMBER); + const memberRole = await stores.accessStore.getRoleByName(RoleName.MEMBER); await expect(async () => { await projectService.addUser('some-id', memberRole.id, user.id); @@ -346,8 +343,7 @@ test('add user should fail if user already have access', async () => { email: 'member42@getunleash.io', }); - const roles = await stores.accessStore.getRolesForProject(project.id); - const memberRole = roles.find((r) => r.name === RoleName.MEMBER); + const memberRole = await stores.accessStore.getRoleByName(RoleName.MEMBER); await projectService.addUser(project.id, memberRole.id, projectMember1.id); @@ -371,8 +367,7 @@ test('should remove user from the project', async () => { email: 'member99@getunleash.io', }); - const roles = await stores.accessStore.getRolesForProject(project.id); - const memberRole = roles.find((r) => r.name === RoleName.MEMBER); + const memberRole = await stores.accessStore.getRoleByName(RoleName.MEMBER); await projectService.addUser(project.id, memberRole.id, projectMember1.id); await projectService.removeUser( diff --git a/src/test/fixtures/fake-access-store.ts b/src/test/fixtures/fake-access-store.ts index bba849130e..dd78a7562c 100644 --- a/src/test/fixtures/fake-access-store.ts +++ b/src/test/fixtures/fake-access-store.ts @@ -9,6 +9,21 @@ import { import { IAvailablePermissions, IPermission } from 'lib/types/model'; class AccessStoreMock implements IAccessStore { + unlinkUserRoles(userId: number): Promise { + throw new Error('Method not implemented.'); + } + + getRoleByName(name: string): Promise { + throw new Error('Method not implemented.'); + } + + getProjectUserIdsForRole( + roleId: number, + projectId?: string, + ): Promise { + throw new Error('Method not implemented.'); + } + getProjectRoles(): Promise { throw new Error('Method not implemented.'); }