From 044257531e8cab8fbdc51d9d3541360341bb6777 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 19 Mar 2026 12:41:20 -0400 Subject: [PATCH 001/287] fix(server): fallback to email when name is empty (#27016) --- server/src/schema/tables/user.table.ts | 3 +- server/src/services/auth.service.spec.ts | 382 ++++++++---------- server/src/services/auth.service.ts | 18 +- .../test/factories/oauth-profile.factory.ts | 28 ++ server/test/factories/types.ts | 2 + .../specs/services/user.service.spec.ts | 6 +- 6 files changed, 208 insertions(+), 231 deletions(-) create mode 100644 server/test/factories/oauth-profile.factory.ts diff --git a/server/src/schema/tables/user.table.ts b/server/src/schema/tables/user.table.ts index 3a340d976b..0839924d2a 100644 --- a/server/src/schema/tables/user.table.ts +++ b/server/src/schema/tables/user.table.ts @@ -64,8 +64,9 @@ export class UserTable { @Column({ unique: true, nullable: true, default: null }) storageLabel!: string | null; + // TODO remove default, make nullable, and convert empty spaces to null @Column({ default: '' }) - name!: Generated; + name!: string; @Column({ type: 'bigint', nullable: true }) quotaSizeInBytes!: ColumnType | null; diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index f2cc3ada95..2c4b31c83a 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -8,6 +8,7 @@ import { AuthService } from 'src/services/auth.service'; import { UserMetadataItem } from 'src/types'; import { ApiKeyFactory } from 'test/factories/api-key.factory'; import { AuthFactory } from 'test/factories/auth.factory'; +import { OAuthProfileFactory } from 'test/factories/oauth-profile.factory'; import { SessionFactory } from 'test/factories/session.factory'; import { UserFactory } from 'test/factories/user.factory'; import { sharedLinkStub } from 'test/fixtures/shared-link.stub'; @@ -15,31 +16,7 @@ import { systemConfigStub } from 'test/fixtures/system-config.stub'; import { newUuid } from 'test/small.factory'; import { newTestService, ServiceMocks } from 'test/utils'; -const oauthResponse = ({ - id, - email, - name, - profileImagePath, -}: { - id: string; - email: string; - name: string; - profileImagePath?: string; -}) => ({ - accessToken: 'cmFuZG9tLWJ5dGVz', - userId: id, - userEmail: email, - name, - profileImagePath, - isAdmin: false, - isOnboarded: false, - shouldChangePassword: false, -}); - -// const token = Buffer.from('my-api-key', 'utf8').toString('base64'); - const email = 'test@immich.com'; -const sub = 'my-auth-user-sub'; const loginDetails = { isSecure: true, clientIp: '127.0.0.1', @@ -48,11 +25,9 @@ const loginDetails = { appVersion: null, }; -const fixtures = { - login: { - email, - password: 'password', - }, +const dto = { + email, + password: 'password', }; describe(AuthService.name, () => { @@ -63,7 +38,6 @@ describe(AuthService.name, () => { ({ sut, mocks } = newTestService(AuthService)); mocks.oauth.authorize.mockResolvedValue({ url: 'http://test', state: 'state', codeVerifier: 'codeVerifier' }); - mocks.oauth.getProfile.mockResolvedValue({ sub, email }); mocks.oauth.getLogoutEndpoint.mockResolvedValue('http://end-session-endpoint'); }); @@ -75,13 +49,13 @@ describe(AuthService.name, () => { it('should throw an error if password login is disabled', async () => { mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.disabled); - await expect(sut.login(fixtures.login, loginDetails)).rejects.toBeInstanceOf(UnauthorizedException); + await expect(sut.login(dto, loginDetails)).rejects.toBeInstanceOf(UnauthorizedException); }); it('should check the user exists', async () => { mocks.user.getByEmail.mockResolvedValue(void 0); - await expect(sut.login(fixtures.login, loginDetails)).rejects.toBeInstanceOf(UnauthorizedException); + await expect(sut.login(dto, loginDetails)).rejects.toBeInstanceOf(UnauthorizedException); expect(mocks.user.getByEmail).toHaveBeenCalledTimes(1); }); @@ -89,7 +63,7 @@ describe(AuthService.name, () => { it('should check the user has a password', async () => { mocks.user.getByEmail.mockResolvedValue({} as UserAdmin); - await expect(sut.login(fixtures.login, loginDetails)).rejects.toBeInstanceOf(UnauthorizedException); + await expect(sut.login(dto, loginDetails)).rejects.toBeInstanceOf(UnauthorizedException); expect(mocks.user.getByEmail).toHaveBeenCalledTimes(1); }); @@ -100,7 +74,7 @@ describe(AuthService.name, () => { mocks.user.getByEmail.mockResolvedValue(user); mocks.session.create.mockResolvedValue(session); - await expect(sut.login(fixtures.login, loginDetails)).resolves.toEqual({ + await expect(sut.login(dto, loginDetails)).resolves.toEqual({ accessToken: 'cmFuZG9tLWJ5dGVz', userId: user.id, userEmail: user.email, @@ -624,6 +598,7 @@ describe(AuthService.name, () => { it('should not allow auto registering', async () => { mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); mocks.user.getByEmail.mockResolvedValue(void 0); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create()); await expect( sut.callback( @@ -638,31 +613,31 @@ describe(AuthService.name, () => { it('should link an existing user', async () => { const user = UserFactory.create(); + const profile = OAuthProfileFactory.create(); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthEnabled); + mocks.oauth.getProfile.mockResolvedValue(profile); mocks.user.getByEmail.mockResolvedValue(user); mocks.user.update.mockResolvedValue(user); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foobar' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foobar' }, + {}, + loginDetails, + ); expect(mocks.user.getByEmail).toHaveBeenCalledTimes(1); - expect(mocks.user.update).toHaveBeenCalledWith(user.id, { oauthId: sub }); + expect(mocks.user.update).toHaveBeenCalledWith(user.id, { oauthId: profile.sub }); }); it('should not link to a user with a different oauth sub', async () => { - const user = UserFactory.create({ isAdmin: true, oauthId: 'existing-sub' }); + const user = UserFactory.create({ oauthId: 'existing-sub' }); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create()); mocks.user.getByEmail.mockResolvedValueOnce(user); - mocks.user.getAdmin.mockResolvedValue(user); - mocks.user.create.mockResolvedValue(user); + mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); await expect( sut.callback( @@ -677,35 +652,30 @@ describe(AuthService.name, () => { }); it('should allow auto registering by default', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); mocks.user.getByEmail.mockResolvedValue(void 0); mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); - mocks.user.create.mockResolvedValue(user); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create()); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foobar' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foobar' }, + {}, + loginDetails, + ); expect(mocks.user.getByEmail).toHaveBeenCalledTimes(2); // second call is for domain check before create expect(mocks.user.create).toHaveBeenCalledTimes(1); }); it('should throw an error if user should be auto registered but the email claim does not exist', async () => { - const user = UserFactory.create({ isAdmin: true }); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); mocks.user.getByEmail.mockResolvedValue(void 0); - mocks.user.getAdmin.mockResolvedValue(user); - mocks.user.create.mockResolvedValue(user); + mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); + mocks.user.create.mockResolvedValue(UserFactory.create()); mocks.session.create.mockResolvedValue(SessionFactory.create()); - mocks.oauth.getProfile.mockResolvedValue({ sub, email: undefined }); + mocks.oauth.getProfile.mockResolvedValue({ sub: 'sub' }); await expect( sut.callback( @@ -725,10 +695,9 @@ describe(AuthService.name, () => { 'app.immich:///oauth-callback?code=abc123', ]) { it(`should use the mobile redirect override for a url of ${url}`, async () => { - const user = UserFactory.create(); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithMobileOverride); - mocks.user.getByOAuthId.mockResolvedValue(user); + mocks.user.getByOAuthId.mockResolvedValue(UserFactory.create()); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create()); mocks.session.create.mockResolvedValue(SessionFactory.create()); await sut.callback({ url, state: 'xyz789', codeVerifier: 'foo' }, {}, loginDetails); @@ -743,135 +712,136 @@ describe(AuthService.name, () => { } it('should use the default quota', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota); mocks.user.getByEmail.mockResolvedValue(void 0); mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); - mocks.user.create.mockResolvedValue(user); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create()); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 1_073_741_824 })); }); - it('should ignore an invalid storage quota', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota); - mocks.oauth.getProfile.mockResolvedValue({ sub: user.oauthId, email: user.email, immich_quota: 'abc' }); - mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); + it('should infer name from given and family names', async () => { + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); + mocks.oauth.getProfile.mockResolvedValue( + OAuthProfileFactory.create({ name: undefined, given_name: 'Given', family_name: 'Family' }), + ); mocks.user.getByEmail.mockResolvedValue(void 0); - mocks.user.create.mockResolvedValue(user); + mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); + mocks.user.create.mockResolvedValue(UserFactory.create()); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); + + expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ name: 'Given Family' })); + }); + + it('should fallback to email when no username is provided', async () => { + const profile = OAuthProfileFactory.create({ name: undefined, given_name: undefined, family_name: undefined }); + + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); + mocks.oauth.getProfile.mockResolvedValue(profile); + mocks.user.getByEmail.mockResolvedValue(void 0); + mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); + mocks.user.create.mockResolvedValue(UserFactory.create()); + mocks.session.create.mockResolvedValue(SessionFactory.create()); + + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); + + expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ name: profile.email })); + }); + + it('should ignore an invalid storage quota', async () => { + mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create({ immich_quota: 'abc' })); + mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); + mocks.user.getByEmail.mockResolvedValue(void 0); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); + mocks.session.create.mockResolvedValue(SessionFactory.create()); + + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 1_073_741_824 })); }); it('should ignore a negative quota', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota); - mocks.oauth.getProfile.mockResolvedValue({ sub: user.oauthId, email: user.email, immich_quota: -5 }); - mocks.user.getAdmin.mockResolvedValue(user); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create({ immich_quota: -5 })); + mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); mocks.user.getByEmail.mockResolvedValue(void 0); - mocks.user.create.mockResolvedValue(user); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 1_073_741_824 })); }); it('should set quota for 0 quota', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota); - mocks.oauth.getProfile.mockResolvedValue({ sub: user.oauthId, email: user.email, immich_quota: 0 }); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create({ immich_quota: 0 })); mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); mocks.user.getByEmail.mockResolvedValue(void 0); - mocks.user.create.mockResolvedValue(user); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); - expect(mocks.user.create).toHaveBeenCalledWith({ - email: user.email, - isAdmin: false, - name: ' ', - oauthId: user.oauthId, - quotaSizeInBytes: 0, - storageLabel: null, - }); + expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 0 })); }); it('should use a valid storage quota', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota); - mocks.oauth.getProfile.mockResolvedValue({ sub: user.oauthId, email: user.email, immich_quota: 5 }); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create({ immich_quota: 5 })); mocks.user.getByEmail.mockResolvedValue(void 0); mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); mocks.user.getByOAuthId.mockResolvedValue(void 0); - mocks.user.create.mockResolvedValue(user); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); - expect(mocks.user.create).toHaveBeenCalledWith({ - email: user.email, - isAdmin: false, - name: ' ', - oauthId: user.oauthId, - quotaSizeInBytes: 5_368_709_120, - storageLabel: null, - }); + expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ quotaSizeInBytes: 5_368_709_120 })); }); it('should sync the profile picture', async () => { const fileId = newUuid(); const user = UserFactory.create({ oauthId: 'oauth-id' }); - const pictureUrl = 'https://auth.immich.cloud/profiles/1.jpg'; + const profile = OAuthProfileFactory.create({ picture: '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.oauth.getProfile.mockResolvedValue(profile); mocks.user.getByOAuthId.mockResolvedValue(user); mocks.crypto.randomUUID.mockReturnValue(fileId); mocks.oauth.getProfilePicture.mockResolvedValue({ @@ -881,131 +851,96 @@ describe(AuthService.name, () => { mocks.user.update.mockResolvedValue(user); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); expect(mocks.user.update).toHaveBeenCalledWith(user.id, { profileImagePath: expect.stringContaining(`/data/profile/${user.id}/${fileId}.jpg`), profileChangedAt: expect.any(Date), }); - expect(mocks.oauth.getProfilePicture).toHaveBeenCalledWith(pictureUrl); + expect(mocks.oauth.getProfilePicture).toHaveBeenCalledWith(profile.picture); }); it('should not sync the profile picture if the user already has one', async () => { const user = UserFactory.create({ 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.oauth.getProfile.mockResolvedValue( + OAuthProfileFactory.create({ + 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(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); expect(mocks.user.update).not.toHaveBeenCalled(); expect(mocks.oauth.getProfilePicture).not.toHaveBeenCalled(); }); it('should only allow "admin" and "user" for the role claim', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister); - mocks.oauth.getProfile.mockResolvedValue({ sub: user.oauthId, email: user.email, immich_role: 'foo' }); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create({ immich_role: 'foo' })); mocks.user.getByEmail.mockResolvedValue(void 0); mocks.user.getAdmin.mockResolvedValue(UserFactory.create({ isAdmin: true })); mocks.user.getByOAuthId.mockResolvedValue(void 0); - mocks.user.create.mockResolvedValue(user); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); - expect(mocks.user.create).toHaveBeenCalledWith({ - email: user.email, - name: ' ', - oauthId: user.oauthId, - quotaSizeInBytes: null, - storageLabel: null, - isAdmin: false, - }); + expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ isAdmin: false })); }); it('should create an admin user if the role claim is set to admin', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithAutoRegister); - mocks.oauth.getProfile.mockResolvedValue({ sub: user.oauthId, email: user.email, immich_role: 'admin' }); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create({ immich_role: 'admin' })); mocks.user.getByEmail.mockResolvedValue(void 0); mocks.user.getByOAuthId.mockResolvedValue(void 0); - mocks.user.create.mockResolvedValue(user); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); - expect(mocks.user.create).toHaveBeenCalledWith({ - email: user.email, - name: ' ', - oauthId: user.oauthId, - quotaSizeInBytes: null, - storageLabel: null, - isAdmin: true, - }); + expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ isAdmin: true })); }); it('should accept a custom role claim', async () => { - const user = UserFactory.create({ oauthId: 'oauth-id' }); - mocks.systemMetadata.get.mockResolvedValue({ - oauth: { ...systemConfigStub.oauthWithAutoRegister, roleClaim: 'my_role' }, + oauth: { ...systemConfigStub.oauthWithAutoRegister.oauth, roleClaim: 'my_role' }, }); - mocks.oauth.getProfile.mockResolvedValue({ sub: user.oauthId, email: user.email, my_role: 'admin' }); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create({ my_role: 'admin' })); mocks.user.getByEmail.mockResolvedValue(void 0); mocks.user.getByOAuthId.mockResolvedValue(void 0); - mocks.user.create.mockResolvedValue(user); + mocks.user.create.mockResolvedValue(UserFactory.create({ oauthId: 'oauth-id' })); mocks.session.create.mockResolvedValue(SessionFactory.create()); - await expect( - sut.callback( - { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, - {}, - loginDetails, - ), - ).resolves.toEqual(oauthResponse(user)); + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); - expect(mocks.user.create).toHaveBeenCalledWith({ - email: user.email, - name: ' ', - oauthId: user.oauthId, - quotaSizeInBytes: null, - storageLabel: null, - isAdmin: true, - }); + expect(mocks.user.create).toHaveBeenCalledWith(expect.objectContaining({ isAdmin: true })); }); }); @@ -1013,8 +948,10 @@ describe(AuthService.name, () => { it('should link an account', async () => { const user = UserFactory.create(); const auth = AuthFactory.from(user).apiKey({ permissions: [] }).build(); + const profile = OAuthProfileFactory.create(); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); + mocks.oauth.getProfile.mockResolvedValue(profile); mocks.user.update.mockResolvedValue(user); await sut.link( @@ -1023,7 +960,7 @@ describe(AuthService.name, () => { {}, ); - expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: sub }); + expect(mocks.user.update).toHaveBeenCalledWith(auth.user.id, { oauthId: profile.sub }); }); it('should not link an already linked oauth.sub', async () => { @@ -1032,6 +969,7 @@ describe(AuthService.name, () => { const auth = { user: authUser, apiKey: authApiKey }; mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.enabled); + mocks.oauth.getProfile.mockResolvedValue(OAuthProfileFactory.create()); mocks.user.getByOAuthId.mockResolvedValue({ id: 'other-user' } as UserAdmin); await expect( diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index b8e1b78107..5932855a21 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -261,6 +261,11 @@ export class AuthService extends BaseService { } async callback(dto: OAuthCallbackDto, headers: IncomingHttpHeaders, loginDetails: LoginDetails) { + const { oauth } = await this.getConfig({ withCache: false }); + if (!oauth.enabled) { + throw new BadRequestException('OAuth is not enabled'); + } + const expectedState = dto.state ?? this.getCookieOauthState(headers); if (!expectedState?.length) { throw new BadRequestException('OAuth state is missing'); @@ -271,7 +276,6 @@ export class AuthService extends BaseService { throw new BadRequestException('OAuth code verifier is missing'); } - const { oauth } = await this.getConfig({ withCache: false }); const url = this.resolveRedirectUri(oauth, dto.url); const profile = await this.oauthRepository.getProfile(oauth, url, expectedState, codeVerifier); const { autoRegister, defaultStorageQuota, storageLabelClaim, storageQuotaClaim, roleClaim } = oauth; @@ -298,7 +302,8 @@ export class AuthService extends BaseService { throw new BadRequestException(`User does not exist and auto registering is disabled.`); } - if (!profile.email) { + const email = profile.email; + if (!email) { throw new BadRequestException('OAuth profile does not have an email address'); } @@ -320,10 +325,13 @@ export class AuthService extends BaseService { isValid: (value: unknown) => isString(value) && ['admin', 'user'].includes(value), }); - const userName = profile.name ?? `${profile.given_name || ''} ${profile.family_name || ''}`; user = await this.createUser({ - name: userName, - email: profile.email, + name: + profile.name || + `${profile.given_name || ''} ${profile.family_name || ''}`.trim() || + profile.preferred_username || + email, + email, oauthId: profile.sub, quotaSizeInBytes: storageQuota === null ? null : storageQuota * HumanReadableSize.GiB, storageLabel: storageLabel || null, diff --git a/server/test/factories/oauth-profile.factory.ts b/server/test/factories/oauth-profile.factory.ts new file mode 100644 index 0000000000..31a8e4ea77 --- /dev/null +++ b/server/test/factories/oauth-profile.factory.ts @@ -0,0 +1,28 @@ +import { OAuthProfile } from 'src/repositories/oauth.repository'; +import { OAuthProfileLike } from 'test/factories/types'; +import { newUuid } from 'test/small.factory'; + +export class OAuthProfileFactory { + private constructor(private value: OAuthProfile) {} + + static create(dto: OAuthProfileLike = {}) { + return OAuthProfileFactory.from(dto).build(); + } + + static from(dto: OAuthProfileLike = {}) { + const sub = newUuid(); + return new OAuthProfileFactory({ + sub, + name: 'Name', + given_name: 'Given', + family_name: 'Family', + email: `oauth-${sub}@immich.cloud`, + email_verified: true, + ...dto, + }); + } + + build() { + return { ...this.value }; + } +} diff --git a/server/test/factories/types.ts b/server/test/factories/types.ts index e2d9e4e1c3..5c8c8ee2c2 100644 --- a/server/test/factories/types.ts +++ b/server/test/factories/types.ts @@ -1,4 +1,5 @@ import { Selectable } from 'kysely'; +import { OAuthProfile } from 'src/repositories/oauth.repository'; import { ActivityTable } from 'src/schema/tables/activity.table'; import { AlbumUserTable } from 'src/schema/tables/album-user.table'; import { AlbumTable } from 'src/schema/tables/album.table'; @@ -34,3 +35,4 @@ export type PartnerLike = Partial>; export type ActivityLike = Partial>; export type ApiKeyLike = Partial>; export type SessionLike = Partial>; +export type OAuthProfileLike = Partial; diff --git a/server/test/medium/specs/services/user.service.spec.ts b/server/test/medium/specs/services/user.service.spec.ts index 24a06404b1..2250034eea 100644 --- a/server/test/medium/specs/services/user.service.spec.ts +++ b/server/test/medium/specs/services/user.service.spec.ts @@ -47,15 +47,15 @@ describe(UserService.name, () => { const { sut, ctx } = setup(); ctx.getMock(EventRepository).emit.mockResolvedValue(); const user = mediumFactory.userInsert(); - await expect(sut.createUser({ email: user.email })).resolves.toMatchObject({ email: user.email }); - await expect(sut.createUser({ email: user.email })).rejects.toThrow('User exists'); + await expect(sut.createUser({ name: 'Test', email: user.email })).resolves.toMatchObject({ email: user.email }); + await expect(sut.createUser({ name: 'Test', email: user.email })).rejects.toThrow('User exists'); }); it('should not return password', async () => { const { sut, ctx } = setup(); ctx.getMock(EventRepository).emit.mockResolvedValue(); const dto = mediumFactory.userInsert({ password: 'password' }); - const user = await sut.createUser({ email: dto.email, password: 'password' }); + const user = await sut.createUser({ name: 'Test', email: dto.email, password: 'password' }); expect((user as any).password).toBeUndefined(); }); }); From bc2439883a89634199aead5259f9a52d52334c56 Mon Sep 17 00:00:00 2001 From: bo0tzz Date: Thu, 19 Mar 2026 17:42:29 +0100 Subject: [PATCH 002/287] fix: track PR template auto-close with label (#27042) * fix: track PR template auto-close with label * fix: format --- .github/workflows/check-pr-template.yml | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check-pr-template.yml b/.github/workflows/check-pr-template.yml index 4dcdd20f72..5419fb27f7 100644 --- a/.github/workflows/check-pr-template.yml +++ b/.github/workflows/check-pr-template.yml @@ -6,6 +6,9 @@ on: permissions: {} +env: + LABEL_ID: 'LA_kwDOGyI-8M8AAAACcAeOfg' # auto-closed:template + jobs: parse: runs-on: ubuntu-latest @@ -46,9 +49,10 @@ jobs: run: | gh api graphql \ -f prId="$NODE_ID" \ + -f labelId="$LABEL_ID" \ -f body="This PR has been automatically closed as the description doesn't follow our template. After you edit it to match the template, the PR will automatically be reopened." \ -f query=' - mutation CommentAndClosePR($prId: ID!, $body: String!) { + mutation CommentAndClosePR($prId: ID!, $body: String!, $labelId: ID!) { addComment(input: { subjectId: $prId, body: $body @@ -60,21 +64,34 @@ jobs: }) { __typename } + addLabelsToLabelable(input: { + labelableId: $prId, + labelIds: [$labelId] + }) { + __typename + } }' - - name: Reopen PR (sections now present, PR closed) - if: ${{ needs.parse.outputs.uses_template == 'true' && github.event.pull_request.state == 'closed' }} + - name: Reopen PR (sections now present, PR was auto-closed) + if: ${{ needs.parse.outputs.uses_template == 'true' && github.event.pull_request.state == 'closed' && contains(github.event.pull_request.labels.*.node_id, env.LABEL_ID) }} env: GH_TOKEN: ${{ github.token }} NODE_ID: ${{ github.event.pull_request.node_id }} run: | gh api graphql \ -f prId="$NODE_ID" \ + -f labelId="$LABEL_ID" \ -f query=' - mutation ReopenPR($prId: ID!) { + mutation ReopenPR($prId: ID!, $labelId: ID!) { reopenPullRequest(input: { pullRequestId: $prId }) { __typename } + removeLabelsFromLabelable(input: { + labelableId: $prId, + labelIds: [$labelId] + }) { + __typename + } }' From b30373b24f0c447db079ec308b01b7c410574214 Mon Sep 17 00:00:00 2001 From: Mees Frensel <33722705+meesfrensel@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:45:11 +0100 Subject: [PATCH 003/287] fix: release rootles compose file (#27048) --- .github/workflows/prepare-release.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/prepare-release.yml b/.github/workflows/prepare-release.yml index 6030dc8752..e3e6659140 100644 --- a/.github/workflows/prepare-release.yml +++ b/.github/workflows/prepare-release.yml @@ -151,6 +151,7 @@ jobs: body_path: misc/release/notes.tmpl files: | docker/docker-compose.yml + docker/docker-compose.rootless.yml docker/example.env docker/hwaccel.ml.yml docker/hwaccel.transcoding.yml From 29000461c2793b2cbbc280e0de1d2530199fcc24 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 19 Mar 2026 12:55:43 -0400 Subject: [PATCH 004/287] fix: ignore errors deleting untitled album (#27020) --- web/src/lib/services/album.service.ts | 2 +- web/src/lib/utils/handle-error.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/web/src/lib/services/album.service.ts b/web/src/lib/services/album.service.ts index e0420f4341..6ccd67584e 100644 --- a/web/src/lib/services/album.service.ts +++ b/web/src/lib/services/album.service.ts @@ -273,7 +273,7 @@ export const handleDeleteAlbum = async (album: AlbumResponseDto, options?: { pro } return true; } catch (error) { - handleError(error, $t('errors.unable_to_delete_album')); + handleError(error, $t('errors.unable_to_delete_album'), { notify }); return false; } }; diff --git a/web/src/lib/utils/handle-error.ts b/web/src/lib/utils/handle-error.ts index 4ff35510c6..0817ab70e9 100644 --- a/web/src/lib/utils/handle-error.ts +++ b/web/src/lib/utils/handle-error.ts @@ -23,7 +23,8 @@ export function standardizeError(error: unknown) { return error instanceof Error ? error : new Error(String(error)); } -export function handleError(error: unknown, localizedMessage: string) { +export function handleError(error: unknown, localizedMessage: string, options?: { notify?: boolean }) { + const { notify = true } = options ?? {}; const standardizedError = standardizeError(error); if (standardizedError.name === 'AbortError') { return; @@ -39,7 +40,9 @@ export function handleError(error: unknown, localizedMessage: string) { const errorMessage = serverMessage || localizedMessage; - toastManager.danger(errorMessage); + if (notify) { + toastManager.danger(errorMessage); + } return errorMessage; } catch (error) { From 86311e3cfeaca948f8962872c0da3a6158d87b7b Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 19 Mar 2026 13:04:56 -0400 Subject: [PATCH 005/287] fix(web): wrap long album title (#27012) --- pnpm-lock.yaml | 10 ++-- web/package.json | 2 +- .../album-page/album-description.svelte | 6 +- .../components/album-page/album-title.svelte | 57 +++++++++---------- 4 files changed, 35 insertions(+), 40 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9d47ba73f8..4826666b24 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -747,8 +747,8 @@ importers: specifier: workspace:* version: link:../open-api/typescript-sdk '@immich/ui': - specifier: ^0.64.0 - version: 0.64.0(@sveltejs/kit@2.53.4(@opentelemetry/api@1.9.0)(@sveltejs/vite-plugin-svelte@6.2.4(svelte@5.53.7)(vite@7.3.1(@types/node@25.4.0)(jiti@2.6.1)(lightningcss@1.31.1)(sass@1.97.1)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)))(svelte@5.53.7)(typescript@5.9.3)(vite@7.3.1(@types/node@25.4.0)(jiti@2.6.1)(lightningcss@1.31.1)(sass@1.97.1)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)))(svelte@5.53.7) + specifier: ^0.65.3 + version: 0.65.3(@sveltejs/kit@2.53.4(@opentelemetry/api@1.9.0)(@sveltejs/vite-plugin-svelte@6.2.4(svelte@5.53.7)(vite@7.3.1(@types/node@25.4.0)(jiti@2.6.1)(lightningcss@1.31.1)(sass@1.97.1)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)))(svelte@5.53.7)(typescript@5.9.3)(vite@7.3.1(@types/node@25.4.0)(jiti@2.6.1)(lightningcss@1.31.1)(sass@1.97.1)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)))(svelte@5.53.7) '@mapbox/mapbox-gl-rtl-text': specifier: 0.3.0 version: 0.3.0 @@ -3035,8 +3035,8 @@ packages: peerDependencies: svelte: ^5.0.0 - '@immich/ui@0.64.0': - resolution: {integrity: sha512-jbPN1x9KAAcW18h4RO7skbFYjkR4Lg+mEVjSDzsPC2NBNzSi4IA0PIHhFEwnD5dk4OS7+UjRG8m5/QTyotrm4A==} + '@immich/ui@0.65.3': + resolution: {integrity: sha512-jMXzCzMNTcCdWXt9IUP7GkALE5oEvPQk/jCOuI2bfxsxCZFzMkUfUS+AV83Vg1vQ6l+g39PbKSPKBEzv125ATQ==} peerDependencies: svelte: ^5.0.0 @@ -14955,7 +14955,7 @@ snapshots: node-emoji: 2.2.0 svelte: 5.53.7 - '@immich/ui@0.64.0(@sveltejs/kit@2.53.4(@opentelemetry/api@1.9.0)(@sveltejs/vite-plugin-svelte@6.2.4(svelte@5.53.7)(vite@7.3.1(@types/node@25.4.0)(jiti@2.6.1)(lightningcss@1.31.1)(sass@1.97.1)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)))(svelte@5.53.7)(typescript@5.9.3)(vite@7.3.1(@types/node@25.4.0)(jiti@2.6.1)(lightningcss@1.31.1)(sass@1.97.1)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)))(svelte@5.53.7)': + '@immich/ui@0.65.3(@sveltejs/kit@2.53.4(@opentelemetry/api@1.9.0)(@sveltejs/vite-plugin-svelte@6.2.4(svelte@5.53.7)(vite@7.3.1(@types/node@25.4.0)(jiti@2.6.1)(lightningcss@1.31.1)(sass@1.97.1)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)))(svelte@5.53.7)(typescript@5.9.3)(vite@7.3.1(@types/node@25.4.0)(jiti@2.6.1)(lightningcss@1.31.1)(sass@1.97.1)(terser@5.44.1)(tsx@4.21.0)(yaml@2.8.2)))(svelte@5.53.7)': dependencies: '@immich/svelte-markdown-preprocess': 0.2.1(svelte@5.53.7) '@internationalized/date': 3.10.0 diff --git a/web/package.json b/web/package.json index 847d75a21a..3311db6acd 100644 --- a/web/package.json +++ b/web/package.json @@ -27,7 +27,7 @@ "@formatjs/icu-messageformat-parser": "^3.0.0", "@immich/justified-layout-wasm": "^0.4.3", "@immich/sdk": "workspace:*", - "@immich/ui": "^0.64.0", + "@immich/ui": "^0.65.3", "@mapbox/mapbox-gl-rtl-text": "0.3.0", "@mdi/js": "^7.4.47", "@photo-sphere-viewer/core": "^5.14.0", diff --git a/web/src/lib/components/album-page/album-description.svelte b/web/src/lib/components/album-page/album-description.svelte index 481e110fb0..9443392456 100644 --- a/web/src/lib/components/album-page/album-description.svelte +++ b/web/src/lib/components/album-page/album-description.svelte @@ -33,13 +33,11 @@ {#if isOwned}