diff --git a/src/lib/features/access/createAccessService.ts b/src/lib/features/access/createAccessService.ts index de08bb29fd..ccf745b029 100644 --- a/src/lib/features/access/createAccessService.ts +++ b/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 }, diff --git a/src/lib/services/access-service.test.ts b/src/lib/services/access-service.test.ts index 71af9591d6..41eb9469c0 100644 --- a/src/lib/services/access-service.test.ts +++ b/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', diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts index 81b730c81c..e7f934d2cd 100644 --- a/src/lib/services/access-service.ts +++ b/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 { return this.store.getUserAccessOverview(); } + + async validatePermissions(permissions?: PermissionRef[]): Promise { + 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}.`, + ); + } + } } diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 51bf8c87c9..ed988aefc2 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/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/), + }), + ); +}); diff --git a/src/test/fixtures/fake-access-store.ts b/src/test/fixtures/fake-access-store.ts index 3dcb4f0be6..20067c15a4 100644 --- a/src/test/fixtures/fake-access-store.ts +++ b/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 = 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 { - throw new Error('Method not implemented.'); + return Promise.resolve(this.availablePermissions); } getPermissionsForUser(userId: Number): Promise {