From 431536cdbb8c641693b171a93ebecaf29610bf2e Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Mon, 30 Oct 2023 17:02:36 -0400 Subject: [PATCH] refactor(server): user core (#4722) --- server/src/domain/auth/auth.service.ts | 22 ++++++------ server/src/domain/user/user.core.ts | 40 ++------------------- server/src/domain/user/user.service.spec.ts | 17 ++++----- server/src/domain/user/user.service.ts | 25 +++++++------ 4 files changed, 35 insertions(+), 69 deletions(-) diff --git a/server/src/domain/auth/auth.service.ts b/server/src/domain/auth/auth.service.ts index 1b7a4037a9..5b65909fa4 100644 --- a/server/src/domain/auth/auth.service.ts +++ b/server/src/domain/auth/auth.service.ts @@ -23,7 +23,7 @@ import { IUserTokenRepository, } from '../repositories'; import { SystemConfigCore } from '../system-config/system-config.core'; -import { UserCore, UserResponseDto } from '../user'; +import { UserCore, UserResponseDto, mapUser } from '../user'; import { AuthType, IMMICH_ACCESS_COOKIE, @@ -73,7 +73,7 @@ export class AuthService { @Inject(ICryptoRepository) private cryptoRepository: ICryptoRepository, @Inject(ISystemConfigRepository) configRepository: ISystemConfigRepository, @Inject(ILibraryRepository) libraryRepository: ILibraryRepository, - @Inject(IUserRepository) userRepository: IUserRepository, + @Inject(IUserRepository) private userRepository: IUserRepository, @Inject(IUserTokenRepository) private userTokenRepository: IUserTokenRepository, @Inject(ISharedLinkRepository) private sharedLinkRepository: ISharedLinkRepository, @Inject(IKeyRepository) private keyRepository: IKeyRepository, @@ -91,7 +91,7 @@ export class AuthService { throw new UnauthorizedException('Password login has been disabled'); } - let user = await this.userCore.getByEmail(dto.email, true); + let user = await this.userRepository.getByEmail(dto.email, true); if (user) { const isAuthenticated = this.validatePassword(dto.password, user); if (!isAuthenticated) { @@ -120,7 +120,7 @@ export class AuthService { async changePassword(authUser: AuthUserDto, dto: ChangePasswordDto) { const { password, newPassword } = dto; - const user = await this.userCore.getByEmail(authUser.email, true); + const user = await this.userRepository.getByEmail(authUser.email, true); if (!user) { throw new UnauthorizedException(); } @@ -134,7 +134,7 @@ export class AuthService { } async adminSignUp(dto: SignUpDto): Promise { - const adminUser = await this.userCore.getAdmin(); + const adminUser = await this.userRepository.getAdmin(); if (adminUser) { throw new BadRequestException('The server already has an admin'); @@ -243,13 +243,13 @@ export class AuthService { const config = await this.configCore.getConfig(); const profile = await this.getOAuthProfile(config, dto.url); this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`); - let user = await this.userCore.getByOAuthId(profile.sub); + let user = await this.userRepository.getByOAuthId(profile.sub); // link existing user if (!user) { - const emailUser = await this.userCore.getByEmail(profile.email); + const emailUser = await this.userRepository.getByEmail(profile.email); if (emailUser) { - user = await this.userCore.updateUser(emailUser, emailUser.id, { oauthId: profile.sub }); + user = await this.userRepository.update(emailUser.id, { oauthId: profile.sub }); } } @@ -285,16 +285,16 @@ export class AuthService { async link(user: AuthUserDto, dto: OAuthCallbackDto): Promise { const config = await this.configCore.getConfig(); const { sub: oauthId } = await this.getOAuthProfile(config, dto.url); - const duplicate = await this.userCore.getByOAuthId(oauthId); + const duplicate = await this.userRepository.getByOAuthId(oauthId); if (duplicate && duplicate.id !== user.id) { this.logger.warn(`OAuth link account failed: sub is already linked to another user (${duplicate.email}).`); throw new BadRequestException('This OAuth account has already been linked to another user.'); } - return this.userCore.updateUser(user, user.id, { oauthId }); + return mapUser(await this.userRepository.update(user.id, { oauthId })); } async unlink(user: AuthUserDto): Promise { - return this.userCore.updateUser(user, user.id, { oauthId: '' }); + return mapUser(await this.userRepository.update(user.id, { oauthId: '' })); } private async getLogoutEndpoint(authType: AuthType): Promise { diff --git a/server/src/domain/user/user.core.ts b/server/src/domain/user/user.core.ts index d6cb35b411..7ed550a784 100644 --- a/server/src/domain/user/user.core.ts +++ b/server/src/domain/user/user.core.ts @@ -1,17 +1,9 @@ import { LibraryType, UserEntity } from '@app/infra/entities'; -import { - BadRequestException, - ForbiddenException, - InternalServerErrorException, - Logger, - NotFoundException, -} from '@nestjs/common'; -import { ReadStream, constants, createReadStream } from 'fs'; -import fs from 'fs/promises'; +import { BadRequestException, ForbiddenException, InternalServerErrorException, Logger } from '@nestjs/common'; import path from 'path'; import sanitize from 'sanitize-filename'; import { AuthUserDto } from '../auth'; -import { ICryptoRepository, ILibraryRepository, IUserRepository, UserListFilter } from '../repositories'; +import { ICryptoRepository, ILibraryRepository, IUserRepository } from '../repositories'; const SALT_ROUNDS = 10; @@ -131,34 +123,6 @@ export class UserCore { } } - async get(userId: string, withDeleted?: boolean): Promise { - return this.userRepository.get(userId, withDeleted); - } - - async getAdmin(): Promise { - return this.userRepository.getAdmin(); - } - - async getByEmail(email: string, withPassword?: boolean): Promise { - return this.userRepository.getByEmail(email, withPassword); - } - - async getByOAuthId(oauthId: string): Promise { - return this.userRepository.getByOAuthId(oauthId); - } - - async getUserProfileImage(user: UserEntity): Promise { - if (!user.profileImagePath) { - throw new NotFoundException('User does not have a profile image'); - } - await fs.access(user.profileImagePath, constants.R_OK); - return createReadStream(user.profileImagePath); - } - - async getList(filter?: UserListFilter): Promise { - return this.userRepository.getList(filter); - } - async createProfileImage(authUser: AuthUserDto, filePath: string): Promise { try { return this.userRepository.update(authUser.id, { profileImagePath: filePath }); diff --git a/server/src/domain/user/user.service.spec.ts b/server/src/domain/user/user.service.spec.ts index 16cf68320c..e78faedb1b 100644 --- a/server/src/domain/user/user.service.spec.ts +++ b/server/src/domain/user/user.service.spec.ts @@ -149,9 +149,7 @@ 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(adminUser.id, undefined).mockResolvedValue(adminUser); when(userMock.get).calledWith(immichUser.id).mockResolvedValue(immichUser); - when(userMock.get).calledWith(immichUser.id, undefined).mockResolvedValue(immichUser); }); describe('getAll', () => { @@ -207,7 +205,7 @@ describe(UserService.name, () => { const response = await sut.getMe(adminUser); - expect(userMock.get).toHaveBeenCalledWith(adminUser.id, undefined); + expect(userMock.get).toHaveBeenCalledWith(adminUser.id); expect(response).toEqual(adminUserResponse); }); @@ -216,7 +214,7 @@ describe(UserService.name, () => { await expect(sut.getMe(adminUser)).rejects.toBeInstanceOf(BadRequestException); - expect(userMock.get).toHaveBeenCalledWith(adminUser.id, undefined); + expect(userMock.get).toHaveBeenCalledWith(adminUser.id); }); }); @@ -256,7 +254,7 @@ describe(UserService.name, () => { it('user can only update its information', async () => { when(userMock.get) - .calledWith('not_immich_auth_user_id', undefined) + .calledWith('not_immich_auth_user_id') .mockResolvedValueOnce({ ...immichUser, id: 'not_immich_auth_user_id', @@ -321,7 +319,7 @@ describe(UserService.name, () => { }); it('update user information should throw error if user not found', async () => { - when(userMock.get).calledWith(immichUser.id, undefined).mockResolvedValueOnce(null); + when(userMock.get).calledWith(immichUser.id).mockResolvedValueOnce(null); const result = sut.update(adminUser, { id: immichUser.id, @@ -334,7 +332,6 @@ describe(UserService.name, () => { it('should let the admin update himself', async () => { const dto = { id: adminUser.id, shouldChangePassword: true, isAdmin: true }; - when(userMock.get).calledWith(adminUser.id).mockResolvedValueOnce(null); when(userMock.update).calledWith(adminUser.id, dto).mockResolvedValueOnce(adminUser); await sut.update(adminUser, dto); @@ -398,7 +395,7 @@ describe(UserService.name, () => { userMock.delete.mockResolvedValue(immichUser); await expect(sut.delete(adminUserAuth, immichUser.id)).resolves.toEqual(mapUser(immichUser)); - expect(userMock.get).toHaveBeenCalledWith(immichUser.id, undefined); + expect(userMock.get).toHaveBeenCalledWith(immichUser.id); expect(userMock.delete).toHaveBeenCalledWith(immichUser); }); }); @@ -466,7 +463,7 @@ describe(UserService.name, () => { await expect(sut.getProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(NotFoundException); - expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id, undefined); + expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id); }); it('should throw an error if the user does not have a picture', async () => { @@ -474,7 +471,7 @@ describe(UserService.name, () => { await expect(sut.getProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(NotFoundException); - expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id, undefined); + expect(userMock.get).toHaveBeenCalledWith(adminUserAuth.id); }); }); diff --git a/server/src/domain/user/user.service.ts b/server/src/domain/user/user.service.ts index df58276f76..3e4c79ab67 100644 --- a/server/src/domain/user/user.service.ts +++ b/server/src/domain/user/user.service.ts @@ -1,7 +1,8 @@ import { UserEntity } from '@app/infra/entities'; import { BadRequestException, Inject, Injectable, Logger, NotFoundException } from '@nestjs/common'; import { randomBytes } from 'crypto'; -import { ReadStream } from 'fs'; +import { ReadStream, constants, createReadStream } from 'fs'; +import fs from 'fs/promises'; import { AuthUserDto } from '../auth'; import { IEntityJob, JobName } from '../job'; import { @@ -36,12 +37,12 @@ export class UserService { } async getAll(authUser: AuthUserDto, isAll: boolean): Promise { - const users = await this.userCore.getList({ withDeleted: !isAll }); + const users = await this.userRepository.getList({ withDeleted: !isAll }); return users.map(mapUser); } async get(userId: string, withDeleted = false): Promise { - const user = await this.userCore.get(userId, withDeleted); + const user = await this.userRepository.get(userId, withDeleted); if (!user) { throw new NotFoundException('User not found'); } @@ -50,7 +51,7 @@ export class UserService { } async getMe(authUser: AuthUserDto): Promise { - const user = await this.userCore.get(authUser.id); + const user = await this.userRepository.get(authUser.id); if (!user) { throw new BadRequestException('User not found'); } @@ -63,7 +64,7 @@ export class UserService { } async update(authUser: AuthUserDto, dto: UpdateUserDto): Promise { - const user = await this.userCore.get(dto.id); + const user = await this.userRepository.get(dto.id); if (!user) { throw new NotFoundException('User not found'); } @@ -73,7 +74,7 @@ export class UserService { } async delete(authUser: AuthUserDto, userId: string): Promise { - const user = await this.userCore.get(userId); + const user = await this.userRepository.get(userId); if (!user) { throw new BadRequestException('User not found'); } @@ -83,7 +84,7 @@ export class UserService { } async restore(authUser: AuthUserDto, userId: string): Promise { - const user = await this.userCore.get(userId, true); + const user = await this.userRepository.get(userId, true); if (!user) { throw new BadRequestException('User not found'); } @@ -101,15 +102,19 @@ export class UserService { } async getProfileImage(userId: string): Promise { - const user = await this.userCore.get(userId); + const user = await this.userRepository.get(userId); if (!user) { throw new NotFoundException('User not found'); } - return this.userCore.getUserProfileImage(user); + if (!user.profileImagePath) { + throw new NotFoundException('User does not have a profile image'); + } + await fs.access(user.profileImagePath, constants.R_OK); + return createReadStream(user.profileImagePath); } async resetAdminPassword(ask: (admin: UserResponseDto) => Promise) { - const admin = await this.userCore.getAdmin(); + const admin = await this.userRepository.getAdmin(); if (!admin) { throw new BadRequestException('Admin account does not exist'); }