Fix/project role permission grant (#8084) (#8132)

Cherry pick the fix merged in #8084

Co-authored-by: Fredrik Strand Oseberg <[email protected]>

.gitignore

@@ -3,6 +3,7 @@ lerna-debug
 npm-debug
 .DS_Store
 /dist
+.vscode
 
 # Logs
 logs

src/lib/features/project/can-grant-project-role.test.ts

@@ -0,0 +1,121 @@
+import { canGrantProjectRole } from './can-grant-project-role';
+
+describe('canGrantProjectRole', () => {
+    test('should return true if the granter has all the permissions the receiver needs', () => {
+        const granterPermissions = [
+            {
+                project: 'test',
+                environment: undefined,
+                permission: 'CREATE_FEATURE',
+            },
+            {
+                project: 'test',
+                environment: 'production',
+                permission: 'UPDATE_FEATURE_ENVIRONMENT',
+            },
+            {
+                project: 'test',
+                environment: 'production',
+                permission: 'APPROVE_CHANGE_REQUEST',
+            },
+        ];
+
+        const receiverPermissions = [
+            {
+                id: 28,
+                name: 'UPDATE_FEATURE_ENVIRONMENT',
+                environment: 'production',
+                displayName: 'Enable/disable flags',
+                type: 'environment',
+            },
+            {
+                id: 29,
+                name: 'APPROVE_CHANGE_REQUEST',
+                environment: 'production',
+                displayName: 'Enable/disable flags',
+                type: 'environment',
+            },
+        ];
+
+        canGrantProjectRole(granterPermissions, receiverPermissions);
+    });
+
+    test('should return false if the granter and receiver permissions have different environments', () => {
+        const granterPermissions = [
+            {
+                project: 'test',
+                environment: 'production',
+                permission: 'UPDATE_FEATURE_ENVIRONMENT',
+            },
+            {
+                project: 'test',
+                environment: 'production',
+                permission: 'APPROVE_CHANGE_REQUEST',
+            },
+        ];
+
+        const receiverPermissions = [
+            {
+                id: 28,
+                name: 'UPDATE_FEATURE_ENVIRONMENT',
+                environment: 'development',
+                displayName: 'Enable/disable flags',
+                type: 'environment',
+            },
+            {
+                id: 29,
+                name: 'APPROVE_CHANGE_REQUEST',
+                environment: 'development',
+                displayName: 'Enable/disable flags',
+                type: 'environment',
+            },
+        ];
+
+        expect(
+            canGrantProjectRole(granterPermissions, receiverPermissions),
+        ).toBeFalsy();
+    });
+
+    test('should return false if the granter does not have all receiver permissions', () => {
+        const granterPermissions = [
+            {
+                project: 'test',
+                environment: 'production',
+                permission: 'UPDATE_FEATURE_ENVIRONMENT',
+            },
+            {
+                project: 'test',
+                environment: 'production',
+                permission: 'APPROVE_CHANGE_REQUEST',
+            },
+        ];
+
+        const receiverPermissions = [
+            {
+                id: 28,
+                name: 'UPDATE_FEATURE_ENVIRONMENT',
+                environment: 'production',
+                displayName: 'Enable/disable flags',
+                type: 'environment',
+            },
+            {
+                id: 29,
+                name: 'APPROVE_CHANGE_REQUEST',
+                environment: 'production',
+                displayName: 'Enable/disable flags',
+                type: 'environment',
+            },
+            {
+                id: 26,
+                name: 'UPDATE_FEATURE_STRATEGY',
+                environment: 'production',
+                displayName: 'Update activation strategies',
+                type: 'environment',
+            },
+        ];
+
+        expect(
+            canGrantProjectRole(granterPermissions, receiverPermissions),
+        ).toBeFalsy();
+    });
+});

src/lib/features/project/can-grant-project-role.ts

@@ -0,0 +1,21 @@
+import type { IPermission } from '../../types';
+import type { IUserPermission } from '../../types/stores/access-store';
+
+export const canGrantProjectRole = (
+    granterPermissions: IUserPermission[],
+    receiverPermissions: IPermission[],
+) => {
+    return receiverPermissions.every((receiverPermission) => {
+        return granterPermissions.some((granterPermission) => {
+            if (granterPermission.environment) {
+                return (
+                    granterPermission.permission === receiverPermission.name &&
+                    granterPermission.environment ===
+                        receiverPermission.environment
+                );
+            }
+
+            return granterPermission.permission === receiverPermission.name;
+        });
+    });
+};

src/lib/features/project/project-service.e2e.test.ts

@@ -772,72 +772,92 @@ describe('Managing Project access', () => {
             ),
         );
     });
