feat(server,web): add force delete to immediately remove user (#7681)

* feat(server,web): add force delete to immediately remove user

* update wording on force delete confirmation

* fix force delete css

* PR feedback

* cleanup user service delete for force

* adding user status column

* some cleanup and tests

* more test fixes

* run npm run sql:generate

* chore: cleanup and websocket

* chore: linting

* userRepository.restore

* removed bad color class from delete-confirm-dialoge

* additional confirmation for user force delete

* shorten confirmation message

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>
This commit is contained in:
Sam Holton
2024-03-08 17:49:39 -05:00
committed by GitHub
parent 9cb0a1ffbf
commit 7a4ae7d142
47 changed files with 628 additions and 97 deletions

View File

@@ -280,6 +280,11 @@ export class JobService {
}
break;
}
case JobName.USER_DELETION: {
this.communicationRepository.broadcast(ClientEvent.USER_DELETE, item.data.id);
break;
}
}
}
}

View File

@@ -4,6 +4,7 @@ export const ICommunicationRepository = 'ICommunicationRepository';
export enum ClientEvent {
UPLOAD_SUCCESS = 'on_upload_success',
USER_DELETE = 'on_user_delete',
ASSET_DELETE = 'on_asset_delete',
ASSET_TRASH = 'on_asset_trash',
ASSET_UPDATE = 'on_asset_update',
@@ -22,6 +23,7 @@ export enum ServerEvent {
export interface ClientEventMap {
[ClientEvent.UPLOAD_SUCCESS]: AssetResponseDto;
[ClientEvent.USER_DELETE]: string;
[ClientEvent.ASSET_DELETE]: string;
[ClientEvent.ASSET_TRASH]: string[];
[ClientEvent.ASSET_UPDATE]: AssetResponseDto;

View File

@@ -32,7 +32,6 @@ export interface IUserRepository {
create(user: Partial<UserEntity>): Promise<UserEntity>;
update(id: string, user: Partial<UserEntity>): Promise<UserEntity>;
delete(user: UserEntity, hard?: boolean): Promise<UserEntity>;
restore(user: UserEntity): Promise<UserEntity>;
updateUsage(id: string, delta: number): Promise<void>;
syncUsage(id?: string): Promise<void>;
}

View File

@@ -0,0 +1,6 @@
import { ValidateBoolean } from '../../domain.util';
export class DeleteUserDto {
@ValidateBoolean({ optional: true })
force?: boolean;
}

View File

@@ -1,3 +1,4 @@
export * from './create-profile-image.dto';
export * from './create-user.dto';
export * from './delete-user.dto';
export * from './update-user.dto';

View File

@@ -1,4 +1,4 @@
import { UserAvatarColor, UserEntity } from '@app/infra/entities';
import { UserAvatarColor, UserEntity, UserStatus } from '@app/infra/entities';
import { ApiProperty } from '@nestjs/swagger';
import { IsEnum } from 'class-validator';
@@ -33,6 +33,8 @@ export class UserResponseDto extends UserDto {
quotaSizeInBytes!: number | null;
@ApiProperty({ type: 'integer', format: 'int64' })
quotaUsageInBytes!: number | null;
@ApiProperty({ enumName: 'UserStatus', enum: UserStatus })
status!: string;
}
export const mapSimpleUser = (entity: UserEntity): UserDto => {
@@ -58,5 +60,6 @@ export function mapUser(entity: UserEntity): UserResponseDto {
memoriesEnabled: entity.memoriesEnabled,
quotaSizeInBytes: entity.quotaSizeInBytes,
quotaUsageInBytes: entity.quotaUsageInBytes,
status: entity.status,
};
}

View File

@@ -1,4 +1,4 @@
import { UserEntity } from '@app/infra/entities';
import { UserEntity, UserStatus } from '@app/infra/entities';
import {
BadRequestException,
ForbiddenException,
@@ -243,16 +243,14 @@ describe(UserService.name, () => {
it('should throw error if user could not be found', async () => {
when(userMock.get).calledWith(userStub.admin.id, { withDeleted: true }).mockResolvedValue(null);
await expect(sut.restore(authStub.admin, userStub.admin.id)).rejects.toThrowError(BadRequestException);
expect(userMock.restore).not.toHaveBeenCalled();
expect(userMock.update).not.toHaveBeenCalled();
});
it('should restore an user', async () => {
userMock.get.mockResolvedValue(userStub.user1);
userMock.restore.mockResolvedValue(userStub.user1);
userMock.update.mockResolvedValue(userStub.user1);
await expect(sut.restore(authStub.admin, userStub.user1.id)).resolves.toEqual(mapUser(userStub.user1));
expect(userMock.get).toHaveBeenCalledWith(userStub.user1.id, { withDeleted: true });
expect(userMock.restore).toHaveBeenCalledWith(userStub.user1);
expect(userMock.update).toHaveBeenCalledWith(userStub.user1.id, { status: UserStatus.ACTIVE, deletedAt: null });
});
});
@@ -260,27 +258,47 @@ describe(UserService.name, () => {
it('should throw error if user could not be found', async () => {
userMock.get.mockResolvedValue(null);
await expect(sut.delete(authStub.admin, userStub.admin.id)).rejects.toThrowError(BadRequestException);
await expect(sut.delete(authStub.admin, userStub.admin.id, {})).rejects.toThrowError(BadRequestException);
expect(userMock.delete).not.toHaveBeenCalled();
});
it('cannot delete admin user', async () => {
await expect(sut.delete(authStub.admin, userStub.admin.id)).rejects.toBeInstanceOf(ForbiddenException);
await expect(sut.delete(authStub.admin, userStub.admin.id, {})).rejects.toBeInstanceOf(ForbiddenException);
});
it('should require the auth user be an admin', async () => {
await expect(sut.delete(authStub.user1, authStub.admin.user.id)).rejects.toBeInstanceOf(ForbiddenException);
await expect(sut.delete(authStub.user1, authStub.admin.user.id, {})).rejects.toBeInstanceOf(ForbiddenException);
expect(userMock.delete).not.toHaveBeenCalled();
});
it('should delete user', async () => {
userMock.get.mockResolvedValue(userStub.user1);
userMock.delete.mockResolvedValue(userStub.user1);
userMock.update.mockResolvedValue(userStub.user1);
await expect(sut.delete(authStub.admin, userStub.user1.id)).resolves.toEqual(mapUser(userStub.user1));
expect(userMock.get).toHaveBeenCalledWith(userStub.user1.id, {});
expect(userMock.delete).toHaveBeenCalledWith(userStub.user1);
await expect(sut.delete(authStub.admin, userStub.user1.id, {})).resolves.toEqual(mapUser(userStub.user1));
expect(userMock.update).toHaveBeenCalledWith(userStub.user1.id, {
status: UserStatus.DELETED,
deletedAt: expect.any(Date),
});
});
it('should force delete user', async () => {
userMock.get.mockResolvedValue(userStub.user1);
userMock.update.mockResolvedValue(userStub.user1);
await expect(sut.delete(authStub.admin, userStub.user1.id, { force: true })).resolves.toEqual(
mapUser(userStub.user1),
);
expect(userMock.update).toHaveBeenCalledWith(userStub.user1.id, {
status: UserStatus.REMOVING,
deletedAt: expect.any(Date),
});
expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.USER_DELETION,
data: { id: userStub.user1.id, force: true },
});
});
});

View File

@@ -1,4 +1,4 @@
import { UserEntity } from '@app/infra/entities';
import { UserEntity, UserStatus } from '@app/infra/entities';
import { ImmichLogger } from '@app/infra/logger';
import { BadRequestException, ForbiddenException, Inject, Injectable, NotFoundException } from '@nestjs/common';
import { DateTime } from 'luxon';
@@ -18,7 +18,7 @@ import {
} from '../repositories';
import { StorageCore, StorageFolder } from '../storage';
import { SystemConfigCore } from '../system-config/system-config.core';
import { CreateUserDto, UpdateUserDto } from './dto';
import { CreateUserDto, DeleteUserDto, UpdateUserDto } from './dto';
import { CreateProfileImageResponseDto, UserResponseDto, mapCreateProfileImageResponse, mapUser } from './response-dto';
import { UserCore } from './user.core';
@@ -73,22 +73,29 @@ export class UserService {
return this.userCore.updateUser(auth.user, dto.id, dto).then(mapUser);
}
async delete(auth: AuthDto, id: string): Promise<UserResponseDto> {
const user = await this.findOrFail(id, {});
if (user.isAdmin) {
async delete(auth: AuthDto, id: string, dto: DeleteUserDto): Promise<UserResponseDto> {
const { force } = dto;
const { isAdmin } = await this.findOrFail(id, {});
if (isAdmin) {
throw new ForbiddenException('Cannot delete admin user');
}
await this.albumRepository.softDeleteAll(id);
return this.userRepository.delete(user).then(mapUser);
const status = force ? UserStatus.REMOVING : UserStatus.DELETED;
const user = await this.userRepository.update(id, { status, deletedAt: new Date() });
if (force) {
await this.jobRepository.queue({ name: JobName.USER_DELETION, data: { id: user.id, force } });
}
return mapUser(user);
}
async restore(auth: AuthDto, id: string): Promise<UserResponseDto> {
let user = await this.findOrFail(id, { withDeleted: true });
user = await this.userRepository.restore(user);
await this.findOrFail(id, { withDeleted: true });
await this.albumRepository.restoreAll(id);
return mapUser(user);
return this.userRepository.update(id, { deletedAt: null, status: UserStatus.ACTIVE }).then(mapUser);
}
async createProfileImage(auth: AuthDto, fileInfo: Express.Multer.File): Promise<CreateProfileImageResponseDto> {
@@ -154,7 +161,7 @@ export class UserService {
return true;
}
async handleUserDelete({ id }: IEntityJob) {
async handleUserDelete({ id, force }: IEntityJob) {
const config = await this.configCore.getConfig();
const user = await this.userRepository.get(id, { withDeleted: true });
if (!user) {
@@ -162,7 +169,7 @@ export class UserService {
}
// just for extra protection here
if (!this.isReadyForDeletion(user, config.user.deleteDelay)) {
if (!force && !this.isReadyForDeletion(user, config.user.deleteDelay)) {
this.logger.warn(`Skipped user that was not ready for deletion: id=${id}`);
return false;
}