diff --git a/src/lib/db/access-store.test.ts b/src/lib/db/access-store.test.ts index 6298c2bac36..8f4c625c5cd 100644 --- a/src/lib/db/access-store.test.ts +++ b/src/lib/db/access-store.test.ts @@ -2,6 +2,7 @@ import dbInit from '../../test/e2e/helpers/database-init'; import getLogger from '../../test/fixtures/no-logger'; import { PermissionRef } from 'lib/services/access-service'; import { AccessStore } from './access-store'; +import { BadDataError } from '../../lib/error'; let db; @@ -104,3 +105,29 @@ test.each([ expect(result).toStrictEqual(expected); }, ); + +describe('addAccessToProject', () => { + test.each(['roles', 'groups', 'users'])( + 'should throw a bad data error if there is invalid data in the %s property of the addAccessToProject payload', + async (property) => { + const access = db.stores.accessStore as AccessStore; + + const payload = { + roles: [4, 5], + groups: [], + users: [], + [property]: [123456789], + }; + + await expect(() => + access.addAccessToProject( + payload.roles, + payload.groups, + payload.users, + 'projectId', + 'createdBy', + ), + ).rejects.toThrow(BadDataError); + }, + ); +}); diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts index a49c420cb9f..5c9a6d13ad9 100644 --- a/src/lib/db/access-store.ts +++ b/src/lib/db/access-store.ts @@ -26,6 +26,7 @@ import { PermissionRef, } from 'lib/services/access-service'; import { inTransaction } from './transaction'; +import BadDataError from '../error/bad-data-error'; const T = { ROLE_USER: 'role_user', @@ -618,6 +619,18 @@ export class AccessStore implements IAccessStore { .whereIn('type', PROJECT_ROLE_TYPES) .pluck('id'); + if (validatedProjectRoleIds.length !== roles.length) { + const invalidRoles = roles.filter( + (role) => !validatedProjectRoleIds.includes(role), + ); + + throw new BadDataError( + `You can't add access to a project with roles that aren't project roles or that don't exist. These roles are not valid: ${invalidRoles.join( + ', ', + )}`, + ); + } + const groupRows = groups.flatMap((group) => validatedProjectRoleIds.map((role) => ({ group_id: group, @@ -636,17 +649,54 @@ export class AccessStore implements IAccessStore { ); await inTransaction(this.db, async (tx) => { + const errors: string[] = []; if (groupRows.length > 0) { await tx(T.GROUP_ROLE) .insert(groupRows) .onConflict(['project', 'role_id', 'group_id']) - .merge(); + .merge() + .catch((err) => { + if ( + err.message.includes( + `violates foreign key constraint "group_role_group_id_fkey"`, + ) + ) { + errors.push( + `Your request contains one or more group IDs that do not exist. You sent these group IDs: ${groups.join( + ', ', + )}.`, + ); + } + }); } if (userRows.length > 0) { await tx(T.ROLE_USER) .insert(userRows) .onConflict(['project', 'role_id', 'user_id']) - .merge(); + .merge() + .catch((err) => { + if ( + err.message.includes( + `violates foreign key constraint "role_user_user_id_fkey"`, + ) + ) { + errors.push( + `Your request contains one or more user IDs that do not exist. You sent these user IDs: ${users.join( + ', ', + )}.`, + ); + } + }); + } + + if (errors.length) { + const mapped = errors.map((message) => ({ + message, + })); + + // because TS doesn't understand that the non-empty + // array is guaranteed to have at least one element + throw new BadDataError('', [mapped[0], ...mapped.slice(1)]); } }); } diff --git a/src/lib/services/access-service.test.ts b/src/lib/services/access-service.test.ts index ddd0edcdbb0..fd0a544a680 100644 --- a/src/lib/services/access-service.test.ts +++ b/src/lib/services/access-service.test.ts @@ -19,6 +19,7 @@ import { IRole } from '../../lib/types/stores/access-store'; import { IGroup, ROLE_CREATED } from '../../lib/types'; import EventService from './event-service'; import FakeFeatureTagStore from '../../test/fixtures/fake-feature-tag-store'; +import BadDataError from '../../lib/error/bad-data-error'; function getSetup(customRootRolesKillSwitch: boolean = true) { const config = createTestConfig({ @@ -265,3 +266,18 @@ test('throws error when trying to delete a project role in use by group', async ); } }); + +describe('addAccessToProject', () => { + test('should throw an error when you try add access with an empty list of roles', async () => { + const { accessService } = getSetup(); + await expect(() => + accessService.addAccessToProject( + [], + [1], + [1], + 'projectId', + 'createdBy', + ), + ).rejects.toThrow(BadDataError); + }); +}); diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts index 1f80a5d4f01..10585a0719a 100644 --- a/src/lib/services/access-service.ts +++ b/src/lib/services/access-service.ts @@ -277,6 +277,11 @@ export class AccessService { projectId: string, createdBy: string, ): Promise { + if (roles.length === 0) { + throw new BadDataError( + "You can't grant access without any roles. The roles array you sent was empty.", + ); + } return this.store.addAccessToProject( roles, groups, diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 0a3ba025fd3..279cd5f817c 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -21,6 +21,7 @@ import { createFeatureToggleService, createProjectService, } from '../../../lib/features'; +import { BadDataError } from '../../../lib/error'; let db: ITestDb; let stores: IUnleashStores; @@ -1382,13 +1383,15 @@ test('calling add access with invalid project role ids should not assign those r const adminRootRole = await accessService.getRoleByName(RoleName.ADMIN); - accessService.addAccessToProject( - [adminRootRole.id, 9999], - [], - [emptyUser.id], - projectName, - 'some-admin-user', - ); + await expect(() => + accessService.addAccessToProject( + [adminRootRole.id, 9999], + [], + [emptyUser.id], + projectName, + 'some-admin-user', + ), + ).rejects.toThrow(BadDataError); const newAssignedPermissions = await accessService.getPermissionsForUser(emptyUser);