-    test('Users can not assign roles they do not have to a user through explicit roles endpoint', async () => {
+
+    test('Users can not assign roles where they do not hold the same permissions', async () => {
         const project = {
             id: 'user_fail_assign_to_user',
             name: 'user_fail_assign_to_user',
             description: '',
             mode: 'open' as const,
             defaultStickiness: 'clientId',
         };
+
+        const auditUser = extractAuditInfoFromUser(user);
         await projectService.createProject(project, user, auditUser);
         const projectUser = await stores.userStore.insert({
             name: 'Some project user',
             email: '[email protected]',
         });
-        const projectAuditUser = extractAuditInfoFromUser(projectUser);
         const secondUser = await stores.userStore.insert({
             name: 'Some other user',
             email: '[email protected]',
         });
-        const customRole = await stores.roleStore.create({
-            name: 'role_that_noone_has',
-            roleType: 'custom',
-            description:
-                'Used to prove that you can not assign a role you do not have via setRolesForUser',
-        });
+
+        const customRoleUserAccess = await accessService.createRole(
+            {
+                name: 'Project-permissions-lead',
+                description: 'Role',
+                permissions: [
+                    {
+                        name: 'PROJECT_USER_ACCESS_WRITE',
+                    },
+                ],
+                createdByUserId: SYSTEM_USER_ID,
+            },
+            SYSTEM_USER_AUDIT,
+        );
+
+        const customRoleUpdateEnvironments = await accessService.createRole(
+            {
+                name: 'Project Lead',
+                description: 'Role',
+                permissions: [
+                    {
+                        name: 'UPDATE_FEATURE_ENVIRONMENT',
+                        environment: 'production',
+                    },
+                    {
+                        name: 'CREATE_FEATURE_STRATEGY',
+                        environment: 'production',
+                    },
+                ],
+                createdByUserId: SYSTEM_USER_ID,
+            },
+            SYSTEM_USER_AUDIT,
+        );
+
+        await projectService.setRolesForUser(
+            project.id,
+            projectUser.id,
+            [customRoleUserAccess.id],
+            auditUser,
+        );
+
+        const auditProjectUser = extractAuditInfoFromUser(projectUser);
+
         await expect(
             projectService.setRolesForUser(
                 project.id,
                 secondUser.id,
-                [customRole.id],
-                projectAuditUser,
+                [customRoleUpdateEnvironments.id],
+                auditProjectUser,
             ),
         ).rejects.toThrow(
             new InvalidOperationError(
                 'User tried to assign a role they did not have access to',
             ),
         );
-    });
-    test('Users can not assign roles they do not have to a group through explicit roles endpoint', async () => {
-        const project = {
-            id: 'user_fail_assign_to_group',
-            name: 'user_fail_assign_to_group',
-            description: '',
-            mode: 'open' as const,
-            defaultStickiness: 'clientId',
-        };
-        await projectService.createProject(project, user, auditUser);
-        const projectUser = await stores.userStore.insert({
-            name: 'Some project user',
-            email: '[email protected]',
-        });
-        const projectAuditUser = extractAuditInfoFromUser(projectUser);
+
         const group = await stores.groupStore.create({
             name: 'Some group_awaiting_role',
         });
-        const customRole = await stores.roleStore.create({
-            name: 'role_that_noone_has_fail_assign_group',
-            roleType: 'custom',
-            description:
-                'Used to prove that you can not assign a role you do not have via setRolesForGroup',
-        });
-        return expect(
+
+        await expect(
             projectService.setRolesForGroup(
                 project.id,
                 group.id,
-                [customRole.id],
-                projectAuditUser,
+                [customRoleUpdateEnvironments.id],
+                auditProjectUser,
             ),
         ).rejects.toThrow(
             new InvalidOperationError(

src/lib/features/project/project-service.ts

@@ -89,6 +89,7 @@ import { throwExceedsLimitError } from '../../error/exceeds-limit-error';
 import type EventEmitter from 'events';
 import type { ApiTokenService } from '../../services/api-token-service';
 import type { TransitionalProjectData } from './project-read-model-type';
+import { canGrantProjectRole } from './can-grant-project-role';
 
 type Days = number;
 type Count = number;
@@ -940,15 +941,38 @@ export default class ProjectService {
             userAddingAccess.id,
             projectId,
         );
+
         if (
             this.isAdmin(userAddingAccess.id, userRoles) ||
             this.isProjectOwner(userRoles, projectId)
         ) {
             return true;
         }
-        return rolesBeingAdded.every((roleId) =>
-            userRoles.some((userRole) => userRole.id === roleId),
-        );
+
+        // Users may have access to multiple projects, so we need to filter out the permissions based on this project.
+        // Since the project roles are just collections of permissions that are not tied to a project in the database
+        // not filtering here might lead to false positives as they may have the permission in another project.
+        if (this.flagResolver.isEnabled('projectRoleAssignment')) {
+            const filteredUserPermissions = userPermissions.filter(
+                (permission) => permission.project === projectId,
+            );
+
+            const rolesToBeAssignedData = await Promise.all(
+                rolesBeingAdded.map((role) => this.accessService.getRole(role)),
+            );
+            const rolesToBeAssignedPermissions = rolesToBeAssignedData.flatMap(
+                (role) => role.permissions,
+            );
+
+            return canGrantProjectRole(
+                filteredUserPermissions,
+                rolesToBeAssignedPermissions,
+            );
+        } else {
+            return rolesBeingAdded.every((roleId) =>
+                userRoles.some((userRole) => userRole.id === roleId),
+            );
+        }
     }
 
     async addAccess(

src/lib/routes/admin-api/user/user.ts

@@ -13,7 +13,10 @@ import { createRequestSchema } from '../../../openapi/util/create-request-schema
 import { createResponseSchema } from '../../../openapi/util/create-response-schema';
 import { meSchema, type MeSchema } from '../../../openapi/spec/me-schema';
 import { serializeDates } from '../../../types/serialize-dates';
-import type { IUserPermission } from '../../../types/stores/access-store';
+import type {
+    IRole,
+    IUserPermission,
+} from '../../../types/stores/access-store';
 import type { PasswordSchema } from '../../../openapi/spec/password-schema';
 import {
     emptyResponse,
@@ -28,6 +31,7 @@ import {
     rolesSchema,
     type RolesSchema,
 } from '../../../openapi/spec/roles-schema';
+import type { IFlagResolver } from '../../../types';
 
 class UserController extends Controller {
     private accessService: AccessService;
@@ -42,6 +46,8 @@ class UserController extends Controller {
 
     private projectService: ProjectService;
 
+    private flagResolver: IFlagResolver;
+
     constructor(
         config: IUnleashConfig,
         {
@@ -68,6 +74,7 @@ class UserController extends Controller {
         this.userSplashService = userSplashService;
         this.openApiService = openApiService;
         this.projectService = projectService;
+        this.flagResolver = config.flagResolver;
 
         this.route({
             method: 'get',
@@ -174,10 +181,15 @@ class UserController extends Controller {
     ): Promise<void> {
         const { projectId } = req.query;
         if (projectId) {
-            const roles = await this.accessService.getAllProjectRolesForUser(
-                req.user.id,
-                projectId,
-            );
+            let roles: IRole[];
+            if (this.flagResolver.isEnabled('projectRoleAssignment')) {
+                roles = await this.accessService.getProjectRoles();
+            } else {
+                roles = await this.accessService.getAllProjectRolesForUser(
+                    req.user.id,
+                    projectId,
+                );
+            }
             this.openApiService.respondWithValidation(
                 200,
                 res,