mirror of
https://github.com/Unleash/unleash.git
synced 2025-01-31 00:16:47 +01:00
feat: allow editing root role/description on SCIM group (#7874)
This commit is contained in:
parent
745c69bd75
commit
b6db704699
@ -106,6 +106,7 @@ export const CreateGroup = () => {
|
|||||||
handleSubmit={handleSubmit}
|
handleSubmit={handleSubmit}
|
||||||
handleCancel={handleCancel}
|
handleCancel={handleCancel}
|
||||||
mode={CREATE}
|
mode={CREATE}
|
||||||
|
isScimGroup={false}
|
||||||
>
|
>
|
||||||
<Button
|
<Button
|
||||||
type='submit'
|
type='submit'
|
||||||
|
@ -149,6 +149,7 @@ export const EditGroup = ({
|
|||||||
handleSubmit={handleSubmit}
|
handleSubmit={handleSubmit}
|
||||||
handleCancel={handleCancel}
|
handleCancel={handleCancel}
|
||||||
mode={EDIT}
|
mode={EDIT}
|
||||||
|
isScimGroup={isScimGroup}
|
||||||
>
|
>
|
||||||
<Tooltip title={isScimGroup ? scimGroupTooltip : ''} arrow>
|
<Tooltip title={isScimGroup ? scimGroupTooltip : ''} arrow>
|
||||||
<div>
|
<div>
|
||||||
@ -156,7 +157,7 @@ export const EditGroup = ({
|
|||||||
type='submit'
|
type='submit'
|
||||||
variant='contained'
|
variant='contained'
|
||||||
color='primary'
|
color='primary'
|
||||||
disabled={isScimGroup || !isValid}
|
disabled={!isValid}
|
||||||
data-testid={UG_SAVE_BTN_ID}
|
data-testid={UG_SAVE_BTN_ID}
|
||||||
>
|
>
|
||||||
Save
|
Save
|
||||||
|
@ -261,7 +261,6 @@ export const Group: VFC = () => {
|
|||||||
? scimGroupTooltip
|
? scimGroupTooltip
|
||||||
: 'Edit group',
|
: 'Edit group',
|
||||||
}}
|
}}
|
||||||
disabled={isScimGroup}
|
|
||||||
>
|
>
|
||||||
<Edit />
|
<Edit />
|
||||||
</PermissionIconButton>
|
</PermissionIconButton>
|
||||||
|
@ -81,6 +81,7 @@ interface IGroupForm {
|
|||||||
mappingsSSO: string[];
|
mappingsSSO: string[];
|
||||||
users: IGroupUser[];
|
users: IGroupUser[];
|
||||||
rootRole: number | null;
|
rootRole: number | null;
|
||||||
|
isScimGroup: boolean;
|
||||||
setName: (name: string) => void;
|
setName: (name: string) => void;
|
||||||
setDescription: React.Dispatch<React.SetStateAction<string>>;
|
setDescription: React.Dispatch<React.SetStateAction<string>>;
|
||||||
setMappingsSSO: React.Dispatch<React.SetStateAction<string[]>>;
|
setMappingsSSO: React.Dispatch<React.SetStateAction<string[]>>;
|
||||||
@ -99,6 +100,7 @@ export const GroupForm: FC<IGroupForm> = ({
|
|||||||
mappingsSSO,
|
mappingsSSO,
|
||||||
users,
|
users,
|
||||||
rootRole,
|
rootRole,
|
||||||
|
isScimGroup,
|
||||||
setName,
|
setName,
|
||||||
setDescription,
|
setDescription,
|
||||||
setMappingsSSO,
|
setMappingsSSO,
|
||||||
@ -138,6 +140,7 @@ export const GroupForm: FC<IGroupForm> = ({
|
|||||||
onChange={(e) => setName(e.target.value)}
|
onChange={(e) => setName(e.target.value)}
|
||||||
data-testid={UG_NAME_ID}
|
data-testid={UG_NAME_ID}
|
||||||
required
|
required
|
||||||
|
disabled={isScimGroup}
|
||||||
/>
|
/>
|
||||||
<StyledInputDescription>
|
<StyledInputDescription>
|
||||||
How would you describe your group?
|
How would you describe your group?
|
||||||
@ -152,7 +155,7 @@ export const GroupForm: FC<IGroupForm> = ({
|
|||||||
data-testid={UG_DESC_ID}
|
data-testid={UG_DESC_ID}
|
||||||
/>
|
/>
|
||||||
<ConditionallyRender
|
<ConditionallyRender
|
||||||
condition={isGroupSyncingEnabled}
|
condition={isGroupSyncingEnabled && !isScimGroup}
|
||||||
show={
|
show={
|
||||||
<>
|
<>
|
||||||
<StyledInputDescription>
|
<StyledInputDescription>
|
||||||
|
@ -74,7 +74,6 @@ export const GroupCardActions: FC<IGroupCardActions> = ({
|
|||||||
aria-expanded={open ? 'true' : undefined}
|
aria-expanded={open ? 'true' : undefined}
|
||||||
onClick={handleClick}
|
onClick={handleClick}
|
||||||
type='button'
|
type='button'
|
||||||
disabled={isScimGroup}
|
|
||||||
>
|
>
|
||||||
<MoreVert />
|
<MoreVert />
|
||||||
</IconButton>
|
</IconButton>
|
||||||
@ -103,6 +102,7 @@ export const GroupCardActions: FC<IGroupCardActions> = ({
|
|||||||
</ListItemText>
|
</ListItemText>
|
||||||
</MenuItem>
|
</MenuItem>
|
||||||
<MenuItem
|
<MenuItem
|
||||||
|
disabled={isScimGroup}
|
||||||
onClick={() => {
|
onClick={() => {
|
||||||
onEditUsers();
|
onEditUsers();
|
||||||
handleClose();
|
handleClose();
|
||||||
@ -122,6 +122,7 @@ export const GroupCardActions: FC<IGroupCardActions> = ({
|
|||||||
onRemove();
|
onRemove();
|
||||||
handleClose();
|
handleClose();
|
||||||
}}
|
}}
|
||||||
|
disabled={isScimGroup}
|
||||||
>
|
>
|
||||||
<ListItemIcon>
|
<ListItemIcon>
|
||||||
<Delete />
|
<Delete />
|
||||||
|
@ -1,2 +1,2 @@
|
|||||||
export const scimGroupTooltip =
|
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 type EventService from '../features/events/event-service';
|
||||||
import { SSO_SYNC_USER } from '../db/group-store';
|
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 {
|
export class GroupService {
|
||||||
private groupStore: IGroupStore;
|
private groupStore: IGroupStore;
|
||||||
|
|
||||||
@ -231,6 +235,28 @@ export class GroupService {
|
|||||||
throw new NameExistsError('Group name already exists');
|
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[]> {
|
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',
|
'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