Make user business logic reusable (#1114)

- Refactor user business logic from `user.service` into `user.domain`
  Make user business logic reusable by using `user.domain` from other services than `user.service`

- Add `jest-when` lib to make testing easier and use it in `userService`
  Using when helps from coupling tests to order of mock implementations execution

- Move all user business logic from user-repository to user.service

- Fix user.service tests not awaiting promises leaking state between tests

- Presentation logic for `getUserProfileImage` moved from UserService to UserController

- Fix `user.e2e` test logic. Pending fixing the configuration of the test itself

server/apps/immich/src/api-v1/album/album.service.spec.ts

@@ -17,6 +17,7 @@ describe('Album service', () => {
   const authUser: AuthUserDto = Object.freeze({
     id: '1111',
     email: 'auth@test.com',
+    isAdmin: false,
   });
   const albumId = 'f19ab956-4761-41ea-a5d6-bae948308d58';
   const sharedAlbumOwnerId = '2222';
@@ -400,7 +401,7 @@ describe('Album service', () => {
     albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
     albumRepositoryMock.addAssets.mockImplementation(() => Promise.resolve<AddAssetsResponseDto>(albumResponse));
 
-    expect(
+    await expect(
       sut.addAssetsToAlbum(
         authUser,
         {
@@ -464,7 +465,7 @@ describe('Album service', () => {
     albumRepositoryMock.get.mockImplementation(() => Promise.resolve<AlbumEntity>(albumEntity));
     albumRepositoryMock.addAssets.mockImplementation(() => Promise.resolve<AddAssetsResponseDto>(albumResponse));
 
-    expect(
+    await expect(
       sut.removeAssetsFromAlbum(
         authUser,
         {

server/apps/immich/src/api-v1/asset/asset.service.spec.ts

@@ -27,6 +27,7 @@ describe('AssetService', () => {
   const authUser: AuthUserDto = Object.freeze({
     id: 'user_id_1',
     email: 'auth@test.com',
+    isAdmin: false,
   });
 
   const _getCreateAssetDto = (): CreateAssetDto => {

server/apps/immich/src/api-v1/auth/auth.service.ts

@@ -19,17 +19,22 @@ import { AdminSignupResponseDto, mapAdminSignupResponse } from './response-dto/a
 import { LoginResponseDto } from './response-dto/login-response.dto';
 import { LogoutResponseDto } from './response-dto/logout-response.dto';
 import { OAuthService } from '../oauth/oauth.service';
+import { UserCore } from '../user/user.core';
 
 @Injectable()
 export class AuthService {
+  private userCore: UserCore;
+
   constructor(
     private oauthService: OAuthService,
     private immichJwtService: ImmichJwtService,
-    @Inject(USER_REPOSITORY) private userRepository: IUserRepository,
+    @Inject(USER_REPOSITORY) userRepository: IUserRepository,
-  ) {}
+  ) {
+    this.userCore = new UserCore(userRepository);
+  }
 
   public async login(loginCredential: LoginCredentialDto, clientIp: string): Promise<LoginResponseDto> {
-    let user = await this.userRepository.getByEmail(loginCredential.email, true);
+    let user = await this.userCore.getByEmail(loginCredential.email, true);
 
     if (user) {
       const isAuthenticated = await this.validatePassword(loginCredential.password, user);
@@ -59,7 +64,7 @@ export class AuthService {
 
   public async changePassword(authUser: AuthUserDto, dto: ChangePasswordDto) {
     const { password, newPassword } = dto;
-    const user = await this.userRepository.getByEmail(authUser.email, true);
+    const user = await this.userCore.getByEmail(authUser.email, true);
     if (!user) {
       throw new UnauthorizedException();
     }
@@ -71,18 +76,18 @@ export class AuthService {
 
     user.password = newPassword;
 
-    return this.userRepository.update(user.id, user);
+    return this.userCore.updateUser(authUser, user, dto);
   }
 
   public async adminSignUp(dto: SignUpDto): Promise<AdminSignupResponseDto> {
-    const adminUser = await this.userRepository.getAdmin();
+    const adminUser = await this.userCore.getAdmin();
 
     if (adminUser) {
       throw new BadRequestException('The server already has an admin');
     }
 
     try {
-      const admin = await this.userRepository.create({
+      const admin = await this.userCore.createUser({
         isAdmin: true,
         email: dto.email,
         firstName: dto.firstName,

server/apps/immich/src/api-v1/oauth/oauth.controller.ts

@@ -2,6 +2,7 @@ import { Body, Controller, Post, Res, ValidationPipe } from '@nestjs/common';
 import { ApiTags } from '@nestjs/swagger';
 import { Response } from 'express';
 import { AuthType } from '../../constants/jwt.constant';
+import { AuthUserDto, GetAuthUser } from '../../decorators/auth-user.decorator';
 import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service';
 import { LoginResponseDto } from '../auth/response-dto/login-response.dto';
 import { OAuthCallbackDto } from './dto/oauth-auth-code.dto';
@@ -21,10 +22,11 @@ export class OAuthController {
 
   @Post('/callback')
   public async callback(
+    @GetAuthUser() authUser: AuthUserDto,
     @Res({ passthrough: true }) response: Response,
     @Body(ValidationPipe) dto: OAuthCallbackDto,
   ): Promise<LoginResponseDto> {
-    const loginResponse = await this.oauthService.callback(dto);
+    const loginResponse = await this.oauthService.callback(authUser, dto);
     response.setHeader('Set-Cookie', this.immichJwtService.getCookies(loginResponse, AuthType.OAUTH));
     return loginResponse;
   }

server/apps/immich/src/api-v1/oauth/oauth.service.spec.ts

@@ -3,6 +3,7 @@ import { UserEntity } from '@app/database/entities/user.entity';
 import { ImmichConfigService } from '@app/immich-config';
 import { BadRequestException } from '@nestjs/common';
 import { generators, Issuer } from 'openid-client';
+import { AuthUserDto } from '../../decorators/auth-user.decorator';
 import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service';
 import { LoginResponseDto } from '../auth/response-dto/login-response.dto';
 import { OAuthService } from '../oauth/oauth.service';
@@ -12,13 +13,19 @@ const email = 'user@immich.com';
 const sub = 'my-auth-user-sub';
 
 const user = {
-  id: 'user',
+  id: 'user_id',
   email,
   firstName: 'user',
   lastName: 'imimch',
   oauthId: '',
 } as UserEntity;
 
+const authUser: AuthUserDto = {
+  id: 'user_id',
+  email,
+  isAdmin: true,
+};
+
 const loginResponse = {
   accessToken: 'access-token',
   userId: 'user',
@@ -104,7 +111,7 @@ describe('OAuthService', () => {
 
   describe('callback', () => {
     it('should throw an error if OAuth is not enabled', async () => {
-      await expect(sut.callback({ url: '' })).rejects.toBeInstanceOf(BadRequestException);
+      await expect(sut.callback(authUser, { url: '' })).rejects.toBeInstanceOf(BadRequestException);
     });
 
     it('should not allow auto registering', async () => {
@@ -118,7 +125,7 @@ describe('OAuthService', () => {
       jest.spyOn(sut['logger'], 'debug').mockImplementation(() => null);
       jest.spyOn(sut['logger'], 'warn').mockImplementation(() => null);
       userRepositoryMock.getByEmail.mockResolvedValue(null);
-      await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' })).rejects.toBeInstanceOf(
+      await expect(sut.callback(authUser, { url: 'http://immich/auth/login?code=abc123' })).rejects.toBeInstanceOf(
         BadRequestException,
       );
       expect(userRepositoryMock.getByEmail).toHaveBeenCalledTimes(1);
@@ -138,7 +145,9 @@ describe('OAuthService', () => {
       userRepositoryMock.update.mockResolvedValue(user);
       immichJwtServiceMock.createLoginResponse.mockResolvedValue(loginResponse);
 
-      await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' })).resolves.toEqual(loginResponse);
+      await expect(sut.callback(authUser, { url: 'http://immich/auth/login?code=abc123' })).resolves.toEqual(
+        loginResponse,
+      );
 
       expect(userRepositoryMock.getByEmail).toHaveBeenCalledTimes(1);
       expect(userRepositoryMock.update).toHaveBeenCalledWith(user.id, { oauthId: sub });
@@ -155,12 +164,15 @@ describe('OAuthService', () => {
       jest.spyOn(sut['logger'], 'debug').mockImplementation(() => null);
       jest.spyOn(sut['logger'], 'log').mockImplementation(() => null);
       userRepositoryMock.getByEmail.mockResolvedValue(null);
+      userRepositoryMock.getAdmin.mockResolvedValue(user);
       userRepositoryMock.create.mockResolvedValue(user);
       immichJwtServiceMock.createLoginResponse.mockResolvedValue(loginResponse);
 
-      await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' })).resolves.toEqual(loginResponse);
+      await expect(sut.callback(authUser, { url: 'http://immich/auth/login?code=abc123' })).resolves.toEqual(
+        loginResponse,
+      );
 
-      expect(userRepositoryMock.getByEmail).toHaveBeenCalledTimes(1);
+      expect(userRepositoryMock.getByEmail).toHaveBeenCalledTimes(2); // second call is for domain check before create
       expect(userRepositoryMock.create).toHaveBeenCalledTimes(1);
       expect(immichJwtServiceMock.createLoginResponse).toHaveBeenCalledTimes(1);
     });

server/apps/immich/src/api-v1/oauth/oauth.service.ts

@@ -1,9 +1,11 @@
 import { ImmichConfigService } from '@app/immich-config';
 import { BadRequestException, Inject, Injectable, Logger } from '@nestjs/common';
 import { ClientMetadata, custom, generators, Issuer, UserinfoResponse } from 'openid-client';
+import { AuthUserDto } from '../../decorators/auth-user.decorator';
 import { ImmichJwtService } from '../../modules/immich-jwt/immich-jwt.service';
 import { LoginResponseDto } from '../auth/response-dto/login-response.dto';
 import { IUserRepository, USER_REPOSITORY } from '../user/user-repository';
+import { UserCore } from '../user/user.core';
 import { OAuthCallbackDto } from './dto/oauth-auth-code.dto';
 import { OAuthConfigDto } from './dto/oauth-config.dto';
 import { OAuthConfigResponseDto } from './response-dto/oauth-config-response.dto';
@@ -14,13 +16,16 @@ type OAuthProfile = UserinfoResponse & {
 
 @Injectable()
 export class OAuthService {
+  private readonly userCore: UserCore;
   private readonly logger = new Logger(OAuthService.name);
 
   constructor(
     private immichJwtService: ImmichJwtService,
     private immichConfigService: ImmichConfigService,
-    @Inject(USER_REPOSITORY) private userRepository: IUserRepository,
+    @Inject(USER_REPOSITORY) userRepository: IUserRepository,
   ) {
+    this.userCore = new UserCore(userRepository);
+
     custom.setHttpOptionsDefaults({
       timeout: 30000,
     });
@@ -42,7 +47,7 @@ export class OAuthService {
     return { enabled: true, buttonText, url };
   }
 
-  public async callback(dto: OAuthCallbackDto): Promise<LoginResponseDto> {
+  public async callback(authUser: AuthUserDto, dto: OAuthCallbackDto): Promise<LoginResponseDto> {
     const redirectUri = dto.url.split('?')[0];
     const client = await this.getClient();
     const params = client.callbackParams(dto.url);
@@ -50,13 +55,13 @@ export class OAuthService {
     const profile = await client.userinfo<OAuthProfile>(tokens.access_token || '');
 
     this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`);
-    let user = await this.userRepository.getByOAuthId(profile.sub);
+    let user = await this.userCore.getByOAuthId(profile.sub);
 
     // link existing user
     if (!user) {
-      const emailUser = await this.userRepository.getByEmail(profile.email);
+      const emailUser = await this.userCore.getByEmail(profile.email);
       if (emailUser) {
-        user = await this.userRepository.update(emailUser.id, { oauthId: profile.sub });
+        user = await this.userCore.updateUser(authUser, emailUser, { oauthId: profile.sub });
       }
     }
 
@@ -72,7 +77,7 @@ export class OAuthService {
       }
 
       this.logger.log(`Registering new user: ${profile.email}/${profile.sub}`);
-      user = await this.userRepository.create({
+      user = await this.userCore.createUser({
         firstName: profile.given_name || '',
         lastName: profile.family_name || '',
         email: profile.email,

server/apps/immich/src/api-v1/tag/tag.service.spec.ts

@@ -11,6 +11,7 @@ describe('TagService', () => {
   const user1AuthUser: AuthUserDto = Object.freeze({
     id: '1111',
     email: 'testuser@email.com',
+    isAdmin: false,
   });
 
   const user1: UserEntity = Object.freeze({

server/apps/immich/src/api-v1/user/dto/create-user.dto.ts

@@ -20,3 +20,34 @@ export class CreateUserDto {
   @ApiProperty({ example: 'Doe' })
   lastName!: string;
 }
+
+export class CreateAdminDto {
+  @IsNotEmpty()
+  isAdmin!: true;
+
+  @IsEmail()
+  @Transform(({ value }) => value?.toLowerCase())
+  email!: string;
+
+  @IsNotEmpty()
+  password!: string;
+
+  @IsNotEmpty()
+  firstName!: string;
+
+  @IsNotEmpty()
+  lastName!: string;
+}
+
+export class CreateUserOAuthDto {
+  @IsEmail()
+  @Transform(({ value }) => value?.toLowerCase())
+  email!: string;
+
+  @IsNotEmpty()
+  oauthId!: string;
+
+  firstName?: string;
+
+  lastName?: string;
+}

server/apps/immich/src/api-v1/user/user-repository.spec.ts

@@ -1,30 +0,0 @@
-import { UserEntity } from '@app/database/entities/user.entity';
-import { BadRequestException } from '@nestjs/common';
-import { Repository } from 'typeorm';
-import { UserRepository } from './user-repository';
-
-describe('UserRepository', () => {
-  let sui: UserRepository;
-  let userRepositoryMock: jest.Mocked<Repository<UserEntity>>;
-
-  beforeAll(() => {
-    userRepositoryMock = {
-      findOne: jest.fn(),
-      save: jest.fn(),
-    } as unknown as jest.Mocked<Repository<UserEntity>>;
-
-    sui = new UserRepository(userRepositoryMock);
-  });
-
-  it('should be defined', () => {
-    expect(sui).toBeDefined();
-  });
-
-  describe('create', () => {
-    it('should not create a user if there is no local admin account', async () => {
-      userRepositoryMock.findOne.mockResolvedValue(null);
-      await expect(sui.create({ isAdmin: false })).rejects.toBeInstanceOf(BadRequestException);
-      expect(userRepositoryMock.findOne).toHaveBeenCalled();
-    });
-  });
-});

server/apps/immich/src/api-v1/user/user-repository.ts

@@ -1,7 +1,6 @@
 import { UserEntity } from '@app/database/entities/user.entity';
-import { BadRequestException } from '@nestjs/common';
+import { InternalServerErrorException } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
-import * as bcrypt from 'bcrypt';
 import { Not, Repository } from 'typeorm';
 
 export interface IUserRepository {
@@ -9,13 +8,17 @@ export interface IUserRepository {
   getAdmin(): Promise<UserEntity | null>;
   getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null>;
   getByOAuthId(oauthId: string): Promise<UserEntity | null>;
-  getList(filter?: { excludeId?: string }): Promise<UserEntity[]>;
+  getList(filter?: UserListFilter): Promise<UserEntity[]>;
   create(user: Partial<UserEntity>): Promise<UserEntity>;
   update(id: string, user: Partial<UserEntity>): Promise<UserEntity>;
   delete(user: UserEntity): Promise<UserEntity>;
   restore(user: UserEntity): Promise<UserEntity>;
 }
 
+export interface UserListFilter {
+  excludeId?: string;
+}
+
 export const USER_REPOSITORY = 'USER_REPOSITORY';
 
 export class UserRepository implements IUserRepository {
@@ -24,15 +27,15 @@ export class UserRepository implements IUserRepository {
     private userRepository: Repository<UserEntity>,
   ) {}
 
-  public async get(userId: string, withDeleted?: boolean): Promise<UserEntity | null> {
+  async get(userId: string, withDeleted?: boolean): Promise<UserEntity | null> {
     return this.userRepository.findOne({ where: { id: userId }, withDeleted: withDeleted });
   }
 
-  public async getAdmin(): Promise<UserEntity | null> {
+  async getAdmin(): Promise<UserEntity | null> {
     return this.userRepository.findOne({ where: { isAdmin: true } });
   }
 
-  public async getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null> {
+  async getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null> {
     let builder = this.userRepository.createQueryBuilder('user').where({ email });
 
     if (withPassword) {
@@ -42,11 +45,11 @@ export class UserRepository implements IUserRepository {
     return builder.getOne();
   }
 
-  public async getByOAuthId(oauthId: string): Promise<UserEntity | null> {
+  async getByOAuthId(oauthId: string): Promise<UserEntity | null> {
     return this.userRepository.findOne({ where: { oauthId } });
   }
 
-  public async getList({ excludeId }: { excludeId?: string } = {}): Promise<UserEntity[]> {
+  async getList({ excludeId }: UserListFilter = {}): Promise<UserEntity[]> {
     if (!excludeId) {
       return this.userRepository.find(); // TODO: this should also be ordered the same as below
     }
@@ -59,55 +62,26 @@ export class UserRepository implements IUserRepository {
     });
   }
 
-  public async create(user: Partial<UserEntity>): Promise<UserEntity> {
+  async create(user: Partial<UserEntity>): Promise<UserEntity> {
-    const localAdmin = await this.getAdmin();
-    if (!localAdmin && !user.isAdmin) {
-      throw new BadRequestException('The first registered account must the administrator.');
-    }
-
-    if (user.password) {
-      user.salt = await bcrypt.genSalt();
-      user.password = await this.hashPassword(user.password, user.salt);
-    }
-
     return this.userRepository.save(user);
   }
 
-  public async update(id: string, user: Partial<UserEntity>): Promise<UserEntity> {
+  async update(id: string, user: Partial<UserEntity>): Promise<UserEntity> {
     user.id = id;
 
-    // If payload includes password - Create new password for user
+    await this.userRepository.save(user);
-    if (user.password) {
+    const updatedUser = await this.get(id);
-      user.salt = await bcrypt.genSalt();
+    if (!updatedUser) {
-      user.password = await this.hashPassword(user.password, user.salt);
+      throw new InternalServerErrorException('Cannot reload user after update');
     }
-
+    return updatedUser;
-    // TODO: can this happen? If so we can move it to the service, otherwise remove it (also from DTO)
-    if (user.isAdmin) {
-      const adminUser = await this.userRepository.findOne({ where: { isAdmin: true } });
-
-      if (adminUser && adminUser.id !== id) {
-        throw new BadRequestException('Admin user exists');
-      }
-
-      user.isAdmin = true;
-    }
-
-    return this.userRepository.save(user);
   }
 
-  public async delete(user: UserEntity): Promise<UserEntity> {
+  async delete(user: UserEntity): Promise<UserEntity> {
-    if (user.isAdmin) {
-      throw new BadRequestException('Cannot delete admin user! stay sane!');
-    }
     return this.userRepository.softRemove(user);
   }
 
-  public async restore(user: UserEntity): Promise<UserEntity> {
+  async restore(user: UserEntity): Promise<UserEntity> {
     return this.userRepository.recover(user);
   }
-
-  private async hashPassword(password: string, salt: string): Promise<string> {
-    return bcrypt.hash(password, salt);
-  }
 }

server/apps/immich/src/api-v1/user/user.controller.ts

@@ -12,6 +12,7 @@ import {
   UploadedFile,
   Response,
   ParseBoolPipe,
+  StreamableFile,
 } from '@nestjs/common';
 import { UserService } from './user.service';
 import { Authenticated } from '../../decorators/authenticated.decorator';
@@ -111,6 +112,10 @@ export class UserController {
 
   @Get('/profile-image/:userId')
   async getProfileImage(@Param('userId') userId: string, @Response({ passthrough: true }) res: Res): Promise<any> {
-    return this.userService.getUserProfileImage(userId, res);
+    const readableStream = await this.userService.getUserProfileImage(userId);
+    res.set({
+      'Content-Type': 'image/jpeg',
+    });
+    return new StreamableFile(readableStream);
   }
 }

server/apps/immich/src/api-v1/user/user.core.ts

@@ -0,0 +1,163 @@
+import { UserEntity } from '@app/database/entities/user.entity';
+import {
+  BadRequestException,
+  ForbiddenException,
+  InternalServerErrorException,
+  Logger,
+  NotFoundException,
+  UnauthorizedException,
+} from '@nestjs/common';
+import { genSalt, hash } from 'bcrypt';
+import { createReadStream, constants, ReadStream } from 'fs';
+import fs from 'fs/promises';
+import { AuthUserDto } from '../../decorators/auth-user.decorator';
+import { CreateAdminDto, CreateUserDto, CreateUserOAuthDto } from './dto/create-user.dto';
+import { IUserRepository, UserListFilter } from './user-repository';
+
+export class UserCore {
+  constructor(private userRepository: IUserRepository) {}
+
+  private async generateSalt(): Promise<string> {
+    return genSalt();
+  }
+
+  private async hashPassword(password: string, salt: string): Promise<string> {
+    return hash(password, salt);
+  }
+
+  async updateUser(authUser: AuthUserDto, userToUpdate: UserEntity, data: Partial<UserEntity>): Promise<UserEntity> {
+    if (!authUser.isAdmin && (authUser.id !== userToUpdate.id || userToUpdate.id != data.id)) {
+      throw new ForbiddenException('You are not allowed to update this user');
+    }
+
+    // TODO: can this happen? If so we should implement a test case, otherwise remove it (also from DTO)
+    if (userToUpdate.isAdmin) {
+      const adminUser = await this.userRepository.getAdmin();
+      if (adminUser && adminUser.id !== userToUpdate.id) {
+        throw new BadRequestException('Admin user exists');
+      }
+    }
+
+    try {
+      const payload: Partial<UserEntity> = { ...data };
+      if (payload.password) {
+        const salt = await this.generateSalt();
+        payload.salt = salt;
+        payload.password = await this.hashPassword(payload.password, salt);
+      }
+      return this.userRepository.update(userToUpdate.id, payload);
+    } catch (e) {
+      Logger.error(e, 'Failed to update user info');
+      throw new InternalServerErrorException('Failed to update user info');
+    }
+  }
+
+  async createUser(createUserDto: CreateUserDto | CreateAdminDto | CreateUserOAuthDto): Promise<UserEntity> {
+    const user = await this.userRepository.getByEmail(createUserDto.email);
+    if (user) {
+      throw new BadRequestException('User exists');
+    }
+
+    if (!(createUserDto as CreateAdminDto).isAdmin) {
+      const localAdmin = await this.userRepository.getAdmin();
+      if (!localAdmin) {
+        throw new BadRequestException('The first registered account must the administrator.');
+      }
+    }
+
+    try {
+      const payload: Partial<UserEntity> = { ...createUserDto };
+      if (payload.password) {
+        const salt = await this.generateSalt();
+        payload.salt = salt;
+        payload.password = await this.hashPassword(payload.password, salt);
+      }
+      return this.userRepository.create(payload);
+    } catch (e) {
+      Logger.error(e, 'Create new user');
+      throw new InternalServerErrorException('Failed to register new user');
+    }
+  }
+
+  async get(userId: string, withDeleted?: boolean): Promise<UserEntity | null> {
+    return this.userRepository.get(userId, withDeleted);
+  }
+
+  async getAdmin(): Promise<UserEntity | null> {
+    return this.userRepository.getAdmin();
+  }
+
+  async getByEmail(email: string, withPassword?: boolean): Promise<UserEntity | null> {
+    return this.userRepository.getByEmail(email, withPassword);
+  }
+
+  async getByOAuthId(oauthId: string): Promise<UserEntity | null> {
+    return this.userRepository.getByOAuthId(oauthId);
+  }
+
+  async getUserProfileImage(user: UserEntity): Promise<ReadStream> {
+    if (!user.profileImagePath) {
+      throw new NotFoundException('User does not have a profile image');
+    }
+    await fs.access(user.profileImagePath, constants.R_OK | constants.W_OK);
+    return createReadStream(user.profileImagePath);
+  }
+
+  async getList(filter?: UserListFilter): Promise<UserEntity[]> {
+    return this.userRepository.getList(filter);
+  }
+
+  async createProfileImage(authUser: AuthUserDto, filePath: string): Promise<UserEntity> {
+    // TODO: do we need to do this? Maybe we can trust the authUser
+    const user = await this.userRepository.get(authUser.id);
+    if (!user) {
+      throw new NotFoundException('User not found');
+    }
+
+    try {
+      return this.userRepository.update(user.id, { profileImagePath: filePath });
+    } catch (e) {
+      Logger.error(e, 'Create User Profile Image');
+      throw new InternalServerErrorException('Failed to create new user profile image');
+    }
+  }
+
+  async restoreUser(authUser: AuthUserDto, userToRestore: UserEntity): Promise<UserEntity> {
+    // TODO: do we need to do this? Maybe we can trust the authUser
+    const requestor = await this.userRepository.get(authUser.id);
+    if (!requestor) {
+      throw new UnauthorizedException('Requestor not found');
+    }
+    if (!requestor.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> {
+    // TODO: do we need to do this? Maybe we can trust the authUser
+    const requestor = await this.userRepository.get(authUser.id);
+    if (!requestor) {
+      throw new UnauthorizedException('Requestor not found');
+    }
+    if (!requestor.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/apps/immich/src/api-v1/user/user.service.spec.ts

@@ -1,26 +1,30 @@
 import { UserEntity } from '@app/database/entities/user.entity';
-import { BadRequestException, NotFoundException } from '@nestjs/common';
+import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common';
 import { newUserRepositoryMock } from '../../../test/test-utils';
 import { AuthUserDto } from '../../decorators/auth-user.decorator';
 import { IUserRepository } from './user-repository';
+import { when } from 'jest-when';
 import { UserService } from './user.service';
+import { UpdateUserDto } from './dto/update-user.dto';
 
 describe('UserService', () => {
-  let sui: UserService;
+  let sut: UserService;
   let userRepositoryMock: jest.Mocked<IUserRepository>;
 
-  const adminAuthUser: AuthUserDto = Object.freeze({
+  const adminUserAuth: AuthUserDto = Object.freeze({
     id: 'admin_id',
     email: 'admin@test.com',
+    isAdmin: true,
   });
 
-  const immichAuthUser: AuthUserDto = Object.freeze({
+  const immichUserAuth: AuthUserDto = Object.freeze({
     id: 'immich_id',
     email: 'immich@test.com',
+    isAdmin: false,
   });
 
-  const adminUser: UserEntity = {
+  const adminUser: UserEntity = Object.freeze({
-    id: 'admin_id',
+    id: adminUserAuth.id,
     email: 'admin@test.com',
     password: 'admin_password',
     salt: 'admin_salt',
@@ -32,10 +36,10 @@ describe('UserService', () => {
     profileImagePath: '',
     createdAt: '2021-01-01',
     tags: [],
-  };
+  });
 
-  const immichUser: UserEntity = {
+  const immichUser: UserEntity = Object.freeze({
-    id: 'immich_id',
+    id: immichUserAuth.id,
     email: 'immich@test.com',
     password: 'immich_password',
     salt: 'immich_salt',
@@ -47,10 +51,10 @@ describe('UserService', () => {
     profileImagePath: '',
     createdAt: '2021-01-01',
     tags: [],
-  };
+  });
 
-  const updatedImmichUser: UserEntity = {
+  const updatedImmichUser: UserEntity = Object.freeze({
-    id: 'immich_id',
+    id: immichUserAuth.id,
     email: 'immich@test.com',
     password: 'immich_password',
     salt: 'immich_salt',
@@ -62,87 +66,92 @@ describe('UserService', () => {
     profileImagePath: '',
     createdAt: '2021-01-01',
     tags: [],
-  };
-
-  beforeAll(() => {
-    userRepositoryMock = newUserRepositoryMock();
-
-    sui = new UserService(userRepositoryMock);
   });
 
-  it('should be defined', () => {
+  beforeEach(() => {
-    expect(sui).toBeDefined();
+    userRepositoryMock = newUserRepositoryMock();
+    when(userRepositoryMock.get).calledWith(adminUser.id).mockResolvedValue(adminUser);
+    when(userRepositoryMock.get).calledWith(adminUser.id, undefined).mockResolvedValue(adminUser);
+    when(userRepositoryMock.get).calledWith(immichUser.id, undefined).mockResolvedValue(immichUser);
+
+    sut = new UserService(userRepositoryMock);
   });
 
   describe('Update user', () => {
     it('should update user', async () => {
-      const requestor = immichAuthUser;
+      const update: UpdateUserDto = {
-      const userToUpdate = immichUser;
+        id: immichUser.id,
-
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(immichUser));
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(userToUpdate));
-      userRepositoryMock.update.mockImplementationOnce(() => Promise.resolve(updatedImmichUser));
-
-      const result = await sui.updateUser(requestor, {
-        id: userToUpdate.id,
         shouldChangePassword: true,
-      });
+      };
-      expect(result.shouldChangePassword).toEqual(true);
+
+      when(userRepositoryMock.update).calledWith(update.id, update).mockResolvedValueOnce(updatedImmichUser);
+
+      const updatedUser = await sut.updateUser(immichUserAuth, update);
+      expect(updatedUser.shouldChangePassword).toEqual(true);
     });
 
-    it('user can only update its information', () => {
+    it('user can only update its information', async () => {
-      const requestor = immichAuthUser;
+      when(userRepositoryMock.get)
+        .calledWith('not_immich_auth_user_id', undefined)
+        .mockResolvedValueOnce({
+          ...immichUser,
+          id: 'not_immich_auth_user_id',
+        });
 
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(immichUser));
+      const result = sut.updateUser(immichUserAuth, {
-
-      const result = sui.updateUser(requestor, {
         id: 'not_immich_auth_user_id',
         password: 'I take over your account now',
       });
-      expect(result).rejects.toBeInstanceOf(BadRequestException);
+      await expect(result).rejects.toBeInstanceOf(ForbiddenException);
     });
 
     it('admin can update any user information', async () => {
-      const requestor = adminAuthUser;
+      const update: UpdateUserDto = {
-      const userToUpdate = immichUser;
+        id: immichUser.id,
-
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(adminUser));
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(userToUpdate));
-      userRepositoryMock.update.mockImplementationOnce(() => Promise.resolve(updatedImmichUser));
-
-      const result = await sui.updateUser(requestor, {
-        id: userToUpdate.id,
         shouldChangePassword: true,
-      });
+      };
+
+      when(userRepositoryMock.update).calledWith(immichUser.id, update).mockResolvedValueOnce(updatedImmichUser);
+
+      const result = await sut.updateUser(adminUserAuth, update);
 
       expect(result).toBeDefined();
       expect(result.id).toEqual(updatedImmichUser.id);
       expect(result.shouldChangePassword).toEqual(updatedImmichUser.shouldChangePassword);
     });
 
-    it('update user information should throw error if user not found', () => {
+    it('update user information should throw error if user not found', async () => {
-      const requestor = adminAuthUser;
+      when(userRepositoryMock.get).calledWith(immichUser.id, undefined).mockResolvedValueOnce(null);
-      const userToUpdate = immichUser;
 
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(adminUser));
+      const result = sut.updateUser(adminUser, {
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(null));
+        id: immichUser.id,
-
-      const result = sui.updateUser(requestor, {
-        id: userToUpdate.id,
         shouldChangePassword: true,
       });
-      expect(result).rejects.toBeInstanceOf(NotFoundException);
+
+      await expect(result).rejects.toBeInstanceOf(NotFoundException);
     });
+  });
 
-    it('cannot delete admin user', () => {
+  describe('Delete user', () => {
-      const requestor = adminAuthUser;
+    it('cannot delete admin user', async () => {
+      const result = sut.deleteUser(adminUserAuth, adminUserAuth.id);
 
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(adminUser));
+      await expect(result).rejects.toBeInstanceOf(ForbiddenException);
-      userRepositoryMock.get.mockImplementationOnce(() => Promise.resolve(adminUser));
+    });
+  });
 
-      const result = sui.deleteUser(requestor, adminAuthUser.id);
+  describe('Create user', () => {
+    it('should not create a user if there is no local admin account', async () => {
+      when(userRepositoryMock.getAdmin).calledWith().mockResolvedValueOnce(null);
 
-      expect(result).rejects.toBeInstanceOf(BadRequestException);
+      await expect(
+        sut.createUser({
+          email: 'john_smith@email.com',
+          firstName: 'John',
+          lastName: 'Smith',
+          password: 'password',
+        }),
+      ).rejects.toBeInstanceOf(BadRequestException);
     });
   });
 });

server/apps/immich/src/api-v1/user/user.service.ts

@@ -1,16 +1,5 @@
-import {
+import { BadRequestException, Inject, Injectable, NotFoundException } from '@nestjs/common';
-  BadRequestException,
+import { ReadStream } from 'fs';
-  ForbiddenException,
-  Inject,
-  Injectable,
-  InternalServerErrorException,
-  Logger,
-  NotFoundException,
-  StreamableFile,
-  UnauthorizedException,
-} from '@nestjs/common';
-import { Response as Res } from 'express';
-import { constants, createReadStream } from 'fs';
 import { AuthUserDto } from '../../decorators/auth-user.decorator';
 import { CreateUserDto } from './dto/create-user.dto';
 import { UpdateUserDto } from './dto/update-user.dto';
@@ -22,28 +11,30 @@ import {
 import { mapUserCountResponse, UserCountResponseDto } from './response-dto/user-count-response.dto';
 import { mapUser, UserResponseDto } from './response-dto/user-response.dto';
 import { IUserRepository, USER_REPOSITORY } from './user-repository';
-import fs from 'fs/promises';
+import { UserCore } from './user.core';
 
 @Injectable()
 export class UserService {
+  private userCore: UserCore;
   constructor(
     @Inject(USER_REPOSITORY)
-    private userRepository: IUserRepository,
+    userRepository: IUserRepository,
-  ) {}
+  ) {
+    this.userCore = new UserCore(userRepository);
+  }
 
   async getAllUsers(authUser: AuthUserDto, isAll: boolean): Promise<UserResponseDto[]> {
     if (isAll) {
-      const allUsers = await this.userRepository.getList();
+      const allUsers = await this.userCore.getList();
       return allUsers.map(mapUser);
     }
 
-    const allUserExceptRequestedUser = await this.userRepository.getList({ excludeId: authUser.id });
+    const allUserExceptRequestedUser = await this.userCore.getList({ excludeId: authUser.id });
-
     return allUserExceptRequestedUser.map(mapUser);
   }
 
   async getUserById(userId: string, withDeleted = false): Promise<UserResponseDto> {
-    const user = await this.userRepository.get(userId, withDeleted);
+    const user = await this.userCore.get(userId, withDeleted);
     if (!user) {
       throw new NotFoundException('User not found');
     }
@@ -52,7 +43,7 @@ export class UserService {
   }
 
   async getUserInfo(authUser: AuthUserDto): Promise<UserResponseDto> {
-    const user = await this.userRepository.get(authUser.id);
+    const user = await this.userCore.get(authUser.id);
     if (!user) {
       throw new BadRequestException('User not found');
     }
@@ -60,7 +51,7 @@ export class UserService {
   }
 
   async getUserCount(dto: UserCountDto): Promise<UserCountResponseDto> {
-    let users = await this.userRepository.getList();
+    let users = await this.userCore.getList();
 
     if (dto.admin) {
       users = users.filter((user) => user.isAdmin);
@@ -70,142 +61,50 @@ export class UserService {
   }
 
   async createUser(createUserDto: CreateUserDto): Promise<UserResponseDto> {
-    const user = await this.userRepository.getByEmail(createUserDto.email);
+    const createdUser = await this.userCore.createUser(createUserDto);
-
+    return mapUser(createdUser);
-    if (user) {
-      throw new BadRequestException('User exists');
-    }
-
-    try {
-      const savedUser = await this.userRepository.create(createUserDto);
-
-      return mapUser(savedUser);
-    } catch (e) {
-      Logger.error(e, 'Create new user');
-      throw new InternalServerErrorException('Failed to register new user');
-    }
   }
 
   async updateUser(authUser: AuthUserDto, updateUserDto: UpdateUserDto): Promise<UserResponseDto> {
-    const requestor = await this.userRepository.get(authUser.id);
+    const user = await this.userCore.get(updateUserDto.id);
-
-    if (!requestor) {
-      throw new NotFoundException('Requestor not found');
-    }
-
-    if (!requestor.isAdmin) {
-      if (requestor.id !== updateUserDto.id) {
-        throw new BadRequestException('Unauthorized');
-      }
-    }
-
-    const user = await this.userRepository.get(updateUserDto.id);
     if (!user) {
       throw new NotFoundException('User not found');
     }
-    try {
+    const updatedUser = await this.userCore.updateUser(authUser, user, updateUserDto);
-      user.password = updateUserDto.password ?? user.password;
+    return mapUser(updatedUser);
-      user.firstName = updateUserDto.firstName ?? user.firstName;
-      user.lastName = updateUserDto.lastName ?? user.lastName;
-      user.isAdmin = updateUserDto.isAdmin ?? user.isAdmin;
-      user.shouldChangePassword = updateUserDto.shouldChangePassword ?? user.shouldChangePassword;
-      user.profileImagePath = updateUserDto.profileImagePath ?? user.profileImagePath;
-
-      const updatedUser = await this.userRepository.update(user.id, user);
-
-      return mapUser(updatedUser);
-    } catch (e) {
-      Logger.error(e, 'Failed to update user info');
-      throw new InternalServerErrorException('Failed to update user info');
-    }
   }
 
   async deleteUser(authUser: AuthUserDto, userId: string): Promise<UserResponseDto> {
-    const requestor = await this.userRepository.get(authUser.id);
+    const user = await this.userCore.get(userId);
-    if (!requestor) {
-      throw new UnauthorizedException('Requestor not found');
-    }
-    if (!requestor.isAdmin) {
-      throw new ForbiddenException('Unauthorized');
-    }
-    const user = await this.userRepository.get(userId);
     if (!user) {
       throw new BadRequestException('User not found');
     }
-
+    const deletedUser = await this.userCore.deleteUser(authUser, user);
-    if (user.isAdmin) {
+    return mapUser(deletedUser);
-      throw new BadRequestException('Cannot delete admin user');
-    }
-
-    try {
-      const deletedUser = await this.userRepository.delete(user);
-      return mapUser(deletedUser);
-    } catch (e) {
-      Logger.error(e, 'Failed to delete user');
-      throw new InternalServerErrorException('Failed to delete user');
-    }
   }
 
   async restoreUser(authUser: AuthUserDto, userId: string): Promise<UserResponseDto> {
-    const requestor = await this.userRepository.get(authUser.id);
+    const user = await this.userCore.get(userId, true);
-    if (!requestor) {
-      throw new UnauthorizedException('Requestor not found');
-    }
-    if (!requestor.isAdmin) {
-      throw new ForbiddenException('Unauthorized');
-    }
-    const user = await this.userRepository.get(userId, true);
     if (!user) {
       throw new BadRequestException('User not found');
     }
-    try {
+    const updatedUser = await this.userCore.restoreUser(authUser, user);
-      const restoredUser = await this.userRepository.restore(user);
+    return mapUser(updatedUser);
-      return mapUser(restoredUser);
-    } catch (e) {
-      Logger.error(e, 'Failed to restore deleted user');
-      throw new InternalServerErrorException('Failed to restore deleted user');
-    }
   }
 
   async createProfileImage(
     authUser: AuthUserDto,
     fileInfo: Express.Multer.File,
   ): Promise<CreateProfileImageResponseDto> {
-    const user = await this.userRepository.get(authUser.id);
+    const updatedUser = await this.userCore.createProfileImage(authUser, fileInfo.path);
+    return mapCreateProfileImageResponse(updatedUser.id, updatedUser.profileImagePath);
+  }
+
+  async getUserProfileImage(userId: string): Promise<ReadStream> {
+    const user = await this.userCore.get(userId);
     if (!user) {
       throw new NotFoundException('User not found');
     }
-
+    return this.userCore.getUserProfileImage(user);
-    try {
-      await this.userRepository.update(user.id, { profileImagePath: fileInfo.path });
-
-      return mapCreateProfileImageResponse(authUser.id, fileInfo.path);
-    } catch (e) {
-      Logger.error(e, 'Create User Profile Image');
-      throw new InternalServerErrorException('Failed to create new user profile image');
-    }
-  }
-
-  async getUserProfileImage(userId: string, res: Res) {
-    try {
-      const user = await this.userRepository.get(userId);
-      if (!user) {
-        throw new NotFoundException('User not found');
-      }
-
-      if (!user.profileImagePath) {
-        throw new NotFoundException('User does not have a profile image');
-      }
-
-      await fs.access(user.profileImagePath, constants.R_OK | constants.W_OK);
-
-      res.set({
-        'Content-Type': 'image/jpeg',
-      });
-      const fileStream = createReadStream(user.profileImagePath);
-      return new StreamableFile(fileStream);
-    } catch (e) {
-      throw new NotFoundException('User does not have a profile image');
-    }
   }
 }

server/apps/immich/src/decorators/auth-user.decorator.ts

@@ -5,16 +5,18 @@ import { UserEntity } from '@app/database/entities/user.entity';
 export class AuthUserDto {
   id!: string;
   email!: string;
+  isAdmin!: boolean;
 }
 
 export const GetAuthUser = createParamDecorator((data, ctx: ExecutionContext): AuthUserDto => {
   const req = ctx.switchToHttp().getRequest<{ user: UserEntity }>();
 
-  const { id, email } = req.user;
+  const { id, email, isAdmin } = req.user;
 
   const authUser: AuthUserDto = {
     id: id.toString(),
     email,
+    isAdmin,
   };
 
   return authUser;

server/apps/immich/test/jest-e2e.json

@@ -7,8 +7,15 @@
     "^.+\\.(t|j)s$": "ts-jest"
   },
   "moduleNameMapper": {
-    "@app/common/(.*)": "<rootDir>../../../libs/common/src/$1",
+    "^@app/database(|/.*)$": "<rootDir>../../../libs/database/src/$1",
+    "@app/database/config": "<rootDir>../../../libs/database/src/config",
     "@app/database/config/(.*)": "<rootDir>../../../libs/database/src/config/$1",
-    "@app/database/entities/(.*)": "<rootDir>../../../libs/database/src/entities/$1"
+    "@app/database/entities/(.*)": "<rootDir>../../../libs/database/src/entities/$1",
+    "@app/common": "<rootDir>../../../libs/common/src",
+    "@app/common/(.*)": "<rootDir>../../../libs/common/src/$1",
+    "^@app/job(|/.*)$": "<rootDir>../../../libs/job/src/$1",
+    "@app/job": "<rootDir>../../../libs/job/src",
+    "^@app/immich-config(|/.*)$": "<rootDir>../../../libs/immich-config/src/$1",
+    "^@app/storage(|/.*)$": "<rootDir>../../../libs/storage/src/$1"
   }
 }

server/apps/immich/test/test-utils.ts

@@ -33,6 +33,7 @@ export function getAuthUser(): AuthUserDto {
   return {
     id: '3108ac14-8afb-4b7e-87fd-39ebb6b79750',
     email: 'test@email.com',
+    isAdmin: false,
   };
 }
 

server/apps/immich/test/user.e2e-spec.ts

@@ -7,11 +7,11 @@ import { databaseConfig } from '@app/database/config/database.config';
 import { UserModule } from '../src/api-v1/user/user.module';
 import { ImmichJwtModule } from '../src/modules/immich-jwt/immich-jwt.module';
 import { UserService } from '../src/api-v1/user/user.service';
-import { CreateUserDto } from '../src/api-v1/user/dto/create-user.dto';
+import { CreateAdminDto, CreateUserDto } from '../src/api-v1/user/dto/create-user.dto';
 import { UserResponseDto } from '../src/api-v1/user/response-dto/user-response.dto';
 import { DataSource } from 'typeorm';
 
-function _createUser(userService: UserService, data: CreateUserDto) {
+function _createUser(userService: UserService, data: CreateUserDto | CreateAdminDto) {
   return userService.createUser(data);
 }
 
@@ -67,13 +67,15 @@ describe('User', () => {
       const userTwoEmail = 'two@test.com';
 
       beforeAll(async () => {
+        // first user must be admin
+        authUser = await _createUser(userService, {
+          firstName: 'auth-user',
+          lastName: 'test',
+          email: authUserEmail,
+          password: '1234',
+          isAdmin: true,
+        });
         await Promise.allSettled([
-          _createUser(userService, {
-            firstName: 'auth-user',
-            lastName: 'test',
-            email: authUserEmail,
-            password: '1234',
-          }).then((user) => (authUser = user)),
           _createUser(userService, {
             firstName: 'one',
             lastName: 'test',

server/package.json

@@ -61,6 +61,7 @@
     "geo-tz": "^7.0.2",
     "handlebars": "^4.7.7",
     "i18n-iso-countries": "^7.5.0",
+    "jest-when": "^3.5.2",
     "joi": "^17.5.0",
     "local-reverse-geocoder": "^0.12.5",
     "lodash": "^4.17.21",
@@ -96,6 +97,7 @@
     "@types/fluent-ffmpeg": "^2.1.20",
     "@types/imagemin": "^8.0.0",
     "@types/jest": "27.0.2",
+    "@types/jest-when": "^3.5.2",
     "@types/lodash": "^4.14.178",
     "@types/multer": "^1.4.7",
     "@types/mv": "^2.1.2",
@@ -145,6 +147,7 @@
       "@app/database/config": "<rootDir>/libs/database/src/config",
       "@app/common": "<rootDir>/libs/common/src",
       "^@app/job(|/.*)$": "<rootDir>/libs/job/src/$1",
+      "@app/job": "<rootDir>/libs/job/src",
       "^@app/immich-config(|/.*)$": "<rootDir>/libs/immich-config/src/$1",
       "^@app/storage(|/.*)$": "<rootDir>/libs/storage/src/$1"
     }