fix: handle invalid permissions in role creation (#11003)

The `create` and `update` role methods used to blindly accept any
incoming permissions, but if the permissions don't exist in the
database, then the database would throw, yielding a 500 error to the
user.

To fix this, we can validate that all the permissions exist before we
try to add the incoming permissions.

The http error only manifests in enterprise, but the fix requires
modifying the access service. Therefore, I've added the tests to the
access service too, such that if you break something, then you don't
need to wait for it to propagate to enterprise.

---------

Co-authored-by: Gastón Fournier <gaston@getunleash.io>

src/lib/features/access/createAccessService.ts

@@ -10,7 +10,9 @@ import FakeEventStore from '../../../test/fixtures/fake-event-store.js';
 import { FakeAccountStore } from '../../../test/fixtures/fake-account-store.js';
 import FakeRoleStore from '../../../test/fixtures/fake-role-store.js';
 import FakeEnvironmentStore from '../project-environments/fake-environment-store.js';
-import FakeAccessStore from '../../../test/fixtures/fake-access-store.js';
+import FakeAccessStore, {
+    type FakeAccessStoreConfig,
+} from '../../../test/fixtures/fake-access-store.js';
 import type {
     IAccessStore,
     IEventStore,
@@ -45,8 +47,13 @@ export const createAccessService = (
     );
 };
 
+export type FakeAccessServiceConfig = {
+    accessStoreConfig?: FakeAccessStoreConfig;
+};
+
 export const createFakeAccessService = (
     config: IUnleashConfig,
+    { accessStoreConfig }: FakeAccessServiceConfig = {},
 ): {
     accessService: AccessService;
     eventStore: IEventStore;
@@ -59,7 +66,7 @@ export const createFakeAccessService = (
     const accountStore = new FakeAccountStore();
     const roleStore = new FakeRoleStore();
     const environmentStore = new FakeEnvironmentStore();
-    const accessStore = new FakeAccessStore(roleStore);
+    const accessStore = new FakeAccessStore(roleStore, accessStoreConfig);
     const eventService = createFakeEventsService(config, { eventStore });
     const groupService = new GroupService(
         { groupStore, accountStore },

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

@@ -1,6 +1,9 @@
 import NameExistsError from '../error/name-exists-error.js';
 import getLogger from '../../test/fixtures/no-logger.js';
-import { createFakeAccessService } from '../features/access/createAccessService.js';
+import {
+    createFakeAccessService,
+    type FakeAccessServiceConfig,
+} from '../features/access/createAccessService.js';
 import {
     AccessService,
     type IRoleCreation,
@@ -29,13 +32,15 @@ import { createFakeAccessReadModel } from '../features/access/createAccessReadMo
 import { ROLE_CREATED } from '../events/index.js';
 import { expect } from 'vitest';
 
-function getSetup() {
+function getSetup(accessServiceConfig?: FakeAccessServiceConfig) {
     const config = createTestConfig({
         getLogger,
     });
 
-    const { accessService, eventStore, accessStore } =
-        createFakeAccessService(config);
+    const { accessService, eventStore, accessStore } = createFakeAccessService(
+        config,
+        accessServiceConfig,
+    );
 
     return {
         accessService,
@@ -213,7 +218,24 @@ 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, eventStore } = getSetup();
+    const availablePermissions = [
+        {
+            id: 1,
+            environment: 'development',
+            name: 'fake',
+            displayName: 'fake',
+            type: '',
+        },
+        {
+            id: 2,
+            name: 'root-fake-permission',
+            displayName: '',
+            type: '',
+        },
+    ];
+    const { accessService, eventStore } = getSetup({
+        accessStoreConfig: { availablePermissions },
+    });
     const createRoleInput: IRoleCreation = {
         name: 'custom-root-role',
         description: 'test custom root role',

src/lib/services/access-service.ts

@@ -707,6 +707,8 @@ export class AccessService {
             roleType,
         };
 
+        await this.validatePermissions(role.permissions);
+
         const rolePermissions = cleanPermissionEnvironment(role.permissions);
         const newRole = await this.roleStore.create(baseRole);
         if (rolePermissions) {
@@ -756,6 +758,8 @@ export class AccessService {
             description: role.description,
             roleType,
         };
+
+        await this.validatePermissions(role.permissions);
         const rolePermissions = cleanPermissionEnvironment(role.permissions);
         const updatedRole = await this.roleStore.update(baseRole);
         const existingPermissions = await this.store.getPermissionsForRole(
@@ -878,4 +882,33 @@ export class AccessService {
     async getUserAccessOverview(): Promise<IUserAccessOverview[]> {
         return this.store.getUserAccessOverview();
     }
+
+    async validatePermissions(permissions?: PermissionRef[]): Promise<void> {
+        if (!permissions?.length) {
+            return;
+        }
+        const availablePermissions = await this.store.getAvailablePermissions();
+        const invalidPermissions = permissions.filter(
+            (permission) =>
+                !availablePermissions.some((availablePermission) =>
+                    'id' in permission
+                        ? availablePermission.id === permission.id
+                        : availablePermission.name === permission.name,
+                ),
+        );
+
+        if (invalidPermissions.length > 0) {
+            const invalidPermissionList = invalidPermissions
+                .map((permission) =>
+                    'id' in permission
+                        ? `permission with ID: ${permission.id}`
+                        : permission.name,
+                )
+                .join(', ');
+
+            throw new BadDataError(
+                `Invalid permissions supplied. The following permissions don't exist: ${invalidPermissionList}.`,
+            );
+        }
+    }
 }

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

@@ -1848,3 +1848,56 @@ test('access overview should include users with custom root roles', async () =>
     expect(userAccess.userId).toBe(user.id);
     expect(userAccess.rootRole).toBe('Mischievous Messenger');
 });
+
+test("creating a role with permissions that don't exist should throw a bad data error", async () => {
+    await expect(() =>
+        accessService.createRole(
+            {
+                name: 'Oogus Boogus',
+                type: CUSTOM_ROOT_ROLE_TYPE,
+                description:
+                    "Well, well, well ... what have we here? Sandy Claws, huh? Oooh, I'm really scared!",
+                permissions: [{ name: 'BOGUS' }],
+                createdByUserId: 1,
+            },
+            SYSTEM_USER_AUDIT,
+        ),
+    ).rejects.toThrow(
+        expect.objectContaining({
+            name: 'BadDataError',
+            message: expect.stringMatching(/BOGUS/),
+        }),
+    );
+});
+
+test("Updating a role with permissions that don't exist should throw a bad data error", async () => {
+    const custom_role = await accessService.createRole(
+        {
+            name: 'Legit custom role',
+            type: CUSTOM_ROOT_ROLE_TYPE,
+            description: '',
+            permissions: [{ name: permissions.CREATE_ADDON }],
+            createdByUserId: 1,
+        },
+        SYSTEM_USER_AUDIT,
+    );
+    await expect(() =>
+        accessService.updateRole(
+            {
+                id: custom_role.id,
+                name: 'Oogus Boogus',
+                type: CUSTOM_ROOT_ROLE_TYPE,
+                description:
+                    'This might be the last time that you hear the Boogus song',
+                permissions: [{ name: 'BOGUS' }],
+                createdByUserId: 1,
+            },
+            SYSTEM_USER_AUDIT,
+        ),
+    ).rejects.toThrow(
+        expect.objectContaining({
+            name: 'BadDataError',
+            message: expect.stringMatching(/BOGUS/),
+        }),
+    );
+});

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

@@ -18,6 +18,10 @@ import {
 import FakeRoleStore from './fake-role-store.js';
 import type { PermissionRef } from '../../lib/services/access-service.js';
 
+export type FakeAccessStoreConfig = Partial<{
+    availablePermissions: IPermission[];
+}>;
+
 export class FakeAccessStore implements IAccessStore {
     fakeRolesStore: IRoleStore;
 
@@ -25,8 +29,11 @@ export class FakeAccessStore implements IAccessStore {
 
     rolePermissions: Map<number, IPermission[]> = new Map();
 
-    constructor(roleStore?: IRoleStore) {
+    availablePermissions: IPermission[] = [];
+
+    constructor(roleStore?: IRoleStore, config?: FakeAccessStoreConfig) {
         this.fakeRolesStore = roleStore ?? new FakeRoleStore();
+        this.availablePermissions = config?.availablePermissions ?? [];
     }
 
     getProjectUserAndGroupCountsForRole(
@@ -112,7 +119,7 @@ export class FakeAccessStore implements IAccessStore {
     }
 
     getAvailablePermissions(): Promise<IPermission[]> {
-        throw new Error('Method not implemented.');
+        return Promise.resolve(this.availablePermissions);
     }
 
     getPermissionsForUser(userId: Number): Promise<IUserPermission[]> {