diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index 8e1c7ff2c4..3d79a88126 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -928,6 +928,7 @@ describe(AuthService.name, () => { const fileId = newUuid(); const user = UserFactory.create({ oauthId: 'oauth-id' }); const profile = OAuthProfileFactory.create({ picture: 'https://auth.immich.cloud/profiles/1.jpg' }); + const pictureBytes = new Uint8Array([1, 2, 3, 4, 5]); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ profile }); @@ -935,7 +936,7 @@ describe(AuthService.name, () => { mocks.crypto.randomUUID.mockReturnValue(fileId); mocks.oauth.getProfilePicture.mockResolvedValue({ contentType: 'image/jpeg', - data: new Uint8Array([1, 2, 3, 4, 5]).buffer, + data: pictureBytes.buffer, }); mocks.user.update.mockResolvedValue(user); mocks.session.create.mockResolvedValue(SessionFactory.create()); @@ -947,10 +948,41 @@ describe(AuthService.name, () => { ); expect(mocks.user.update).toHaveBeenCalledWith(user.id, { - profileImagePath: expect.stringContaining(`/data/profile/${user.id}/${fileId}.jpg`), + profileImagePath: expect.stringContaining(`/data/profile/${user.id}/${fileId}.webp`), profileChangedAt: expect.any(Date), }); expect(mocks.oauth.getProfilePicture).toHaveBeenCalledWith(profile.picture); + expect(mocks.media.generateThumbnail).toHaveBeenCalledWith( + Buffer.from(pictureBytes.buffer), + expect.objectContaining({ format: 'webp', processInvalidImages: false }), + expect.stringContaining(`/data/profile/${user.id}/${fileId}.webp`), + ); + }); + + it('should not update the user when thumbnail processing fails on the OAuth picture', async () => { + const user = UserFactory.create({ oauthId: 'oauth-id' }); + const profile = OAuthProfileFactory.create({ picture: 'https://auth.immich.cloud/profiles/1.jpg' }); + + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); + mocks.oauth.getProfile.mockResolvedValue(profile); + mocks.user.getByOAuthId.mockResolvedValue(user); + mocks.oauth.getProfilePicture.mockResolvedValue({ + contentType: 'text/html', + data: new Uint8Array([1, 2, 3, 4, 5]).buffer, + }); + mocks.media.generateThumbnail.mockRejectedValue(new Error('not an image')); + mocks.session.create.mockResolvedValue(SessionFactory.create()); + + await expect( + sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ), + ).resolves.toBeDefined(); + + expect(mocks.user.update).not.toHaveBeenCalled(); + expect(mocks.job.queue).not.toHaveBeenCalled(); }); it('should not sync the profile picture if the user already has one', async () => { diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index 1824b043ef..bbc4591477 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -2,9 +2,7 @@ import { BadRequestException, ForbiddenException, Injectable, UnauthorizedExcept import { parse } from 'cookie'; import { DateTime } from 'luxon'; import { IncomingHttpHeaders } from 'node:http'; -import { join } from 'node:path'; import { LOGIN_URL, MOBILE_REDIRECT, SALT_ROUNDS } from 'src/constants'; -import { StorageCore } from 'src/cores/storage.core'; import { AuthSharedLink, AuthUser, UserAdmin } from 'src/database'; import { AuthDto, @@ -23,12 +21,12 @@ import { mapLoginResponse, } from 'src/dtos/auth.dto'; import { UserAdminResponseDto, mapUserAdmin } from 'src/dtos/user.dto'; -import { AuthType, ImmichCookie, ImmichHeader, ImmichQuery, JobName, Permission, StorageFolder } from 'src/enum'; +import { AuthType, ImmichCookie, ImmichHeader, ImmichQuery, JobName, Permission } from 'src/enum'; import { OAuthProfile } from 'src/repositories/oauth.repository'; import { BaseService } from 'src/services/base.service'; import { isGranted } from 'src/utils/access'; import { HumanReadableSize } from 'src/utils/bytes'; -import { mimeTypes } from 'src/utils/mime-types'; +import { generateProfileImage } from 'src/utils/profile-image'; import { getUserAgentDetails } from 'src/utils/request'; export interface LoginDetails { isSecure: boolean; @@ -388,16 +386,16 @@ export class AuthService extends BaseService { private async syncProfilePicture(user: UserAdmin, url: string) { try { const oldPath = user.profileImagePath; + const { data } = await this.oauthRepository.getProfilePicture(url); - const { contentType, data } = await this.oauthRepository.getProfilePicture(url); - const extensionWithDot = mimeTypes.toExtension(contentType || 'image/jpeg') ?? 'jpg'; - const profileImagePath = join( - StorageCore.getFolderLocation(StorageFolder.Profile, user.id), - `${this.cryptoRepository.randomUUID()}${extensionWithDot}`, + const config = await this.getConfig({ withCache: true }); + const profileImagePath = await generateProfileImage( + { media: this.mediaRepository, crypto: this.cryptoRepository, storageCore: this.storageCore }, + config, + user.id, + Buffer.from(data), ); - this.storageCore.ensureFolders(profileImagePath); - await this.storageRepository.createFile(profileImagePath, Buffer.from(data)); await this.userRepository.update(user.id, { profileImagePath, profileChangedAt: new Date() }); if (oldPath) { diff --git a/server/src/services/user.service.spec.ts b/server/src/services/user.service.spec.ts index 0dc83928fc..847f96cfc6 100644 --- a/server/src/services/user.service.spec.ts +++ b/server/src/services/user.service.spec.ts @@ -113,20 +113,34 @@ describe(UserService.name, () => { await expect(sut.createProfileImage(authStub.admin, file)).rejects.toThrowError(InternalServerErrorException); }); - it('should delete the previous profile image', async () => { + it('should throw BadRequestException and clean up raw upload when thumbnail processing fails', async () => { + const file = { path: '/profile/path' } as Express.Multer.File; + const user = UserFactory.create({ profileImagePath: '/path/to/profile.jpg' }); + + mocks.user.get.mockResolvedValue(user); + mocks.media.generateThumbnail.mockRejectedValue(new Error('not an image')); + + await expect(sut.createProfileImage(authStub.admin, file)).rejects.toThrowError(BadRequestException); + + expect(mocks.user.update).not.toHaveBeenCalled(); + expect(mocks.job.queue.mock.calls).toEqual([[{ name: JobName.FileDelete, data: { files: [file.path] } }]]); + }); + + it('should delete the raw upload and the previous profile image', async () => { const user = UserFactory.create({ profileImagePath: '/path/to/profile.jpg' }); const file = { path: '/profile/path' } as Express.Multer.File; - const files = [user.profileImagePath]; mocks.user.get.mockResolvedValue(user); mocks.user.update.mockResolvedValue({ ...userStub.admin, profileImagePath: file.path }); await sut.createProfileImage(authStub.admin, file); - expect(mocks.job.queue.mock.calls).toEqual([[{ name: JobName.FileDelete, data: { files } }]]); + expect(mocks.job.queue.mock.calls).toEqual([ + [{ name: JobName.FileDelete, data: { files: [file.path, user.profileImagePath] } }], + ]); }); - it('should not delete the profile image if it has not been set', async () => { + it('should delete only the raw upload if no previous profile image is set', async () => { const file = { path: '/profile/path' } as Express.Multer.File; mocks.user.get.mockResolvedValue(userStub.admin); @@ -134,7 +148,7 @@ describe(UserService.name, () => { await sut.createProfileImage(authStub.admin, file); - expect(mocks.job.queue).not.toHaveBeenCalled(); + expect(mocks.job.queue.mock.calls).toEqual([[{ name: JobName.FileDelete, data: { files: [file.path] } }]]); expect(mocks.job.queueAll).not.toHaveBeenCalled(); }); }); @@ -192,6 +206,19 @@ describe(UserService.name, () => { expect(mocks.user.get).toHaveBeenCalledWith(user.id, {}); }); + + it('should return the profile picture with the content-type matching the stored file', async () => { + const user = UserFactory.create({ profileImagePath: '/path/to/profile.webp' }); + mocks.user.get.mockResolvedValue(user); + + await expect(sut.getProfileImage(user.id)).resolves.toEqual( + new ImmichFileResponse({ + path: '/path/to/profile.webp', + contentType: 'image/webp', + cacheControl: CacheControl.None, + }), + ); + }); }); describe('handleQueueUserDelete', () => { diff --git a/server/src/services/user.service.ts b/server/src/services/user.service.ts index 9fb1f45e54..8e1f74bcf4 100644 --- a/server/src/services/user.service.ts +++ b/server/src/services/user.service.ts @@ -16,7 +16,9 @@ import { UserTable } from 'src/schema/tables/user.table'; import { BaseService } from 'src/services/base.service'; import { JobOf, UserMetadataItem } from 'src/types'; import { ImmichFileResponse } from 'src/utils/file'; +import { mimeTypes } from 'src/utils/mime-types'; import { getPreferences, getPreferencesPartial, mergePreferences } from 'src/utils/preferences'; +import { generateProfileImage } from 'src/utils/profile-image'; @Injectable() export class UserService extends BaseService { @@ -91,16 +93,29 @@ export class UserService extends BaseService { } async createProfileImage(auth: AuthDto, file: Express.Multer.File): Promise { - const { profileImagePath: oldpath } = await this.findOrFail(auth.user.id, { withDeleted: false }); + const { profileImagePath: oldPath } = await this.findOrFail(auth.user.id, { withDeleted: false }); + + let profileImagePath: string; + try { + const config = await this.getConfig({ withCache: true }); + profileImagePath = await generateProfileImage( + { media: this.mediaRepository, crypto: this.cryptoRepository, storageCore: this.storageCore }, + config, + auth.user.id, + file.path, + ); + } catch (error) { + await this.jobRepository.queue({ name: JobName.FileDelete, data: { files: [file.path] } }); + throw new BadRequestException('Unable to process profile image', { cause: error }); + } const user = await this.userRepository.update(auth.user.id, { - profileImagePath: file.path, + profileImagePath, profileChangedAt: new Date(), }); - if (oldpath !== '') { - await this.jobRepository.queue({ name: JobName.FileDelete, data: { files: [oldpath] } }); - } + const toDelete = [file.path, ...(oldPath ? [oldPath] : [])]; + await this.jobRepository.queue({ name: JobName.FileDelete, data: { files: toDelete } }); return { userId: user.id, @@ -126,7 +141,7 @@ export class UserService extends BaseService { return new ImmichFileResponse({ path: user.profileImagePath, - contentType: 'image/jpeg', + contentType: mimeTypes.lookup(user.profileImagePath), cacheControl: CacheControl.None, }); } diff --git a/server/src/utils/profile-image.ts b/server/src/utils/profile-image.ts new file mode 100644 index 0000000000..ee94dd8986 --- /dev/null +++ b/server/src/utils/profile-image.ts @@ -0,0 +1,40 @@ +import { join } from 'node:path'; +import { SystemConfig } from 'src/config'; +import { StorageCore } from 'src/cores/storage.core'; +import { StorageFolder } from 'src/enum'; +import { CryptoRepository } from 'src/repositories/crypto.repository'; +import { MediaRepository } from 'src/repositories/media.repository'; + +type Repos = { + media: MediaRepository; + crypto: CryptoRepository; + storageCore: StorageCore; +}; + +export const generateProfileImage = async ( + { media, crypto, storageCore }: Repos, + { image }: SystemConfig, + userId: string, + input: string | Buffer, +): Promise => { + const outputPath = join( + StorageCore.getFolderLocation(StorageFolder.Profile, userId), + `${crypto.randomUUID()}.${image.thumbnail.format}`, + ); + storageCore.ensureFolders(outputPath); + + await media.generateThumbnail( + input, + { + colorspace: image.colorspace, + format: image.thumbnail.format, + quality: image.thumbnail.quality, + progressive: image.thumbnail.progressive, + size: image.thumbnail.size, + processInvalidImages: false, + }, + outputPath, + ); + + return outputPath; +};