diff --git a/frontend/cypress/integration/projects/access.spec.ts b/frontend/cypress/integration/projects/access.spec.ts index d3f26a433f..68898e7ceb 100644 --- a/frontend/cypress/integration/projects/access.spec.ts +++ b/frontend/cypress/integration/projects/access.spec.ts @@ -47,19 +47,6 @@ describe('project-access', () => { id: groupAndProjectName, name: groupAndProjectName, }); - - cy.intercept('GET', `${baseUrl}/api/admin/ui-config`, req => { - req.headers['cache-control'] = - 'no-cache, no-store, must-revalidate'; - req.on('response', res => { - if (res.body) { - res.body.flags = { - ...res.body.flags, - multipleRoles: true, - }; - } - }); - }); }); after(() => { @@ -78,6 +65,20 @@ describe('project-access', () => { beforeEach(() => { cy.login_UI(); + + cy.intercept('GET', `${baseUrl}/api/admin/ui-config`, req => { + req.headers['cache-control'] = + 'no-cache, no-store, must-revalidate'; + req.on('response', res => { + if (res.body) { + res.body.flags = { + ...res.body.flags, + multipleRoles: true, + }; + } + }); + }); + cy.visit(`/projects/${groupAndProjectName}/settings/access`); if (document.querySelector("[data-testid='CLOSE_SPLASH']")) { cy.get("[data-testid='CLOSE_SPLASH']").click(); diff --git a/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx b/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx index 94a350ccfc..a30a9174b8 100644 --- a/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx +++ b/frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx @@ -151,11 +151,13 @@ export const ProjectAccessTable: VFC = () => { id: 'role', Header: 'Role', accessor: (row: IProjectAccess) => - row.entity.roles.length > 1 - ? `${row.entity.roles.length} roles` - : access?.roles.find( - ({ id }) => id === row.entity.roleId - )?.name, + row.entity.roles + ? row.entity.roles.length > 1 + ? `${row.entity.roles.length} roles` + : access?.roles.find( + ({ id }) => id === row.entity.roleId + )?.name + : 'No Roles!', Cell: ({ value, row: { original: row }, diff --git a/src/lib/db/group-store.ts b/src/lib/db/group-store.ts index f8ae1091bd..055a7c2a35 100644 --- a/src/lib/db/group-store.ts +++ b/src/lib/db/group-store.ts @@ -1,9 +1,9 @@ import { IGroupStore, IStoreGroup } from '../types/stores/group-store'; import NotFoundError from '../error/notfound-error'; import Group, { - ICreateGroupModel, ICreateGroupUserModel, IGroup, + IGroupModel, IGroupProject, IGroupRole, IGroupUser, @@ -82,7 +82,7 @@ export default class GroupStore implements IGroupStore { return groups.map(rowToGroup); } - async update(group: ICreateGroupModel): Promise { + async update(group: IGroupModel): Promise { try { const rows = await this.db(T.GROUPS) .where({ id: group.id }) diff --git a/src/lib/services/access-service.test.ts b/src/lib/services/access-service.test.ts index 5d633bf380..6a4b72997e 100644 --- a/src/lib/services/access-service.test.ts +++ b/src/lib/services/access-service.test.ts @@ -190,7 +190,7 @@ test('throws error when trying to delete a project role in use by group', async const eventStore = new FakeEventStore(); const groupStore = new FakeGroupStore(); groupStore.getAllWithId = async (): Promise => { - return [{ name: 'group' }]; + return [{ id: 1, name: 'group' }]; }; const accountStore = new FakeAccountStore(); const roleStore = new FakeRoleStore(); diff --git a/src/lib/services/group-service.ts b/src/lib/services/group-service.ts index d079819e8d..589a5707ba 100644 --- a/src/lib/services/group-service.ts +++ b/src/lib/services/group-service.ts @@ -105,10 +105,7 @@ export class GroupService { return newGroup; } - async updateGroup( - group: ICreateGroupModel, - userName: string, - ): Promise { + async updateGroup(group: IGroupModel, userName: string): Promise { const preData = await this.groupStore.get(group.id); await this.validateGroup(group, preData); @@ -153,10 +150,10 @@ export class GroupService { if (projectGroups.length > 0) { const groups = await this.groupStore.getAllWithId( - projectGroups.map((g) => g.id!), + projectGroups.map((g) => g.id), ); const groupUsers = await this.groupStore.getAllUsersByGroups( - groups.map((g) => g.id!), + groups.map((g) => g.id), ); const users = await this.accountStore.getAllWithId( groupUsers.map((u) => u.userId), @@ -178,7 +175,7 @@ export class GroupService { } async validateGroup( - group: ICreateGroupModel, + group: IGroupModel | ICreateGroupModel, existingGroup?: IGroup, ): Promise { if (!group.name) { @@ -190,16 +187,6 @@ export class GroupService { throw new NameExistsError('Group name already exists'); } } - - if ( - group.id && - group.rootRole && - (await this.groupStore.hasProjectRole(group.id)) - ) { - throw new BadDataError( - 'This group already has a project role and cannot also be given a root role', - ); - } } async getRolesForProject(projectId: string): Promise { diff --git a/src/lib/types/group.ts b/src/lib/types/group.ts index e2092ee3c6..ab3b384350 100644 --- a/src/lib/types/group.ts +++ b/src/lib/types/group.ts @@ -2,7 +2,7 @@ import Joi, { ValidationError } from 'joi'; import { IUser } from './user'; export interface IGroup { - id?: number; + id: number; name: string; description?: string; mappingsSSO?: string[]; @@ -33,7 +33,7 @@ export interface IGroupModel extends IGroup { projects?: string[]; } -export interface ICreateGroupModel extends IGroup { +export interface ICreateGroupModel extends Omit { users?: ICreateGroupUserModel[]; projects?: string[]; } diff --git a/src/lib/types/stores/group-store.ts b/src/lib/types/stores/group-store.ts index e4cbdfa2f9..f7974e0e2e 100644 --- a/src/lib/types/stores/group-store.ts +++ b/src/lib/types/stores/group-store.ts @@ -1,8 +1,8 @@ import { Store } from './store'; import Group, { - ICreateGroupModel, ICreateGroupUserModel, IGroup, + IGroupModel, IGroupProject, IGroupRole, IGroupUser, @@ -48,7 +48,7 @@ export interface IGroupStore extends Store { deleteUsersFromGroup(deletableUsers: IGroupUser[]): Promise; - update(group: ICreateGroupModel): Promise; + update(group: IGroupModel): Promise; getAllUsersByGroups(groupIds: number[]): Promise; diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 3959f27e66..a959b82d73 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -63,7 +63,7 @@ const createGroup = async ({ name: `Group ${groupIndex}`, rootRole: role, }); - if (users) await groupStore.addUsersToGroup(group.id!, users, 'Admin'); + if (users) await groupStore.addUsersToGroup(group.id, users, 'Admin'); return group; }; @@ -1292,7 +1292,7 @@ test('if group has two roles user has union of permissions from the two roles', await accessService.setProjectRolesForGroup( projectName, - emptyGroup.id!, + emptyGroup.id, [firstRole.id, secondRole.id], 'testusr', ); @@ -1347,7 +1347,7 @@ test('calling set for group overwrites existing roles', async () => { await accessService.setProjectRolesForGroup( projectName, - emptyGroup.id!, + emptyGroup.id, [firstRole.id, secondRole.id], 'testusr', ); @@ -1363,7 +1363,7 @@ test('calling set for group overwrites existing roles', async () => { await accessService.setProjectRolesForGroup( projectName, - emptyGroup.id!, + emptyGroup.id, [firstRole.id], 'testusr', ); @@ -1404,7 +1404,7 @@ test('group with root role can be assigned a project specific role', async () => await accessService.setProjectRolesForGroup( projectName, - emptyGroup.id!, + emptyGroup.id, [firstRole.id], 'testusr', ); @@ -1638,7 +1638,7 @@ test('calling set roles for group with invalid project role ids should not assig accessService.setProjectRolesForGroup( projectName, - emptyGroup.id!, + emptyGroup.id, [adminRootRole.id, 9999], 'admin', ); @@ -1669,7 +1669,7 @@ test('calling set roles for group with empty role array removes all roles', asyn await accessService.setProjectRolesForGroup( projectName, - emptyGroup.id!, + emptyGroup.id, [role.id], 'admin', ); @@ -1682,7 +1682,7 @@ test('calling set roles for group with empty role array removes all roles', asyn await accessService.setProjectRolesForGroup( projectName, - emptyGroup.id!, + emptyGroup.id, [], 'admin', ); @@ -1719,7 +1719,7 @@ test('calling set roles for group with empty role array should not remove root r await accessService.setProjectRolesForGroup( projectName, - group.id!, + group.id, [role.id], 'admin', ); @@ -1732,7 +1732,7 @@ test('calling set roles for group with empty role array should not remove root r await accessService.setProjectRolesForGroup( projectName, - group.id!, + group.id, [], 'admin', ); @@ -1778,7 +1778,7 @@ test('remove group access should remove all project roles', async () => { await accessService.setProjectRolesForGroup( projectName, - group.id!, + group.id, [firstRole.id, secondRole.id], 'admin', ); @@ -1789,7 +1789,7 @@ test('remove group access should remove all project roles', async () => { expect(assignedPermissions.length).toBe(3); - await accessService.removeGroupAccess(projectName, group.id!); + await accessService.removeGroupAccess(projectName, group.id); const newAssignedPermissions = await accessService.getPermissionsForUser( emptyUser, @@ -1831,7 +1831,7 @@ test('remove group access should remove all project roles, while leaving root ro await accessService.setProjectRolesForGroup( projectName, - group.id!, + group.id, [firstRole.id, secondRole.id], 'admin', ); @@ -1842,7 +1842,7 @@ test('remove group access should remove all project roles, while leaving root ro expect(assignedPermissions.length).toBe(4); - await accessService.removeGroupAccess(projectName, group.id!); + await accessService.removeGroupAccess(projectName, group.id); const newAssignedPermissions = await accessService.getPermissionsForUser( adminUser, diff --git a/src/test/e2e/services/group-service.e2e.test.ts b/src/test/e2e/services/group-service.e2e.test.ts index 74e6a1884b..e81c4358df 100644 --- a/src/test/e2e/services/group-service.e2e.test.ts +++ b/src/test/e2e/services/group-service.e2e.test.ts @@ -98,7 +98,7 @@ test('should not remove user from no SSO definition group', async () => { expect(groups[0].name).toEqual('no_mapping_group'); }); -test('adding a root role to a group with a project role should fail', async () => { +test('adding a root role to a group with a project role should not fail', async () => { const group = await groupStore.create({ name: 'root_group', description: 'root_group', @@ -118,9 +118,7 @@ test('adding a root role to a group with a project role should fail', async () = }, 'test', ); - }).rejects.toThrow( - 'This group already has a project role and cannot also be given a root role', - ); + }).not.toThrow(); expect.assertions(1); }); diff --git a/src/test/fixtures/fake-group-store.ts b/src/test/fixtures/fake-group-store.ts index c44d472e49..10a6ab3427 100644 --- a/src/test/fixtures/fake-group-store.ts +++ b/src/test/fixtures/fake-group-store.ts @@ -1,8 +1,8 @@ import { IGroupStore, IStoreGroup } from '../../lib/types/stores/group-store'; import Group, { - ICreateGroupModel, ICreateGroupUserModel, IGroup, + IGroupModel, IGroupProject, IGroupRole, IGroupUser, @@ -63,7 +63,7 @@ export default class FakeGroupStore implements IGroupStore { throw new Error('Method not implemented.'); } - update(group: ICreateGroupModel): Promise { + update(group: IGroupModel): Promise { throw new Error('Method not implemented.'); }