fix: Make permissions actually work with new environments

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<IRole> {
+        return this.db(T.ROLES).where({ name }).first();
+    }
+
     async delete(key: number): Promise<void> {
         await this.db(T.ROLES).where({ id: key }).del();
     }
@@ -93,8 +98,20 @@ export class AccessStore implements IAccessStore {
 
     async getAvailablePermissions(): Promise<IAvailablePermissions> {
         const rows = await this.db
-            .select(['id', 'permission', 'environment', 'display_name'])
+            .select([
-            .from(T.PERMISSIONS);
+                '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<string, IPermissionRow[]>();
@@ -174,15 +191,24 @@ export class AccessStore implements IAccessStore {
         return this.db
             .select(['id', 'name', 'type', 'description'])
             .from<IRole>(T.ROLES)
-            .andWhere('type', 'project');
+            .where('type', 'custom')
+            .orWhere('type', 'project');
     }
 
     async getRolesForProject(projectId: string): Promise<IRole[]> {
         return this.db
-            .select(['id', 'name', 'type', 'project', 'description'])
+            .select(['r.id', 'r.name', 'r.type', 'ru.project', 'r.description'])
-            .from<IRole>(T.ROLES)
+            .from<IRole>(`${T.ROLE_USER} as ru`)
-            .where('project', projectId)
+            .innerJoin(`${T.ROLES} as r`, 'ru.role_id', 'r.id')
-            .andWhere('type', 'project');
+            .where('project', projectId);
+    }
+
+    async unlinkUserRoles(userId: number): Promise<void> {
+        return this.db(T.ROLE_USER)
+            .where({
+                user_id: userId,
+            })
+            .delete();
     }
 
     async getRootRoles(): Promise<IRole[]> {
@@ -208,15 +234,25 @@ export class AccessStore implements IAccessStore {
             .where('ru.user_id', '=', userId);
     }
 
-    async getUserIdsForRole(
+    async getUserIdsForRole(roleId: number): Promise<number[]> {
-        roleId: number,
-        projectId?: string,
-    ): Promise<number[]> {
         const rows = await this.db
             .select(['user_id'])
             .from<IRole>(T.ROLE_USER)
-            .where('role_id', roleId)
+            .where('role_id', roleId);
-            .andWhere('project', projectId);
+        return rows.map((r) => r.user_id);
+    }
+
+    async getProjectUserIdsForRole(
+        roleId: number,
+        projectId?: string,
+    ): Promise<number[]> {
+        console.log('Checking for', roleId, projectId);
+        const rows = await this.db
+            .select(['user_id'])
+            .from<IRole>(`${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<void> {
+    async removeUserFromRole(
+        userId: number,
+        roleId: number,
+        projectId: string,
+    ): Promise<void> {
         return this.db(T.ROLE_USER)
             .where({
                 user_id: userId,
                 role_id: roleId,
+                project: projectId,
             })
             .delete();
     }

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<void> {
+    async removeUserFromRole(
-        return this.store.removeUserFromRole(userId, roleId);
+        userId: number,
+        roleId: number,
+        projectId: string,
+    ): Promise<void> {
+        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<void> {
+        return this.store.unlinkUserRoles(userId);
+    }
+
+    async getUsersForRole(roleId: number): Promise<IUser[]> {
+        const userIdList = await this.store.getUserIdsForRole(roleId);
+        if (userIdList.length > 0) {
+            return this.userStore.getAllWithId(userIdList);
+        }
+        return [];
+    }
+
+    async getProjectUsersForRole(
         roleId: number,
         projectId?: string,
     ): Promise<IUser[]> {
-        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(
+        const ownerRole = await this.store.getRoleByName(RoleName.OWNER);
-            RoleName.OWNER,
+
-            RoleType.PROJECT,
-            PROJECT_DESCRIPTION.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,
+        const memberRole = await this.store.getRoleByName(RoleName.MEMBER);
-            RoleType.PROJECT,
+
-            projectId,
-            PROJECT_DESCRIPTION.MEMBER,
-        );
         await this.store.addPermissionsToRole(
             memberRole.id,
             PROJECT_REGULAR,

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<number> {

src/lib/services/user-service.ts

@@ -262,12 +262,7 @@ class UserService {
 
     async deleteUser(userId: number, updatedBy?: User): Promise<void> {
         const user = await this.store.get(userId);
-        const roles = await this.accessService.getRolesForUser(userId);
+        await this.accessService.unlinkUserRoles(userId);
-        await Promise.all(
-            roles.map((role) =>
-                this.accessService.removeUserFromRole(userId, role.id),
-            ),
-        );
         await this.sessionService.deleteSessionsForUser(userId);
 
         await this.store.delete(userId);

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<IRole, number> {
+    getRoleByName(name: string): Promise<IRole>;
     getAvailablePermissions(): Promise<IAvailablePermissions>;
     getPermissionsForUser(userId: number): Promise<IUserPermission[]>;
     getPermissionsForRole(roleId: number): Promise<IUserPermission[]>;
     getRoles(): Promise<IRole[]>;
     getRolesForProject(projectId: string): Promise<IRole[]>;
+    unlinkUserRoles(userId: number): Promise<void>;
     getProjectRoles(): Promise<IRole[]>;
     getRootRoles(): Promise<IRole[]>;
     removeRolesForProject(projectId: string): Promise<void>;
     getRolesForUserId(userId: number): Promise<IRole[]>;
+    getProjectUserIdsForRole(roleId: number, projectId?: string);
     getUserIdsForRole(roleId: number, projectId?: string): Promise<number[]>;
     addEnvironmentPermissionsToRole(
         role_id: number,
@@ -42,7 +51,11 @@ export interface IAccessStore extends Store<IRole, number> {
         roleId: number,
         projectId: string,
     ): Promise<void>;
-    removeUserFromRole(userId: number, roleId: number): Promise<void>;
+    removeUserFromRole(
+        userId: number,
+        roleId: number,
+        projectId: string,
+    ): Promise<void>;
     removeRolesOfTypeForUser(userId: number, roleType: string): Promise<void>;
     createRole(
         name: string,

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';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('ADMIN', 'Admin', 'root');
-        UPDATE permissions SET display_name = 'Create Strategies' where permission = 'CREATE_STRATEGY';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('CLIENT', 'Client', 'root');
-        UPDATE permissions SET display_name = 'Create Addons' where permission = 'CREATE_ADDON';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_STRATEGY','Create Strategies', 'root');
-        UPDATE permissions SET display_name = 'Delete Addons' where permission = 'DELETE_ADDON';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_ADDON', 'Create Addons', 'root');
-        UPDATE permissions SET display_name = 'Update Addons' where permission = 'UPDATE_ADDON';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_ADDON', 'Delete Addons', 'root');
-        UPDATE permissions SET display_name = 'Create Feature Toggles' where permission = 'CREATE_FEATURE';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_ADDON', 'Update Addons', 'root');
-        UPDATE permissions SET display_name = 'Update Feature Toggles' where permission = 'UPDATE_FEATURE';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_FEATURE', 'Create Feature Toggles', 'project');
-        UPDATE permissions SET display_name = 'Delete Feature Toggles' where permission = 'DELETE_FEATURE';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_FEATURE', 'Update Feature Toggles', 'project');
-        UPDATE permissions SET display_name = 'Update Applications' where permission = 'UPDATE_APPLICATION';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_FEATURE', 'Delete Feature Toggles', 'project');
-        UPDATE permissions SET display_name = 'Update Tag Types' where permission = 'UPDATE_TAG_TYPE';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_APPLICATION', 'Update Applications', 'root');
-        UPDATE permissions SET display_name = 'Delete Tag Types' where permission = 'DELETE_TAG_TYPE';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_TAG_TYPE', 'Update Tag Types', 'root');
-        UPDATE permissions SET display_name = 'Create Projects' where permission = 'CREATE_PROJECT';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_TAG_TYPE', 'Delete Tag Types', 'root');
-        UPDATE permissions SET display_name = 'Update Projects' where permission = 'UPDATE_PROJECT';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_PROJECT', 'Create Projects', 'root');
-        UPDATE permissions SET display_name = 'Delete Projects' where permission = 'DELETE_PROJECT';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_PROJECT', 'Update Projects', 'project');
-        UPDATE permissions SET display_name = 'Update Strategies on Toggles' where permission = 'UPDATE_FEATURE_STRATEGY';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_PROJECT', 'Delete Projects', 'project');
-        UPDATE permissions SET display_name = 'Add Strategies to Toggles' where permission = 'CREATE_FEATURE_STRATEGY';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_FEATURE_STRATEGY', 'Update Strategies on Toggles', 'environment');
-        UPDATE permissions SET display_name = 'Remove Strategies from Toggles' where permission = 'DELETE_FEATURE_STRATEGY';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('CREATE_FEATURE_STRATEGY', 'Add Strategies to Toggles', 'environment');
-        UPDATE permissions SET display_name = 'Update Strategies' where permission = 'UPDATE_STRATEGY';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_FEATURE_STRATEGY', 'Remove Strategies from Toggles', 'environment');
-        UPDATE permissions SET display_name = 'Delete Strategies' where permission = 'DELETE_STRATEGY';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_STRATEGY', 'Update Strategies', 'root');
-        UPDATE permissions SET display_name = 'Enable/Disable Toggles for Environments' where permission = 'UPDATE_FEATURE_ENVIRONMENT';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('DELETE_STRATEGY', 'Delete Strategies', 'root');
-        UPDATE permissions SET display_name = 'Update Context Fields' where permission = 'UPDATE_CONTEXT_FIELD';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_FEATURE_ENVIRONMENT', 'Enable/Disable Toggles for Environments', 'environment');
-        UPDATE permissions SET display_name = 'Create Context Fields' where permission = 'CREATE_CONTEXT_FIELD';
+        INSERT INTO permission_types (permission, display_name, type) VALUES ('UPDATE_CONTEXT_FIELD', 'Update Context Fields', 'root');
-        UPDATE permissions SET display_name = 'Delete Context Fields' where permission = 'DELETE_CONTEXT_FIELD';
+        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,
     );

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(
+    const { users } = await projectService.getUsersWithAccess(project.id, user);
-        project.id,
-        user,
-    );
 
-    const owner = roles.find((role) => role.name === RoleName.OWNER);
+    const member = await stores.accessStore.getRoleByName(RoleName.MEMBER);
-    const member = roles.find((role) => role.name === 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 = await stores.accessStore.getRoleByName(RoleName.MEMBER);
-    const memberRole = roles.find((r) => r.name === 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(
+    const ownerRole = await stores.accessStore.getRoleByName(RoleName.OWNER);
-        project.id,
-    );
-    const ownerRole = projectRoles.find((r) => r.name === 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 = await stores.accessStore.getRoleByName(RoleName.MEMBER);
-    const memberRole = roles.find((r) => r.name === 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 = await stores.accessStore.getRoleByName(RoleName.MEMBER);
-    const memberRole = roles.find((r) => r.name === 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 = await stores.accessStore.getRoleByName(RoleName.MEMBER);
-    const memberRole = roles.find((r) => r.name === RoleName.MEMBER);
 
     await projectService.addUser(project.id, memberRole.id, projectMember1.id);
     await projectService.removeUser(

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<void> {
+        throw new Error('Method not implemented.');
+    }
+
+    getRoleByName(name: string): Promise<IRole> {
+        throw new Error('Method not implemented.');
+    }
+
+    getProjectUserIdsForRole(
+        roleId: number,
+        projectId?: string,
+    ): Promise<number[]> {
+        throw new Error('Method not implemented.');
+    }
+
     getProjectRoles(): Promise<IRole[]> {
         throw new Error('Method not implemented.');
     }