feat: sync pictureFile with oidc if it isn't set already (#17397)

* feat: sync pictureFile with oidc if it isn't set already

fix: move picture writer to get userId

fix: move await promise to the top of the setPicure function before checking its value and automatically create the user folder

chore: code cleanup

* fix: extension double dot

---------

Co-authored-by: Jason Rasmussen <jason@rasm.me>
This commit is contained in:
Etienne 2025-04-11 20:00:39 +02:00 committed by GitHub
parent 08b5952c87
commit d7a782da34
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 133 additions and 5 deletions

View File

@ -63,6 +63,18 @@ export class OAuthRepository {
}
}
async getProfilePicture(url: string) {
const response = await fetch(url);
if (!response.ok) {
throw new Error(`Failed to fetch picture: ${response.statusText}`);
}
return {
data: await response.arrayBuffer(),
contentType: response.headers.get('content-type'),
};
}
private async getClient({
issuerUrl,
clientId,

View File

@ -7,15 +7,25 @@ import { AuthService } from 'src/services/auth.service';
import { UserMetadataItem } from 'src/types';
import { sharedLinkStub } from 'test/fixtures/shared-link.stub';
import { systemConfigStub } from 'test/fixtures/system-config.stub';
import { factory } from 'test/small.factory';
import { factory, newUuid } from 'test/small.factory';
import { newTestService, ServiceMocks } from 'test/utils';
const oauthResponse = ({ id, email, name }: { id: string; email: string; name: string }) => ({
const oauthResponse = ({
id,
email,
name,
profileImagePath,
}: {
id: string;
email: string;
name: string;
profileImagePath?: string;
}) => ({
accessToken: 'cmFuZG9tLWJ5dGVz',
userId: id,
userEmail: email,
name,
profileImagePath: '',
profileImagePath,
isAdmin: false,
shouldChangePassword: false,
});
@ -707,6 +717,58 @@ describe(AuthService.name, () => {
storageLabel: null,
});
});
it('should sync the profile picture', async () => {
const fileId = newUuid();
const user = factory.userAdmin({ oauthId: 'oauth-id' });
const pictureUrl = 'https://auth.immich.cloud/profiles/1.jpg';
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled);
mocks.oauth.getProfile.mockResolvedValue({
sub: user.oauthId,
email: user.email,
picture: pictureUrl,
});
mocks.user.getByOAuthId.mockResolvedValue(user);
mocks.crypto.randomUUID.mockReturnValue(fileId);
mocks.oauth.getProfilePicture.mockResolvedValue({
contentType: 'image/jpeg',
data: new Uint8Array([1, 2, 3, 4, 5]),
});
mocks.user.update.mockResolvedValue(user);
mocks.session.create.mockResolvedValue(factory.session());
await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual(
oauthResponse(user),
);
expect(mocks.user.update).toHaveBeenCalledWith(user.id, {
profileImagePath: `upload/profile/${user.id}/${fileId}.jpg`,
profileChangedAt: expect.any(Date),
});
expect(mocks.oauth.getProfilePicture).toHaveBeenCalledWith(pictureUrl);
});
it('should not sync the profile picture if the user already has one', async () => {
const user = factory.userAdmin({ oauthId: 'oauth-id', profileImagePath: 'not-empty' });
mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled);
mocks.oauth.getProfile.mockResolvedValue({
sub: user.oauthId,
email: user.email,
picture: 'https://auth.immich.cloud/profiles/1.jpg',
});
mocks.user.getByOAuthId.mockResolvedValue(user);
mocks.user.update.mockResolvedValue(user);
mocks.session.create.mockResolvedValue(factory.session());
await expect(sut.callback({ url: 'http://immich/auth/login?code=abc123' }, loginDetails)).resolves.toEqual(
oauthResponse(user),
);
expect(mocks.user.update).not.toHaveBeenCalled();
expect(mocks.oauth.getProfilePicture).not.toHaveBeenCalled();
});
});
describe('link', () => {

View File

@ -3,7 +3,9 @@ import { isString } from 'class-validator';
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 { UserAdmin } from 'src/database';
import { OnEvent } from 'src/decorators';
import {
@ -18,12 +20,12 @@ import {
mapLoginResponse,
} from 'src/dtos/auth.dto';
import { UserAdminResponseDto, mapUserAdmin } from 'src/dtos/user.dto';
import { AuthType, ImmichCookie, ImmichHeader, ImmichQuery, Permission } from 'src/enum';
import { AuthType, ImmichCookie, ImmichHeader, ImmichQuery, JobName, Permission, StorageFolder } 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';
export interface LoginDetails {
isSecure: boolean;
clientIp: string;
@ -239,9 +241,36 @@ export class AuthService extends BaseService {
});
}
if (!user.profileImagePath && profile.picture) {
await this.syncProfilePicture(user, profile.picture);
}
return this.createLoginResponse(user, loginDetails);
}
private async syncProfilePicture(user: UserAdmin, url: string) {
try {
const oldPath = user.profileImagePath;
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}`,
);
this.storageCore.ensureFolders(profileImagePath);
await this.storageRepository.createFile(profileImagePath, Buffer.from(data));
await this.userRepository.update(user.id, { profileImagePath, profileChangedAt: new Date() });
if (oldPath) {
await this.jobRepository.queue({ name: JobName.DELETE_FILES, data: { files: [oldPath] } });
}
} catch (error: Error | any) {
this.logger.warn(`Unable to sync oauth profile picture: ${error}`, error?.stack);
}
}
async link(auth: AuthDto, dto: OAuthCallbackDto): Promise<UserAdminResponseDto> {
const { oauth } = await this.getConfig({ withCache: false });
const { sub: oauthId } = await this.oauthRepository.getProfile(

View File

@ -101,6 +101,20 @@ describe('mimeTypes', () => {
});
}
describe('toExtension', () => {
it('should get an extension for a png file', () => {
expect(mimeTypes.toExtension('image/png')).toEqual('.png');
});
it('should get an extension for a jpeg file', () => {
expect(mimeTypes.toExtension('image/jpeg')).toEqual('.jpg');
});
it('should get an extension from a webp file', () => {
expect(mimeTypes.toExtension('image/webp')).toEqual('.webp');
});
});
describe('profile', () => {
it('should contain only lowercase mime types', () => {
const keys = Object.keys(mimeTypes.profile);

View File

@ -55,6 +55,10 @@ const image: Record<string, string[]> = {
'.webp': ['image/webp'],
};
const extensionOverrides: Record<string, string> = {
'image/jpeg': '.jpg',
};
/**
* list of supported image extensions from https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Image_types excluding svg
* @TODO share with the client
@ -104,6 +108,11 @@ const types = { ...image, ...video, ...sidecar };
const isType = (filename: string, r: Record<string, string[]>) => extname(filename).toLowerCase() in r;
const lookup = (filename: string) => types[extname(filename).toLowerCase()]?.[0] ?? 'application/octet-stream';
const toExtension = (mimeType: string) => {
return (
extensionOverrides[mimeType] || Object.entries(types).find(([, mimeTypes]) => mimeTypes.includes(mimeType))?.[0]
);
};
export const mimeTypes = {
image,
@ -120,6 +129,8 @@ export const mimeTypes = {
isVideo: (filename: string) => isType(filename, video),
isRaw: (filename: string) => isType(filename, raw),
lookup,
/** return an extension (including a leading `.`) for a mime-type */
toExtension,
assetType: (filename: string) => {
const contentType = lookup(filename);
if (contentType.startsWith('image/')) {