1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-02-19 00:15:43 +01:00

feat: allow SCIM user deletion (#9190)

Co-authored-by: Gastón Fournier <gaston@getunleash.io>
This commit is contained in:
Simon Hornby 2025-02-10 14:17:46 +02:00 committed by GitHub
parent cdeb515488
commit bf9fdd4f8d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 308 additions and 15 deletions

View File

@ -0,0 +1,36 @@
import { Alert, styled, Typography } from '@mui/material';
import { Dialogue } from 'component/common/Dialogue/Dialogue';
const StyledAlert = styled(Alert)(({ theme }) => ({
marginBottom: theme.spacing(3),
}));
export type EntityType = 'Users' | 'Groups';
interface IScimDeleteUsersProps {
open: boolean;
entityType: EntityType;
closeDialog: () => void;
deleteEntities: () => void;
}
export const ScimDeleteEntityDialog = ({
open,
closeDialog,
deleteEntities: removeUser,
entityType,
}: IScimDeleteUsersProps) => (
<Dialogue
open={open}
primaryButtonText={`Delete SCIM ${entityType}`}
secondaryButtonText='Cancel'
title={`Do you really want to delete ALL SCIM ${entityType}?`}
onClose={closeDialog}
onClick={removeUser}
>
<Typography variant='body1'>
This will delete all {entityType.toLocaleLowerCase()} created or
managed by SCIM.
</Typography>
</Dialogue>
);

View File

@ -9,6 +9,9 @@ import { formatUnknownError } from 'utils/formatUnknownError';
import useToast from 'hooks/useToast';
import { useScimSettingsApi } from 'hooks/api/actions/useScimSettingsApi/useScimSettingsApi';
import { useScimSettings } from 'hooks/api/getters/useScimSettings/useScimSettings';
import { ScimDeleteEntityDialog } from './ScimDeleteUsersDialog';
import useAdminUsersApi from 'hooks/api/actions/useAdminUsersApi/useAdminUsersApi';
import { useGroupApi } from 'hooks/api/actions/useGroupApi/useGroupApi';
const StyledContainer = styled('div')(({ theme }) => ({
padding: theme.spacing(3),
@ -25,8 +28,12 @@ export const ScimSettings = () => {
const { setToastData, setToastApiError } = useToast();
const [newToken, setNewToken] = useState('');
const [tokenGenerationDialog, setTokenGenerationDialog] = useState(false);
const [deleteGroupsDialog, setDeleteGroupsDialog] = useState(false);
const [deleteUsersDialog, setDeleteUsersDialog] = useState(false);
const [tokenDialog, setTokenDialog] = useState(false);
const { settings, refetch } = useScimSettings();
const { deleteScimUsers } = useAdminUsersApi();
const { deleteScimGroups } = useGroupApi();
const [enabled, setEnabled] = useState(settings.enabled ?? true);
useEffect(() => {
@ -40,6 +47,34 @@ export const ScimSettings = () => {
setTokenGenerationDialog(true);
};
const onDeleteScimGroups = async () => {
try {
await deleteScimGroups();
setToastData({
text: 'Scim Groups have been deleted',
type: 'success',
});
setDeleteGroupsDialog(false);
refetch();
} catch (error: unknown) {
setToastApiError(formatUnknownError(error));
}
};
const onDeleteScimUsers = async () => {
try {
await deleteScimUsers();
setToastData({
text: 'Scim Users have been deleted',
type: 'success',
});
setDeleteUsersDialog(false);
refetch();
} catch (error: unknown) {
setToastApiError(formatUnknownError(error));
}
};
const onGenerateNewTokenConfirm = async () => {
setTokenGenerationDialog(false);
const token = await generateNewToken();
@ -138,6 +173,57 @@ export const ScimSettings = () => {
/>
</Grid>
</Grid>
<Grid container spacing={3}>
<Grid item md={10.5} mb={2}>
<StyledTitleDiv>
<strong>Delete SCIM Users</strong>
</StyledTitleDiv>
<p>
This will remove all SCIM users from the Unleash
database. This action cannot be undone through
Unleash but the upstream SCIM provider may re sync
these users.
</p>
</Grid>
<Grid item md={1.5}>
<Button
variant='outlined'
color='error'
disabled={loading}
onClick={() => {
setDeleteUsersDialog(true);
}}
>
Delete Users
</Button>
</Grid>
<Grid item md={10.5} mb={2}>
<StyledTitleDiv>
<strong>Delete SCIM Groups</strong>
</StyledTitleDiv>
<p>
This will remove all SCIM groups from the Unleash
database. This action cannot be undone through
Unleash but the upstream SCIM provider may re sync
these groups. Note that this may affect the
permissions of users present in those groups.
</p>
</Grid>
<Grid item md={1.5}>
<Button
variant='outlined'
color='error'
disabled={loading}
onClick={() => {
setDeleteGroupsDialog(true);
}}
>
Delete Groups
</Button>
</Grid>
</Grid>
<ScimTokenGenerationDialog
open={tokenGenerationDialog}
setOpen={setTokenGenerationDialog}
@ -148,6 +234,20 @@ export const ScimSettings = () => {
setOpen={setTokenDialog}
token={newToken}
/>
<ScimDeleteEntityDialog
open={deleteUsersDialog}
closeDialog={() => setDeleteUsersDialog(false)}
deleteEntities={onDeleteScimUsers}
entityType='Users'
/>
<ScimDeleteEntityDialog
open={deleteGroupsDialog}
closeDialog={() => setDeleteGroupsDialog(false)}
deleteEntities={onDeleteScimGroups}
entityType='Groups'
/>
</StyledContainer>
</>
);

View File

@ -270,11 +270,8 @@ export const Group: VFC = () => {
onClick={() => setRemoveOpen(true)}
permission={ADMIN}
tooltipProps={{
title: isScimGroup
? scimGroupTooltip
: 'Delete group',
title: 'Delete group',
}}
disabled={isScimGroup}
>
<Delete />
</PermissionIconButton>

View File

@ -125,7 +125,6 @@ export const GroupCardActions: FC<IGroupCardActions> = ({
onRemove();
handleClose();
}}
disabled={isScimGroup}
>
<ListItemIcon>
<Delete />

View File

@ -77,6 +77,9 @@ const DeleteUser = ({
})`
: ''}
?
{user.scimId
? ' This user is currently managed by SCIM and may be re-added by your SCIM provider.'
: ''}
</Typography>
</div>
</Dialogue>

View File

@ -91,9 +91,8 @@ export const UsersActionsCell: VFC<IUsersActionsCellProps> = ({
onClick={onDelete}
permission={ADMIN}
tooltipProps={{
title: isScimUser ? scimTooltip : 'Remove user',
title: 'Remove user',
}}
disabled={isScimUser}
>
<Delete />
</PermissionIconButton>

View File

@ -94,6 +94,19 @@ const useAdminUsersApi = () => {
return makeRequest(req.caller, req.id);
};
const deleteScimUsers = async () => {
const requestId = 'deleteScimUsers';
const req = createRequest(
'api/admin/user-admin/scim-users',
{
method: 'DELETE',
},
requestId,
);
return makeRequest(req.caller, req.id);
};
return {
addUser,
updateUser,
@ -101,6 +114,7 @@ const useAdminUsersApi = () => {
changePassword,
validatePassword,
resetPassword,
deleteScimUsers,
userApiErrors: errors,
userLoading: loading,
};

View File

@ -46,10 +46,20 @@ export const useGroupApi = () => {
await makeRequest(req.caller, req.id);
};
const deleteScimGroups = async () => {
const path = `api/admin/groups/scim-groups`;
const req = createRequest(path, {
method: 'DELETE',
});
await makeRequest(req.caller, req.id);
};
return {
createGroup,
updateGroup,
removeGroup,
deleteScimGroups,
errors,
loading,
};

View File

@ -354,4 +354,8 @@ export default class GroupStore implements IGroupStore {
const { present } = result.rows[0];
return present;
}
async deleteScimGroups(): Promise<void> {
await this.db(T.GROUPS).whereNotNull('scim_id').del();
}
}

View File

@ -274,6 +274,10 @@ class UserStore implements IUserStore {
await this.activeUsers().del();
}
async deleteScimUsers(): Promise<void> {
await this.db(TABLE).whereNotNull('scim_id').del();
}
async count(): Promise<number> {
return this.activeUsers()
.count('*')

View File

@ -411,6 +411,26 @@ export default class UserAdminController extends Controller {
],
});
this.route({
method: 'delete',
path: '/scim-users',
acceptAnyContentType: true,
handler: this.deleteScimUsers,
permission: ADMIN,
middleware: [
openApiService.validPath({
tags: ['Users'],
operationId: 'deleteScimUsers',
summary: 'Delete all SCIM users',
description: 'Deletes all users managed by SCIM',
responses: {
200: emptyResponse,
...getStandardResponses(401, 403),
},
}),
],
});
this.route({
method: 'delete',
path: '/:id',
@ -654,8 +674,6 @@ export default class UserAdminController extends Controller {
const { user, params } = req;
const { id } = params;
await this.throwIfScimUser({ id });
await this.userService.deleteUser(+id, req.audit);
res.status(200).send();
}
@ -766,4 +784,10 @@ export default class UserAdminController extends Controller {
Boolean((await this.userService.getUser(id)).scimId)
);
}
async deleteScimUsers(req: IAuthRequest, res: Response): Promise<void> {
const { audit } = req;
await this.userService.deleteScimUsers(audit);
res.status(200).send();
}
}

View File

@ -21,6 +21,7 @@ import {
GROUP_CREATED,
GroupUserAdded,
GroupUserRemoved,
ScimGroupsDeleted,
type IBaseEvent,
} from '../types/events';
import NameExistsError from '../error/name-exists-error';
@ -310,6 +311,16 @@ export class GroupService {
}
}
async deleteScimGroups(auditUser: IAuditUser): Promise<void> {
await this.groupStore.deleteScimGroups();
await this.eventService.storeEvent(
new ScimGroupsDeleted({
data: null,
auditUser,
}),
);
}
private mapGroupWithUsers(
group: IGroup,
allGroupUsers: IGroupUser[],

View File

@ -24,6 +24,7 @@ import type SessionService from './session-service';
import type { IUnleashStores } from '../types/stores';
import PasswordUndefinedError from '../error/password-undefined';
import {
ScimUsersDeleted,
UserCreatedEvent,
UserDeletedEvent,
UserUpdatedEvent,
@ -401,6 +402,17 @@ class UserService {
);
}
async deleteScimUsers(auditUser: IAuditUser): Promise<void> {
await this.store.deleteScimUsers();
await this.eventService.storeEvent(
new ScimUsersDeleted({
data: null,
auditUser,
}),
);
}
async loginUser(
usernameOrEmail: string,
password: string,

View File

@ -217,6 +217,8 @@ export const RELEASE_PLAN_MILESTONE_STARTED =
'release-plan-milestone-started' as const;
export const USER_PREFERENCE_UPDATED = 'user-preference-updated' as const;
export const SCIM_USERS_DELETED = 'scim-users-deleted' as const;
export const SCIM_GROUPS_DELETED = 'scim-groups-deleted' as const;
export const IEventTypes = [
APPLICATION_CREATED,
@ -372,6 +374,8 @@ export const IEventTypes = [
RELEASE_PLAN_REMOVED,
RELEASE_PLAN_MILESTONE_STARTED,
USER_PREFERENCE_UPDATED,
SCIM_USERS_DELETED,
SCIM_GROUPS_DELETED,
] as const;
export type IEventType = (typeof IEventTypes)[number];
@ -1608,6 +1612,30 @@ export class UserDeletedEvent extends BaseEvent {
}
}
export class ScimUsersDeleted extends BaseEvent {
readonly data: any;
constructor(eventData: {
data: any;
auditUser: IAuditUser;
}) {
super(SCIM_USERS_DELETED, eventData.auditUser);
this.data = eventData.data;
}
}
export class ScimGroupsDeleted extends BaseEvent {
readonly data: any;
constructor(eventData: {
data: any;
auditUser: IAuditUser;
}) {
super(SCIM_GROUPS_DELETED, eventData.auditUser);
this.data = eventData.data;
}
}
export class TagTypeCreatedEvent extends BaseEvent {
readonly data: any;

View File

@ -66,4 +66,6 @@ export interface IGroupStore extends Store<IGroup, number> {
create(group: IStoreGroup): Promise<IGroup>;
count(): Promise<number>;
deleteScimGroups(): Promise<void>;
}

View File

@ -40,4 +40,5 @@ export interface IUserStore extends Store<IUser, number> {
count(): Promise<number>;
countRecentlyDeleted(): Promise<number>;
countServiceAccounts(): Promise<number>;
deleteScimUsers(): Promise<void>;
}

View File

@ -462,3 +462,32 @@ test('should return number of sessions per user', async () => {
]),
});
});
test('should only delete scim users', async () => {
userStore.insert({
email: 'boring@example.com',
});
await userStore.insert({
email: 'really-boring@example.com',
});
const scimUser = (
await db
.rawDatabase('users')
.insert({
email: 'made-by-scim@example.com',
scim_id: 'some-random-scim-id',
})
.returning('id')
)[0].id;
await app.request.delete('/api/admin/user-admin/scim-users').expect(200);
const response = await app.request.get(`/api/admin/user-admin`).expect(200);
const users = response.body.users;
expect(users.length).toBe(2);
expect(users.every((u) => u.email !== 'made-by-scim@example.com')).toBe(
true,
);
});

View File

@ -12,6 +12,7 @@ let app: IUnleashTest;
let scimUserId: number;
let regularUserId: number;
let scimDeletableUser: number;
const scimUser = {
email: 'scim-user@test.com',
@ -24,6 +25,12 @@ const regularUser = {
name: 'Regular User',
};
const scimUserToBeDeleted = {
email: 'scim-victim@test.com',
name: 'SCIM Victim',
scim_id: 'some-other-random-scim-id',
};
const scimGuardErrorMessage =
'This user is managed by your SCIM provider and cannot be changed manually';
@ -50,6 +57,13 @@ beforeAll(async () => {
regularUserId = (
await db.rawDatabase('users').insert(regularUser).returning('id')
)[0].id;
scimDeletableUser = (
await db
.rawDatabase('users')
.insert(scimUserToBeDeleted)
.returning('id')
)[0].id;
});
afterAll(async () => {
@ -86,12 +100,10 @@ test('should prevent editing a SCIM user', async () => {
expect(body.details[0].message).toBe(scimGuardErrorMessage);
});
test('should prevent deleting a SCIM user', async () => {
const { body } = await app.request
.delete(`/api/admin/user-admin/${scimUserId}`)
.expect(403);
expect(body.details[0].message).toBe(scimGuardErrorMessage);
test('should not prevent deleting a SCIM user', async () => {
await app.request
.delete(`/api/admin/user-admin/${scimDeletableUser}`)
.expect(200);
});
test('should prevent changing password for a SCIM user', async () => {

View File

@ -125,4 +125,8 @@ export default class FakeGroupStore implements IGroupStore {
hasProjectRole(groupId: number): Promise<boolean> {
throw new Error('Method not implemented.');
}
deleteScimGroups(): Promise<void> {
throw new Error('Method not implemented.');
}
}

View File

@ -159,6 +159,10 @@ class UserStoreMock implements IUserStore {
return Promise.resolve(undefined);
}
deleteScimUsers(): Promise<void> {
throw new Error('Method not implemented.');
}
upsert(user: ICreateUser): Promise<IUser> {
this.data.splice(this.data.findIndex((u) => u.email === user.email));
const userToReturn = {