mirror of
https://github.com/Unleash/unleash.git
synced 2025-10-27 11:02:16 +01:00
feat: allow editing root role/description on SCIM group (#7874)
This commit is contained in:
Simon Hornby
GitHub
parent
5e82e47d94
commit
f276728688
@ -106,6 +106,7 @@ export const CreateGroup = () => {
|
||||
handleSubmit={handleSubmit}
|
||||
handleCancel={handleCancel}
|
||||
mode={CREATE}
|
||||
isScimGroup={false}
|
||||
>
|
||||
<Button
|
||||
type='submit'
|
||||
|
||||
@ -149,6 +149,7 @@ export const EditGroup = ({
|
||||
handleSubmit={handleSubmit}
|
||||
handleCancel={handleCancel}
|
||||
mode={EDIT}
|
||||
isScimGroup={isScimGroup}
|
||||
>
|
||||
<Tooltip title={isScimGroup ? scimGroupTooltip : ''} arrow>
|
||||
<div>
|
||||
@ -156,7 +157,7 @@ export const EditGroup = ({
|
||||
type='submit'
|
||||
variant='contained'
|
||||
color='primary'
|
||||
disabled={isScimGroup || !isValid}
|
||||
disabled={!isValid}
|
||||
data-testid={UG_SAVE_BTN_ID}
|
||||
>
|
||||
Save
|
||||
|
||||
@ -261,7 +261,6 @@ export const Group: VFC = () => {
|
||||
? scimGroupTooltip
|
||||
: 'Edit group',
|
||||
}}
|
||||
disabled={isScimGroup}
|
||||
>
|
||||
<Edit />
|
||||
</PermissionIconButton>
|
||||
|
||||
@ -81,6 +81,7 @@ interface IGroupForm {
|
||||
mappingsSSO: string[];
|
||||
users: IGroupUser[];
|
||||
rootRole: number | null;
|
||||
isScimGroup: boolean;
|
||||
setName: (name: string) => void;
|
||||
setDescription: React.Dispatch<React.SetStateAction<string>>;
|
||||
setMappingsSSO: React.Dispatch<React.SetStateAction<string[]>>;
|
||||
@ -99,6 +100,7 @@ export const GroupForm: FC<IGroupForm> = ({
|
||||
mappingsSSO,
|
||||
users,
|
||||
rootRole,
|
||||
isScimGroup,
|
||||
setName,
|
||||
setDescription,
|
||||
setMappingsSSO,
|
||||
@ -138,6 +140,7 @@ export const GroupForm: FC<IGroupForm> = ({
|
||||
onChange={(e) => setName(e.target.value)}
|
||||
data-testid={UG_NAME_ID}
|
||||
required
|
||||
disabled={isScimGroup}
|
||||
/>
|
||||
<StyledInputDescription>
|
||||
How would you describe your group?
|
||||
@ -152,7 +155,7 @@ export const GroupForm: FC<IGroupForm> = ({
|
||||
data-testid={UG_DESC_ID}
|
||||
/>
|
||||
<ConditionallyRender
|
||||
condition={isGroupSyncingEnabled}
|
||||
condition={isGroupSyncingEnabled && !isScimGroup}
|
||||
show={
|
||||
<>
|
||||
<StyledInputDescription>
|
||||
|
||||
@ -74,7 +74,6 @@ export const GroupCardActions: FC<IGroupCardActions> = ({
|
||||
aria-expanded={open ? 'true' : undefined}
|
||||
onClick={handleClick}
|
||||
type='button'
|
||||
disabled={isScimGroup}
|
||||
>
|
||||
<MoreVert />
|
||||
</IconButton>
|
||||
@ -103,6 +102,7 @@ export const GroupCardActions: FC<IGroupCardActions> = ({
|
||||
</ListItemText>
|
||||
</MenuItem>
|
||||
<MenuItem
|
||||
disabled={isScimGroup}
|
||||
onClick={() => {
|
||||
onEditUsers();
|
||||
handleClose();
|
||||
@ -122,6 +122,7 @@ export const GroupCardActions: FC<IGroupCardActions> = ({
|
||||
onRemove();
|
||||
handleClose();
|
||||
}}
|
||||
disabled={isScimGroup}
|
||||
>
|
||||
<ListItemIcon>
|
||||
<Delete />
|
||||
|
||||
@ -1,2 +1,2 @@
|
||||
export const scimGroupTooltip =
|
||||
'This group is managed by your SCIM provider and cannot be changed manually';
|
||||
'This group is managed by your SCIM provider, only the role and description can be changed manually';
|
||||
|
||||
@ -30,6 +30,10 @@ import type { IUser } from '../types/user';
|
||||
import type EventService from '../features/events/event-service';
|
||||
import { SSO_SYNC_USER } from '../db/group-store';
|
||||
|
||||
const setsAreEqual = (firstSet, secondSet) =>
|
||||
firstSet.size === secondSet.size &&
|
||||
[...firstSet].every((x) => secondSet.has(x));
|
||||
|
||||
export class GroupService {
|
||||
private groupStore: IGroupStore;
|
||||
|
||||
@ -231,6 +235,28 @@ export class GroupService {
|
||||
throw new NameExistsError('Group name already exists');
|
||||
}
|
||||
}
|
||||
|
||||
if (existingGroup && Boolean(existingGroup.scimId)) {
|
||||
if (existingGroup.name !== group.name) {
|
||||
throw new BadDataError(
|
||||
'Cannot update the name of a SCIM group',
|
||||
);
|
||||
}
|
||||
|
||||
const existingUsers = new Set(
|
||||
(
|
||||
await this.groupStore.getAllUsersByGroups([
|
||||
existingGroup.id,
|
||||
])
|
||||
).map((g) => g.userId),
|
||||
);
|
||||
|
||||
const newUsers = new Set(group.users?.map((g) => g.user.id) || []);
|
||||
|
||||
if (!setsAreEqual(existingUsers, newUsers)) {
|
||||
throw new BadDataError('Cannot update users of a SCIM group');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async getRolesForProject(projectId: string): Promise<IGroupRole[]> {
|
||||
|
||||
@ -262,3 +262,101 @@ test('adding a nonexistent role to a group should fail', async () => {
|
||||
'Request validation failed: your request body or params contain invalid data: Incorrect role id 100',
|
||||
);
|
||||
});
|
||||
|
||||
test('trying to modify a SCIM group name should fail', async () => {
|
||||
const group = await groupStore.create({
|
||||
name: 'scim_group',
|
||||
description: 'scim_group',
|
||||
});
|
||||
|
||||
await db.rawDatabase('groups').where({ id: group.id }).update({
|
||||
scim_id: 'some-faux-uuid',
|
||||
scim_external_id: 'external-id',
|
||||
});
|
||||
|
||||
await expect(() => {
|
||||
return groupService.updateGroup(
|
||||
{
|
||||
id: group.id,
|
||||
name: 'new_name',
|
||||
users: [],
|
||||
rootRole: 100,
|
||||
createdAt: new Date(),
|
||||
createdBy: 'test',
|
||||
},
|
||||
TEST_AUDIT_USER,
|
||||
);
|
||||
}).rejects.toThrow(
|
||||
'Request validation failed: your request body or params contain invalid data: Cannot update the name of a SCIM group',
|
||||
);
|
||||
});
|
||||
|
||||
test('trying to modify users in a SCIM group should fail', async () => {
|
||||
const group = await groupStore.create({
|
||||
name: 'another_scim_group',
|
||||
description: 'scim_group',
|
||||
});
|
||||
|
||||
await db.rawDatabase('groups').where({ id: group.id }).update({
|
||||
scim_id: 'some-other-faux-uuid',
|
||||
scim_external_id: 'some-external-id',
|
||||
});
|
||||
|
||||
await expect(() => {
|
||||
return groupService.updateGroup(
|
||||
{
|
||||
id: group.id,
|
||||
name: 'new_name',
|
||||
users: [
|
||||
{
|
||||
user: {
|
||||
id: 1,
|
||||
isAPI: false,
|
||||
permissions: [],
|
||||
},
|
||||
},
|
||||
],
|
||||
rootRole: 100,
|
||||
createdAt: new Date(),
|
||||
createdBy: 'test',
|
||||
},
|
||||
TEST_AUDIT_USER,
|
||||
);
|
||||
}).rejects.toThrow(
|
||||
'Request validation failed: your request body or params contain invalid data: Cannot update the name of a SCIM group',
|
||||
);
|
||||
});
|
||||
|
||||
test('can modify a SCIM group description and root role', async () => {
|
||||
const group = await groupStore.create({
|
||||
name: 'yet_another_scim_group',
|
||||
description: 'scim_group',
|
||||
});
|
||||
|
||||
await db.rawDatabase('groups').where({ id: group.id }).update({
|
||||
scim_id: 'completely-different-faux-uuid',
|
||||
scim_external_id: 'totally-not-the-same-external-id',
|
||||
});
|
||||
|
||||
await groupService.updateGroup(
|
||||
{
|
||||
id: group.id,
|
||||
name: 'yet_another_scim_group',
|
||||
description: 'new description',
|
||||
users: [],
|
||||
rootRole: 1,
|
||||
createdAt: new Date(),
|
||||
createdBy: 'test',
|
||||
},
|
||||
TEST_AUDIT_USER,
|
||||
);
|
||||
|
||||
const updatedGroup = await groupService.getGroup(group.id);
|
||||
expect(updatedGroup).toMatchObject({
|
||||
description: 'new description',
|
||||
id: group.id,
|
||||
mappingsSSO: [],
|
||||
name: 'yet_another_scim_group',
|
||||
rootRole: 1,
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user