fix: run profile picture through thumbnail pipeline (#27890)

* fix: run profile picture through thumbnail pipeline

* fix: format

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 () => {

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) {

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', () => {

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<CreateProfileImageResponseDto> {
-    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,
     });
   }

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<string> => {
+  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;
+};