fix: fetching user root roles include custom ones (#4068)

## About the changes
`getUserRootRoles` should also consider custom root roles

This introduces test cases that unveiled a dependency between stores
(this happens actually at the DB layer having access-service access
tables from two different stores but skipping the store layer).


https://linear.app/unleash/issue/2-1161/a-user-with-custom-root-role-and-permission-to-create-client-api

---------

Co-authored-by: Nuno Góis <github@nunogois.com>

src/lib/features/access/createAccessService.ts

@@ -45,7 +45,7 @@ export const createFakeAccessService = (
     const accountStore = new FakeAccountStore();
     const roleStore = new FakeRoleStore();
     const environmentStore = new FakeEnvironmentStore();
-    const accessStore = new FakeAccessStore();
+    const accessStore = new FakeAccessStore(roleStore);
     const groupService = new GroupService(
         { groupStore, eventStore, accountStore },
         { getLogger },

src/lib/services/access-service.test.ts

@@ -1,35 +1,33 @@
 import NameExistsError from '../error/name-exists-error';
 import getLogger from '../../test/fixtures/no-logger';
-import createStores from '../../test/fixtures/store';
-import { AccessService, IRoleValidation } from './access-service';
-import { GroupService } from './group-service';
+import { createFakeAccessService } from '../features/access/createAccessService';
+import { IRoleValidation } from './access-service';
 import { createTestConfig } from '../../test/config/test-config';
+import { CUSTOM_ROOT_ROLE_TYPE } from '../util/constants';
 
-function getSetup(withNameInUse: boolean) {
-    const stores = createStores();
-
+function getSetup(customRootRoles: boolean = false) {
     const config = createTestConfig({
         getLogger,
+        experimental: {
+            flags: {
+                customRootRoles: customRootRoles,
+            },
+        },
     });
 
-    stores.roleStore = {
-        ...stores.roleStore,
-        async nameInUse(): Promise<boolean> {
-            return withNameInUse;
-        },
-    };
     return {
-        accessService: new AccessService(stores, config, {} as GroupService),
-        stores,
+        accessService: createFakeAccessService(config),
     };
 }
-test('should fail when name exists', async () => {
-    const { accessService } = getSetup(true);
 
-    const existingRole: IRoleValidation = {
+test('should fail when name exists', async () => {
+    const { accessService } = getSetup();
+    const existingRole = await accessService.createRole({
         name: 'existing role',
         description: 'description',
-    };
+        permissions: [],
+    });
+
     expect(accessService.validateRole(existingRole)).rejects.toThrow(
         new NameExistsError(
             `There already exists a role with the name ${existingRole.name}`,
@@ -38,7 +36,7 @@ test('should fail when name exists', async () => {
 });
 
 test('should validate a role without permissions', async () => {
-    const { accessService } = getSetup(false);
+    const { accessService } = getSetup();
 
     const withoutPermissions: IRoleValidation = {
         name: 'name of the role',
@@ -50,7 +48,7 @@ test('should validate a role without permissions', async () => {
 });
 
 test('should complete description field when not present', async () => {
-    const { accessService } = getSetup(false);
+    const { accessService } = getSetup();
     const withoutDescription: IRoleValidation = {
         name: 'name of the role',
     };
@@ -61,7 +59,7 @@ test('should complete description field when not present', async () => {
 });
 
 test('should accept empty permissions', async () => {
-    const { accessService } = getSetup(false);
+    const { accessService } = getSetup();
     const withEmptyPermissions: IRoleValidation = {
         name: 'name of the role',
         description: 'description',
@@ -75,7 +73,7 @@ test('should accept empty permissions', async () => {
 });
 
 test('should complete environment field of permissions when not present', async () => {
-    const { accessService } = getSetup(false);
+    const { accessService } = getSetup();
     const withoutEnvironmentInPermissions: IRoleValidation = {
         name: 'name of the role',
         description: 'description',
@@ -100,7 +98,7 @@ test('should complete environment field of permissions when not present', async
 });
 
 test('should return the same object when all fields are valid and present', async () => {
-    const { accessService } = getSetup(false);
+    const { accessService } = getSetup();
 
     const roleWithAllFields: IRoleValidation = {
         name: 'name of the role',
@@ -125,7 +123,7 @@ test('should return the same object when all fields are valid and present', asyn
 });
 
 test('should be able to validate and cleanup with additional properties', async () => {
-    const { accessService } = getSetup(false);
+    const { accessService } = getSetup();
     const base = {
         name: 'name of the role',
         description: 'description',
@@ -152,3 +150,22 @@ test('should be able to validate and cleanup with additional properties', async
         ],
     });
 });
+
+test('user with custom root role should get a user root role', async () => {
+    const { accessService } = getSetup(true);
+    const customRootRole = await accessService.createRole({
+        name: 'custom-root-role',
+        description: 'test custom root role',
+        type: CUSTOM_ROOT_ROLE_TYPE,
+        permissions: [],
+    });
+    const user = {
+        id: 1,
+        rootRole: customRootRole.id,
+    };
+    await accessService.setUserRootRole(user.id, customRootRole.id);
+
+    const roles = await accessService.getUserRootRoles(user.id);
+    expect(roles).toHaveLength(1);
+    expect(roles[0].name).toBe('custom-root-role');
+});

src/lib/services/access-service.ts

@@ -18,7 +18,6 @@ import {
     IRoleData,
     IUserWithRole,
     RoleName,
-    RoleType,
 } from '../types/model';
 import { IRoleStore } from 'lib/types/stores/role-store';
 import NameExistsError from '../error/name-exists-error';
@@ -30,6 +29,7 @@ import {
     ALL_PROJECTS,
     CUSTOM_ROOT_ROLE_TYPE,
     CUSTOM_PROJECT_ROLE_TYPE,
+    ROOT_ROLE_TYPES,
 } from '../util/constants';
 import { DEFAULT_PROJECT } from '../types/project';
 import InvalidOperationError from '../error/invalid-operation-error';
@@ -253,10 +253,10 @@ export class AccessService {
         const newRootRole = await this.resolveRootRole(role);
         if (newRootRole) {
             try {
-                await this.store.removeRolesOfTypeForUser(userId, [
-                    RoleType.ROOT,
-                    RoleType.ROOT_CUSTOM,
-                ]);
+                await this.store.removeRolesOfTypeForUser(
+                    userId,
+                    ROOT_ROLE_TYPES,
+                );
 
                 await this.store.addUserToRole(
                     userId,
@@ -275,7 +275,7 @@ export class AccessService {
 
     async getUserRootRoles(userId: number): Promise<IRoleWithProject[]> {
         const userRoles = await this.store.getRolesForUserId(userId);
-        return userRoles.filter((r) => r.type === RoleType.ROOT);
+        return userRoles.filter(({ type }) => ROOT_ROLE_TYPES.includes(type));
     }
 
     async removeUserFromRole(

src/lib/types/model.ts

@@ -386,11 +386,11 @@ export interface IProject {
     defaultStickiness: string;
 }
 
-export interface ICustomRole {
-    id: number;
-    name: string;
+/**
+ * Extends IRole making description mandatory
+ */
+export interface ICustomRole extends IRole {
     description: string;
-    type: string;
 }
 
 export interface IProjectWithCount extends IProject {

src/test/fixtures/fake-access-store.ts

@@ -8,8 +8,18 @@ import {
     IUserRole,
 } from '../../lib/types/stores/access-store';
 import { IPermission } from 'lib/types/model';
+import { IRoleStore } from 'lib/types';
+import FakeRoleStore from './fake-role-store';
 
 class AccessStoreMock implements IAccessStore {
+    fakeRolesStore: IRoleStore;
+
+    userToRoleMap: Map<number, number> = new Map();
+
+    constructor(roleStore?: IRoleStore) {
+        this.fakeRolesStore = roleStore ?? new FakeRoleStore();
+    }
+
     addAccessToProject(
         users: IAccessInfo[],
         groups: IAccessInfo[],
@@ -88,13 +98,9 @@ class AccessStoreMock implements IAccessStore {
         role_id: number,
         permissions: IPermission[],
     ): Promise<void> {
-        throw new Error('Method not implemented.');
+        return Promise.resolve(undefined);
     }
 
-    userPermissions: IUserPermission[] = [];
-
-    roles: IRole[] = [];
-
     getAvailablePermissions(): Promise<IPermission[]> {
         throw new Error('Method not implemented.');
     }
@@ -123,8 +129,17 @@ class AccessStoreMock implements IAccessStore {
         throw new Error('Method not implemented.');
     }
 
-    getRolesForUserId(userId: number): Promise<IRoleWithProject[]> {
-        return Promise.resolve([]);
+    async getRolesForUserId(userId: number): Promise<IRoleWithProject[]> {
+        const roleId = this.userToRoleMap.get(userId);
+        const found =
+            roleId === undefined
+                ? undefined
+                : await this.fakeRolesStore.get(roleId);
+        if (found) {
+            return Promise.resolve([found as IRoleWithProject]);
+        } else {
+            return Promise.resolve([]);
+        }
     }
 
     getUserIdsForRole(roleId: number, projectId: string): Promise<number[]> {
@@ -132,7 +147,8 @@ class AccessStoreMock implements IAccessStore {
     }
 
     addUserToRole(userId: number, roleId: number): Promise<void> {
-        throw new Error('Method not implemented.');
+        this.userToRoleMap.set(userId, roleId);
+        return Promise.resolve(undefined);
     }
 
     addPermissionsToRole(
@@ -140,7 +156,8 @@ class AccessStoreMock implements IAccessStore {
         permissions: string[],
         projectId?: string,
     ): Promise<void> {
-        throw new Error('Method not implemented.');
+        // do nothing for now
+        return Promise.resolve(undefined);
     }
 
     removePermissionFromRole(

src/test/fixtures/fake-role-store.ts

@@ -19,7 +19,9 @@ export default class FakeRoleStore implements IRoleStore {
     }
 
     nameInUse(name: string, existingId?: number): Promise<boolean> {
-        throw new Error('Method not implemented.');
+        return Promise.resolve(
+            this.roles.find((r) => r.name === name) !== undefined,
+        );
     }
 
     async getAll(): Promise<ICustomRole[]> {
@@ -29,7 +31,7 @@ export default class FakeRoleStore implements IRoleStore {
     async create(role: ICustomRoleInsert): Promise<ICustomRole> {
         const roleCreated = {
             ...role,
-            type: 'some-type',
+            type: role.roleType,
             id: this.roles.length,
         };
         this.roles.push(roleCreated);
@@ -49,7 +51,7 @@ export default class FakeRoleStore implements IRoleStore {
     }
 
     async getRoleByName(name: string): Promise<IRole> {
-        return this.roles.find((r) => (r.name = name));
+        return this.roles.find((r) => (r.name = name)) as IRole;
     }
 
     getRolesForProject(projectId: string): Promise<IRole[]> {
@@ -72,8 +74,13 @@ export default class FakeRoleStore implements IRoleStore {
         throw new Error('Method not implemented.');
     }
 
-    get(key: number): Promise<ICustomRole> {
-        throw new Error('Method not implemented.');
+    get(id: number): Promise<ICustomRole> {
+        const found = this.roles.find((r) => r.id === id);
+        if (!found) {
+            // this edge case is not properly contemplated in the type definition
+            throw new Error('Not found');
+        }
+        return Promise.resolve(found);
     }
 
     exists(key: number): Promise<boolean> {