refactor(server): user core (#4733)

server/src/domain/album/album.service.spec.ts

@@ -174,7 +174,7 @@ describe(AlbumService.name, () => {
         albumThumbnailAssetId: '123',
       });
 
-      expect(userMock.get).toHaveBeenCalledWith('user-id');
+      expect(userMock.get).toHaveBeenCalledWith('user-id', {});
     });
 
     it('should require valid userIds', async () => {
@@ -185,7 +185,7 @@ describe(AlbumService.name, () => {
           sharedWithUserIds: ['user-3'],
         }),
       ).rejects.toBeInstanceOf(BadRequestException);
-      expect(userMock.get).toHaveBeenCalledWith('user-3');
+      expect(userMock.get).toHaveBeenCalledWith('user-3', {});
       expect(albumMock.create).not.toHaveBeenCalled();
     });
   });

server/src/domain/album/album.service.ts

@@ -95,7 +95,7 @@ export class AlbumService {
 
   async create(authUser: AuthUserDto, dto: CreateAlbumDto): Promise<AlbumResponseDto> {
     for (const userId of dto.sharedWithUserIds || []) {
-      const exists = await this.userRepository.get(userId);
+      const exists = await this.userRepository.get(userId, {});
       if (!exists) {
         throw new BadRequestException('User not found');
       }
@@ -238,7 +238,7 @@ export class AlbumService {
         throw new BadRequestException('User already added');
       }
 
-      const user = await this.userRepository.get(userId);
+      const user = await this.userRepository.get(userId, {});
       if (!user) {
         throw new BadRequestException('User not found');
       }

server/src/domain/library/library.service.ts

@@ -343,7 +343,7 @@ export class LibraryService {
       return false;
     }
 
-    const user = await this.userRepository.get(library.ownerId);
+    const user = await this.userRepository.get(library.ownerId, {});
     if (!user?.externalPath) {
       this.logger.warn('User has no external path set, cannot refresh library');
       return false;

server/src/domain/repositories/user.repository.ts

@@ -13,10 +13,14 @@ export interface UserStatsQueryResponse {
   usage: number;
 }
 
+export interface UserFindOptions {
+  withDeleted?: boolean;
+}
+
 export const IUserRepository = 'IUserRepository';
 
 export interface IUserRepository {
-  get(id: string, withDeleted?: boolean): Promise<UserEntity | null>;
+  get(id: string, options: UserFindOptions): Promise<UserEntity | null>;
   getAdmin(): Promise<UserEntity | null>;
   hasAdmin(): Promise<boolean>;
   getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null>;

server/src/domain/storage-template/storage-template.service.ts

@@ -75,7 +75,7 @@ export class StorageTemplateService {
   async handleMigrationSingle({ id }: IEntityJob) {
     const [asset] = await this.assetRepository.getByIds([id]);
 
-    const user = await this.userRepository.get(asset.ownerId);
+    const user = await this.userRepository.get(asset.ownerId, {});
     const storageLabel = user?.storageLabel || null;
     const filename = asset.originalFileName || asset.id;
     await this.moveAsset(asset, { storageLabel, filename });

server/src/domain/user/user.core.ts

@@ -122,33 +122,4 @@ export class UserCore {
       throw new InternalServerErrorException('Failed to register new user');
     }
   }
-
-  async restoreUser(authUser: AuthUserDto, userToRestore: UserEntity): Promise<UserEntity> {
-    if (!authUser.isAdmin) {
-      throw new ForbiddenException('Unauthorized');
-    }
-    try {
-      return this.userRepository.restore(userToRestore);
-    } catch (e) {
-      Logger.error(e, 'Failed to restore deleted user');
-      throw new InternalServerErrorException('Failed to restore deleted user');
-    }
-  }
-
-  async deleteUser(authUser: AuthUserDto, userToDelete: UserEntity): Promise<UserEntity> {
-    if (!authUser.isAdmin) {
-      throw new ForbiddenException('Unauthorized');
-    }
-
-    if (userToDelete.isAdmin) {
-      throw new ForbiddenException('Cannot delete admin user');
-    }
-
-    try {
-      return this.userRepository.delete(userToDelete);
-    } catch (e) {
-      Logger.error(e, 'Failed to delete user');
-      throw new InternalServerErrorException('Failed to delete user');
-    }
-  }
 }

server/src/domain/user/user.service.spec.ts

@@ -6,6 +6,7 @@ import {
   NotFoundException,
 } from '@nestjs/common';
 import {
+  authStub,
   newAlbumRepositoryMock,
   newAssetRepositoryMock,
   newCryptoRepositoryMock,
@@ -17,7 +18,6 @@ import {
 } from '@test';
 import { when } from 'jest-when';
 import { Readable } from 'stream';
-import { AuthUserDto } from '../auth';
 import { JobName } from '../job';
 import {
   IAlbumRepository,
@@ -29,7 +29,7 @@ import {
   IUserRepository,
 } from '../repositories';
 import { UpdateUserDto } from './dto/update-user.dto';
-import { UserResponseDto, mapUser } from './response-dto';
+import { mapUser } from './response-dto';
 import { UserService } from './user.service';
 
 const makeDeletedAt = (daysAgo: number) => {
@@ -38,95 +38,6 @@ const makeDeletedAt = (daysAgo: number) => {
   return deletedAt;
 };
 
-const adminUserAuth: AuthUserDto = Object.freeze({
-  id: 'admin_id',
-  email: 'admin@test.com',
-  isAdmin: true,
-});
-
-const immichUserAuth: AuthUserDto = Object.freeze({
-  id: 'user-id',
-  email: 'immich@test.com',
-  isAdmin: false,
-});
-
-const adminUser: UserEntity = Object.freeze({
-  id: adminUserAuth.id,
-  email: 'admin@test.com',
-  password: 'admin_password',
-  firstName: 'admin_first_name',
-  lastName: 'admin_last_name',
-  isAdmin: true,
-  oauthId: '',
-  shouldChangePassword: false,
-  profileImagePath: '',
-  createdAt: new Date('2021-01-01'),
-  deletedAt: null,
-  updatedAt: new Date('2021-01-01'),
-  tags: [],
-  assets: [],
-  storageLabel: 'admin',
-  externalPath: null,
-  memoriesEnabled: true,
-});
-
-const immichUser: UserEntity = Object.freeze({
-  id: immichUserAuth.id,
-  email: 'immich@test.com',
-  password: 'immich_password',
-  firstName: 'immich_first_name',
-  lastName: 'immich_last_name',
-  isAdmin: false,
-  oauthId: '',
-  shouldChangePassword: false,
-  profileImagePath: '',
-  createdAt: new Date('2021-01-01'),
-  deletedAt: null,
-  updatedAt: new Date('2021-01-01'),
-  tags: [],
-  assets: [],
-  storageLabel: null,
-  externalPath: null,
-  memoriesEnabled: true,
-});
-
-const updatedImmichUser = Object.freeze<UserEntity>({
-  id: immichUserAuth.id,
-  email: 'immich@test.com',
-  password: 'immich_password',
-  firstName: 'updated_immich_first_name',
-  lastName: 'updated_immich_last_name',
-  isAdmin: false,
-  oauthId: '',
-  shouldChangePassword: true,
-  profileImagePath: '',
-  createdAt: new Date('2021-01-01'),
-  deletedAt: null,
-  updatedAt: new Date('2021-01-01'),
-  tags: [],
-  assets: [],
-  storageLabel: null,
-  externalPath: null,
-  memoriesEnabled: true,
-});
-
-const adminUserResponse = Object.freeze<UserResponseDto>({
-  id: adminUserAuth.id,
-  email: 'admin@test.com',
-  firstName: 'admin_first_name',
-  lastName: 'admin_last_name',
-  isAdmin: true,
-  oauthId: '',
-  shouldChangePassword: false,
-  profileImagePath: '',
-  createdAt: new Date('2021-01-01'),
-  deletedAt: null,
-  updatedAt: new Date('2021-01-01'),
-  storageLabel: 'admin',
-  externalPath: null,
-  memoriesEnabled: true,
-});
-
 describe(UserService.name, () => {
   let sut: UserService;
   let userMock: jest.Mocked<IUserRepository>;
@@ -149,119 +60,92 @@ describe(UserService.name, () => {
 
     sut = new UserService(albumMock, assetMock, cryptoRepositoryMock, jobMock, libraryMock, storageMock, userMock);
 
-    when(userMock.get).calledWith(adminUser.id).mockResolvedValue(adminUser);
-    when(userMock.get).calledWith(immichUser.id).mockResolvedValue(immichUser);
+    when(userMock.get).calledWith(authStub.admin.id, {}).mockResolvedValue(userStub.admin);
+    when(userMock.get).calledWith(authStub.admin.id, { withDeleted: true }).mockResolvedValue(userStub.admin);
+    when(userMock.get).calledWith(authStub.user1.id, {}).mockResolvedValue(userStub.user1);
+    when(userMock.get).calledWith(authStub.user1.id, { withDeleted: true }).mockResolvedValue(userStub.user1);
   });
 
   describe('getAll', () => {
     it('should get all users', async () => {
-      userMock.getList.mockResolvedValue([adminUser]);
-
-      const response = await sut.getAll(adminUserAuth, false);
-
-      expect(userMock.getList).toHaveBeenCalledWith({ withDeleted: true });
-      expect(response).toEqual([
-        {
-          id: adminUserAuth.id,
-          email: 'admin@test.com',
-          firstName: 'admin_first_name',
-          lastName: 'admin_last_name',
-          isAdmin: true,
-          oauthId: '',
-          shouldChangePassword: false,
-          profileImagePath: '',
-          createdAt: new Date('2021-01-01'),
-          deletedAt: null,
-          updatedAt: new Date('2021-01-01'),
-          storageLabel: 'admin',
-          externalPath: null,
-          memoriesEnabled: true,
-        },
+      userMock.getList.mockResolvedValue([userStub.admin]);
+      await expect(sut.getAll(authStub.admin, false)).resolves.toEqual([
+        expect.objectContaining({
+          id: authStub.admin.id,
+          email: authStub.admin.email,
+        }),
       ]);
+      expect(userMock.getList).toHaveBeenCalledWith({ withDeleted: true });
     });
   });
 
   describe('get', () => {
     it('should get a user by id', async () => {
-      userMock.get.mockResolvedValue(adminUser);
-
-      const response = await sut.get(adminUser.id);
-
-      expect(userMock.get).toHaveBeenCalledWith(adminUser.id, false);
-      expect(response).toEqual(adminUserResponse);
+      userMock.get.mockResolvedValue(userStub.admin);
+      await sut.get(authStub.admin.id);
+      expect(userMock.get).toHaveBeenCalledWith(authStub.admin.id, { withDeleted: false });
     });
 
     it('should throw an error if a user is not found', async () => {
       userMock.get.mockResolvedValue(null);
-
-      await expect(sut.get(adminUser.id)).rejects.toBeInstanceOf(NotFoundException);
-
-      expect(userMock.get).toHaveBeenCalledWith(adminUser.id, false);
+      await expect(sut.get(authStub.admin.id)).rejects.toBeInstanceOf(NotFoundException);
+      expect(userMock.get).toHaveBeenCalledWith(authStub.admin.id, { withDeleted: false });
     });
   });
 
   describe('getMe', () => {
     it("should get the auth user's info", async () => {
-      userMock.get.mockResolvedValue(adminUser);
-
-      const response = await sut.getMe(adminUser);
-
-      expect(userMock.get).toHaveBeenCalledWith(adminUser.id);
-      expect(response).toEqual(adminUserResponse);
+      userMock.get.mockResolvedValue(userStub.admin);
+      await sut.getMe(authStub.admin);
+      expect(userMock.get).toHaveBeenCalledWith(authStub.admin.id, {});
     });
 
     it('should throw an error if a user is not found', async () => {
       userMock.get.mockResolvedValue(null);
-
-      await expect(sut.getMe(adminUser)).rejects.toBeInstanceOf(BadRequestException);
-
-      expect(userMock.get).toHaveBeenCalledWith(adminUser.id);
+      await expect(sut.getMe(authStub.admin)).rejects.toBeInstanceOf(BadRequestException);
+      expect(userMock.get).toHaveBeenCalledWith(authStub.admin.id, {});
     });
   });
 
   describe('update', () => {
     it('should update user', async () => {
       const update: UpdateUserDto = {
-        id: immichUser.id,
+        id: userStub.user1.id,
         shouldChangePassword: true,
         email: 'immich@test.com',
         storageLabel: 'storage_label',
       };
       userMock.getByEmail.mockResolvedValue(null);
       userMock.getByStorageLabel.mockResolvedValue(null);
-      userMock.update.mockResolvedValue({ ...updatedImmichUser, isAdmin: true, storageLabel: 'storage_label' });
+      userMock.update.mockResolvedValue(userStub.user1);
+
+      await sut.update({ ...authStub.user1, isAdmin: true }, update);
 
-      const updatedUser = await sut.update({ ...immichUserAuth, isAdmin: true }, update);
-      expect(updatedUser.shouldChangePassword).toEqual(true);
       expect(userMock.getByEmail).toHaveBeenCalledWith(update.email);
       expect(userMock.getByStorageLabel).toHaveBeenCalledWith(update.storageLabel);
     });
 
     it('should not set an empty string for storage label', async () => {
-      userMock.update.mockResolvedValue(updatedImmichUser);
-
-      await sut.update(adminUserAuth, { id: immichUser.id, storageLabel: '' });
-
-      expect(userMock.update).toHaveBeenCalledWith(immichUser.id, { id: immichUser.id, storageLabel: null });
+      userMock.update.mockResolvedValue(userStub.user1);
+      await sut.update(userStub.admin, { id: userStub.user1.id, storageLabel: '' });
+      expect(userMock.update).toHaveBeenCalledWith(userStub.user1.id, { id: userStub.user1.id, storageLabel: null });
     });
 
     it('should omit a storage label set by non-admin users', async () => {
-      userMock.update.mockResolvedValue(updatedImmichUser);
-
-      await sut.update(immichUserAuth, { id: immichUser.id, storageLabel: 'admin' });
-
-      expect(userMock.update).toHaveBeenCalledWith(immichUser.id, { id: immichUser.id });
+      userMock.update.mockResolvedValue(userStub.user1);
+      await sut.update(userStub.user1, { id: userStub.user1.id, storageLabel: 'admin' });
+      expect(userMock.update).toHaveBeenCalledWith(userStub.user1.id, { id: userStub.user1.id });
     });
 
     it('user can only update its information', async () => {
       when(userMock.get)
-        .calledWith('not_immich_auth_user_id')
+        .calledWith('not_immich_auth_user_id', {})
         .mockResolvedValueOnce({
-          ...immichUser,
+          ...userStub.user1,
           id: 'not_immich_auth_user_id',
         });
 
-      const result = sut.update(immichUserAuth, {
+      const result = sut.update(userStub.user1, {
         id: 'not_immich_auth_user_id',
         password: 'I take over your account now',
       });
@@ -269,107 +153,104 @@ describe(UserService.name, () => {
     });
 
     it('should let a user change their email', async () => {
-      const dto = { id: immichUser.id, email: 'updated@test.com' };
+      const dto = { id: userStub.user1.id, email: 'updated@test.com' };
 
-      userMock.get.mockResolvedValue(immichUser);
-      userMock.update.mockResolvedValue(immichUser);
+      userMock.get.mockResolvedValue(userStub.user1);
+      userMock.update.mockResolvedValue(userStub.user1);
 
-      await sut.update(immichUser, dto);
+      await sut.update(userStub.user1, dto);
 
-      expect(userMock.update).toHaveBeenCalledWith(immichUser.id, {
+      expect(userMock.update).toHaveBeenCalledWith(userStub.user1.id, {
         id: 'user-id',
         email: 'updated@test.com',
       });
     });
 
     it('should not let a user change their email to one already in use', async () => {
-      const dto = { id: immichUser.id, email: 'updated@test.com' };
+      const dto = { id: userStub.user1.id, email: 'updated@test.com' };
 
-      userMock.get.mockResolvedValue(immichUser);
-      userMock.getByEmail.mockResolvedValue(adminUser);
+      userMock.get.mockResolvedValue(userStub.user1);
+      userMock.getByEmail.mockResolvedValue(userStub.admin);
 
-      await expect(sut.update(immichUser, dto)).rejects.toBeInstanceOf(BadRequestException);
+      await expect(sut.update(userStub.user1, dto)).rejects.toBeInstanceOf(BadRequestException);
 
       expect(userMock.update).not.toHaveBeenCalled();
     });
 
     it('should not let the admin change the storage label to one already in use', async () => {
-      const dto = { id: immichUser.id, storageLabel: 'admin' };
+      const dto = { id: userStub.user1.id, storageLabel: 'admin' };
 
-      userMock.get.mockResolvedValue(immichUser);
-      userMock.getByStorageLabel.mockResolvedValue(adminUser);
+      userMock.get.mockResolvedValue(userStub.user1);
+      userMock.getByStorageLabel.mockResolvedValue(userStub.admin);
 
-      await expect(sut.update(adminUser, dto)).rejects.toBeInstanceOf(BadRequestException);
+      await expect(sut.update(userStub.admin, dto)).rejects.toBeInstanceOf(BadRequestException);
 
       expect(userMock.update).not.toHaveBeenCalled();
     });
 
     it('admin can update any user information', async () => {
       const update: UpdateUserDto = {
-        id: immichUser.id,
+        id: userStub.user1.id,
         shouldChangePassword: true,
       };
 
-      when(userMock.update).calledWith(immichUser.id, update).mockResolvedValueOnce(updatedImmichUser);
-
-      const result = await sut.update(adminUserAuth, update);
-
-      expect(result).toBeDefined();
-      expect(result.id).toEqual(updatedImmichUser.id);
-      expect(result.shouldChangePassword).toEqual(updatedImmichUser.shouldChangePassword);
+      when(userMock.update).calledWith(userStub.user1.id, update).mockResolvedValueOnce(userStub.user1);
+      await sut.update(userStub.admin, update);
+      expect(userMock.update).toHaveBeenCalledWith(userStub.user1.id, {
+        id: 'user-id',
+        shouldChangePassword: true,
+      });
     });
 
     it('update user information should throw error if user not found', async () => {
-      when(userMock.get).calledWith(immichUser.id).mockResolvedValueOnce(null);
+      when(userMock.get).calledWith(userStub.user1.id, {}).mockResolvedValueOnce(null);
 
-      const result = sut.update(adminUser, {
-        id: immichUser.id,
+      const result = sut.update(userStub.admin, {
+        id: userStub.user1.id,
         shouldChangePassword: true,
       });
 
-      await expect(result).rejects.toBeInstanceOf(NotFoundException);
+      await expect(result).rejects.toBeInstanceOf(BadRequestException);
     });
 
     it('should let the admin update himself', async () => {
-      const dto = { id: adminUser.id, shouldChangePassword: true, isAdmin: true };
+      const dto = { id: userStub.admin.id, shouldChangePassword: true, isAdmin: true };
 
-      when(userMock.update).calledWith(adminUser.id, dto).mockResolvedValueOnce(adminUser);
+      when(userMock.update).calledWith(userStub.admin.id, dto).mockResolvedValueOnce(userStub.admin);
 
-      await sut.update(adminUser, dto);
+      await sut.update(userStub.admin, dto);
 
-      expect(userMock.update).toHaveBeenCalledWith(adminUser.id, dto);
+      expect(userMock.update).toHaveBeenCalledWith(userStub.admin.id, dto);
     });
 
     it('should not let the another user become an admin', async () => {
-      const dto = { id: immichUser.id, shouldChangePassword: true, isAdmin: true };
+      const dto = { id: userStub.user1.id, shouldChangePassword: true, isAdmin: true };
 
-      when(userMock.get).calledWith(immichUser.id).mockResolvedValueOnce(immichUser);
+      when(userMock.get).calledWith(userStub.user1.id, {}).mockResolvedValueOnce(userStub.user1);
 
-      await expect(sut.update(adminUser, dto)).rejects.toBeInstanceOf(BadRequestException);
+      await expect(sut.update(userStub.admin, dto)).rejects.toBeInstanceOf(BadRequestException);
     });
   });
 
   describe('restore', () => {
     it('should throw error if user could not be found', async () => {
-      userMock.get.mockResolvedValue(null);
-
-      await expect(sut.restore(immichUserAuth, adminUser.id)).rejects.toThrowError(BadRequestException);
+      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();
     });
 
     it('should require an admin', async () => {
-      when(userMock.get).calledWith(adminUser.id, true).mockResolvedValue(adminUser);
-      await expect(sut.restore(immichUserAuth, adminUser.id)).rejects.toBeInstanceOf(ForbiddenException);
-      expect(userMock.get).toHaveBeenCalledWith(adminUser.id, true);
+      when(userMock.get).calledWith(userStub.admin.id, { withDeleted: true }).mockResolvedValue(userStub.admin);
+      await expect(sut.restore(authStub.user1, userStub.admin.id)).rejects.toBeInstanceOf(ForbiddenException);
     });
 
     it('should restore an user', async () => {
-      userMock.get.mockResolvedValue(immichUser);
-      userMock.restore.mockResolvedValue(immichUser);
+      userMock.get.mockResolvedValue(userStub.user1);
+      userMock.restore.mockResolvedValue(userStub.user1);
 
-      await expect(sut.restore(adminUserAuth, immichUser.id)).resolves.toEqual(mapUser(immichUser));
-      expect(userMock.get).toHaveBeenCalledWith(immichUser.id, true);
-      expect(userMock.restore).toHaveBeenCalledWith(immichUser);
+      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);
     });
   });
 
@@ -377,27 +258,27 @@ describe(UserService.name, () => {
     it('should throw error if user could not be found', async () => {
       userMock.get.mockResolvedValue(null);
 
-      await expect(sut.delete(immichUserAuth, adminUser.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(adminUserAuth, adminUserAuth.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(immichUserAuth, adminUserAuth.id)).rejects.toBeInstanceOf(ForbiddenException);
+      await expect(sut.delete(authStub.user1, authStub.admin.id)).rejects.toBeInstanceOf(ForbiddenException);
 
       expect(userMock.delete).not.toHaveBeenCalled();
     });
 
     it('should delete user', async () => {
-      userMock.get.mockResolvedValue(immichUser);
-      userMock.delete.mockResolvedValue(immichUser);
+      userMock.get.mockResolvedValue(userStub.user1);
+      userMock.delete.mockResolvedValue(userStub.user1);
 
-      await expect(sut.delete(adminUserAuth, immichUser.id)).resolves.toEqual(mapUser(immichUser));
-      expect(userMock.get).toHaveBeenCalledWith(immichUser.id);
-      expect(userMock.delete).toHaveBeenCalledWith(immichUser);
+      await expect(sut.delete(userStub.admin, userStub.user1.id)).resolves.toEqual(mapUser(userStub.user1));
+      expect(userMock.get).toHaveBeenCalledWith(userStub.user1.id, {});
+      expect(userMock.delete).toHaveBeenCalledWith(userStub.user1);
     });
   });
 
@@ -443,18 +324,18 @@ describe(UserService.name, () => {
   describe('createProfileImage', () => {
     it('should throw an error if the user does not exist', async () => {
       const file = { path: '/profile/path' } as Express.Multer.File;
-      userMock.update.mockResolvedValue({ ...adminUser, profileImagePath: file.path });
+      userMock.update.mockResolvedValue({ ...userStub.admin, profileImagePath: file.path });
 
-      await sut.createProfileImage(adminUserAuth, file);
+      await sut.createProfileImage(userStub.admin, file);
 
-      expect(userMock.update).toHaveBeenCalledWith(adminUserAuth.id, { profileImagePath: file.path });
+      expect(userMock.update).toHaveBeenCalledWith(userStub.admin.id, { profileImagePath: file.path });
     });
 
     it('should throw an error if the user profile could not be updated with the new image', async () => {
       const file = { path: '/profile/path' } as Express.Multer.File;
       userMock.update.mockRejectedValue(new InternalServerErrorException('mocked error'));
 
-      await expect(sut.createProfileImage(adminUserAuth, file)).rejects.toThrowError(InternalServerErrorException);
+      await expect(sut.createProfileImage(userStub.admin, file)).rejects.toThrowError(InternalServerErrorException);
     });
   });
 
@@ -462,17 +343,17 @@ describe(UserService.name, () => {
     it('should throw an error if the user does not exist', async () => {
       userMock.get.mockResolvedValue(null);
 
-      await expect(sut.getProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(BadRequestException);
+      await expect(sut.getProfileImage(userStub.admin.id)).rejects.toBeInstanceOf(BadRequestException);
 
-      expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id);
+      expect(userMock.get).toHaveBeenCalledWith(userStub.admin.id, {});
     });
 
     it('should throw an error if the user does not have a picture', async () => {
-      userMock.get.mockResolvedValue(adminUser);
+      userMock.get.mockResolvedValue(userStub.admin);
 
-      await expect(sut.getProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(NotFoundException);
+      await expect(sut.getProfileImage(userStub.admin.id)).rejects.toBeInstanceOf(NotFoundException);
 
-      expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id);
+      expect(userMock.get).toHaveBeenCalledWith(userStub.admin.id, {});
     });
 
     it('should return the profile picture', async () => {
@@ -483,7 +364,7 @@ describe(UserService.name, () => {
 
       await expect(sut.getProfileImage(userStub.profilePath.id)).resolves.toEqual({ stream });
 
-      expect(userMock.get).toHaveBeenCalledWith(userStub.profilePath.id);
+      expect(userMock.get).toHaveBeenCalledWith(userStub.profilePath.id, {});
       expect(storageMock.createReadStream).toHaveBeenCalledWith('/path/to/profile.jpg', 'image/jpeg');
     });
   });
@@ -499,7 +380,7 @@ describe(UserService.name, () => {
     });
 
     it('should default to a random password', async () => {
-      userMock.getAdmin.mockResolvedValue(adminUser);
+      userMock.getAdmin.mockResolvedValue(userStub.admin);
       const ask = jest.fn().mockResolvedValue(undefined);
 
       const response = await sut.resetAdminPassword(ask);
@@ -508,12 +389,12 @@ describe(UserService.name, () => {
 
       expect(response.provided).toBe(false);
       expect(ask).toHaveBeenCalled();
-      expect(id).toEqual(adminUser.id);
+      expect(id).toEqual(userStub.admin.id);
       expect(update.password).toBeDefined();
     });
 
     it('should use the supplied password', async () => {
-      userMock.getAdmin.mockResolvedValue(adminUser);
+      userMock.getAdmin.mockResolvedValue(userStub.admin);
       const ask = jest.fn().mockResolvedValue('new-password');
 
       const response = await sut.resetAdminPassword(ask);
@@ -522,7 +403,7 @@ describe(UserService.name, () => {
 
       expect(response.provided).toBe(true);
       expect(ask).toHaveBeenCalled();
-      expect(id).toEqual(adminUser.id);
+      expect(id).toEqual(userStub.admin.id);
       expect(update.password).toBeDefined();
     });
   });

server/src/domain/user/user.service.ts

@@ -1,5 +1,5 @@
 import { UserEntity } from '@app/infra/entities';
-import { BadRequestException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common';
+import { BadRequestException, ForbiddenException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common';
 import { randomBytes } from 'crypto';
 import { AuthUserDto } from '../auth';
 import { IEntityJob, JobName } from '../job';
@@ -12,6 +12,7 @@ import {
   IStorageRepository,
   IUserRepository,
   ImmichReadStream,
+  UserFindOptions,
 } from '../repositories';
 import { StorageCore, StorageFolder } from '../storage';
 import { CreateUserDto, UpdateUserDto } from './dto';
@@ -41,7 +42,7 @@ export class UserService {
   }
 
   async get(userId: string): Promise<UserResponseDto> {
-    const user = await this.userRepository.get(userId, false);
+    const user = await this.userRepository.get(userId, { withDeleted: false });
     if (!user) {
       throw new NotFoundException('User not found');
     }
@@ -49,47 +50,43 @@ export class UserService {
     return mapUser(user);
   }
 
-  async getMe(authUser: AuthUserDto): Promise<UserResponseDto> {
-    const user = await this.userRepository.get(authUser.id);
-    if (!user) {
-      throw new BadRequestException('User not found');
-    }
-    return mapUser(user);
+  getMe(authUser: AuthUserDto): Promise<UserResponseDto> {
+    return this.findOrFail(authUser.id, {}).then(mapUser);
   }
 
-  async create(createUserDto: CreateUserDto): Promise<UserResponseDto> {
-    const createdUser = await this.userCore.createUser(createUserDto);
-    return mapUser(createdUser);
+  create(createUserDto: CreateUserDto): Promise<UserResponseDto> {
+    return this.userCore.createUser(createUserDto).then(mapUser);
   }
 
   async update(authUser: AuthUserDto, dto: UpdateUserDto): Promise<UserResponseDto> {
-    const user = await this.userRepository.get(dto.id);
-    if (!user) {
-      throw new NotFoundException('User not found');
-    }
-
-    const updatedUser = await this.userCore.updateUser(authUser, dto.id, dto);
-    return mapUser(updatedUser);
+    await this.findOrFail(dto.id, {});
+    return this.userCore.updateUser(authUser, dto.id, dto).then(mapUser);
   }
 
-  async delete(authUser: AuthUserDto, userId: string): Promise<UserResponseDto> {
-    const user = await this.userRepository.get(userId);
-    if (!user) {
-      throw new BadRequestException('User not found');
+  async delete(authUser: AuthUserDto, id: string): Promise<UserResponseDto> {
+    if (!authUser.isAdmin) {
+      throw new ForbiddenException('Unauthorized');
+    }
+
+    const user = await this.findOrFail(id, {});
+    if (user.isAdmin) {
+      throw new ForbiddenException('Cannot delete admin user');
     }
-    await this.albumRepository.softDeleteAll(userId);
-    const deletedUser = await this.userCore.deleteUser(authUser, user);
-    return mapUser(deletedUser);
+
+    await this.albumRepository.softDeleteAll(id);
+
+    return this.userRepository.delete(user).then(mapUser);
   }
 
-  async restore(authUser: AuthUserDto, userId: string): Promise<UserResponseDto> {
-    const user = await this.userRepository.get(userId, true);
-    if (!user) {
-      throw new BadRequestException('User not found');
+  async restore(authUser: AuthUserDto, id: string): Promise<UserResponseDto> {
+    if (!authUser.isAdmin) {
+      throw new ForbiddenException('Unauthorized');
     }
-    const updatedUser = await this.userCore.restoreUser(authUser, user);
-    await this.albumRepository.restoreAll(userId);
-    return mapUser(updatedUser);
+
+    let user = await this.findOrFail(id, { withDeleted: true });
+    user = await this.userRepository.restore(user);
+    await this.albumRepository.restoreAll(id);
+    return mapUser(user);
   }
 
   async createProfileImage(
@@ -101,7 +98,7 @@ export class UserService {
   }
 
   async getProfileImage(id: string): Promise<ImmichReadStream> {
-    const user = await this.findOrFail(id);
+    const user = await this.findOrFail(id, {});
     if (!user.profileImagePath) {
       throw new NotFoundException('User does not have a profile image');
     }
@@ -134,7 +131,7 @@ export class UserService {
   }
 
   async handleUserDelete({ id }: IEntityJob) {
-    const user = await this.userRepository.get(id, true);
+    const user = await this.userRepository.get(id, { withDeleted: true });
     if (!user) {
       return false;
     }
@@ -181,8 +178,8 @@ export class UserService {
     return msSinceDelete >= msDeleteWait;
   }
 
-  private async findOrFail(id: string) {
-    const user = await this.userRepository.get(id);
+  private async findOrFail(id: string, options: UserFindOptions) {
+    const user = await this.userRepository.get(id, options);
     if (!user) {
       throw new BadRequestException('User not found');
     }

server/src/infra/repositories/user.repository.ts

@@ -1,5 +1,5 @@
-import { IUserRepository, UserListFilter, UserStatsQueryResponse } from '@app/domain';
-import { Injectable, InternalServerErrorException } from '@nestjs/common';
+import { IUserRepository, UserFindOptions, UserListFilter, UserStatsQueryResponse } from '@app/domain';
+import { Injectable } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
 import { IsNull, Not, Repository } from 'typeorm';
 import { UserEntity } from '../entities';
@@ -8,8 +8,12 @@ import { UserEntity } from '../entities';
 export class UserRepository implements IUserRepository {
   constructor(@InjectRepository(UserEntity) private userRepository: Repository<UserEntity>) {}
 
-  async get(userId: string, withDeleted?: boolean): Promise<UserEntity | null> {
-    return this.userRepository.findOne({ where: { id: userId }, withDeleted: withDeleted });
+  async get(userId: string, options: UserFindOptions): Promise<UserEntity | null> {
+    options = options || {};
+    return this.userRepository.findOne({
+      where: { id: userId },
+      withDeleted: options.withDeleted,
+    });
   }
 
   async getAdmin(): Promise<UserEntity | null> {
@@ -51,19 +55,13 @@ export class UserRepository implements IUserRepository {
     });
   }
 
-  async create(user: Partial<UserEntity>): Promise<UserEntity> {
-    return this.userRepository.save(user);
+  create(user: Partial<UserEntity>): Promise<UserEntity> {
+    return this.save(user);
   }
 
-  async update(id: string, user: Partial<UserEntity>): Promise<UserEntity> {
-    user.id = id;
-
-    await this.userRepository.save(user);
-    const updatedUser = await this.get(id);
-    if (!updatedUser) {
-      throw new InternalServerErrorException('Cannot reload user after update');
-    }
-    return updatedUser;
+  // TODO change to (user: Partial<UserEntity>)
+  update(id: string, user: Partial<UserEntity>): Promise<UserEntity> {
+    return this.save({ ...user, id });
   }
 
   async delete(user: UserEntity, hard?: boolean): Promise<UserEntity> {
@@ -101,4 +99,9 @@ export class UserRepository implements IUserRepository {
 
     return stats;
   }
+
+  private async save(user: Partial<UserEntity>) {
+    const { id } = await this.userRepository.save(user);
+    return this.userRepository.findOneByOrFail({ id });
+  }
 }