From f3f56bb5e87baf792b668a5c17ff3f9d2a29889f Mon Sep 17 00:00:00 2001 From: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:55:00 +0100 Subject: [PATCH] fix: oauth regressions (#32934) --- server/src/dtos/auth.dto.ts | 34 ++++++++++++++++++------ server/src/services/auth.service.spec.ts | 29 +++++++++++++++++++- server/src/services/auth.service.ts | 10 +++---- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/server/src/dtos/auth.dto.ts b/server/src/dtos/auth.dto.ts index c50ac025d..c0ecea181 100644 --- a/server/src/dtos/auth.dto.ts +++ b/server/src/dtos/auth.dto.ts @@ -25,13 +25,31 @@ export const isMetadataHidden = (auth?: AuthDto) => !!auth?.sharedLink && !auth. export const isEditedOnly = (auth: AuthDto, edited = false) => !!auth.sharedLink || edited; -const storageLabelClaimSchema = z.string().transform((label) => sanitize(label)); -const quotaSizeInBytesSchema = z +const storageLabelClaimSchema = z + .string() + .optional() + .transform((label) => { + if (label === undefined) { + return; + } + + return label ? sanitize(label) : null; + }) + .catch(undefined); +const quotaSizeInBytesSchema = z.coerce + .number() .int() .min(-1) .nullable() - .transform((quota) => (quota === null || quota === -1 ? null : quota * HumanReadableSize.GiB)); -const userRoleSchema = z.union([z.literal('user'), z.literal('admin')]); + .optional() + .transform((quota) => { + if (quota === undefined) { + return; + } + + return quota === null || quota === -1 ? null : quota * HumanReadableSize.GiB; + }) + .catch(undefined); export const OAuthProfileSchema = z.object({ defaults: z.object({ @@ -45,10 +63,11 @@ export const OAuthProfileSchema = z.object({ family_name: z.string().trim().optional().catch(undefined), preferred_username: z.string().trim().optional().catch(undefined), picture: z.string().optional().catch(undefined), - immich_storage_label: storageLabelClaimSchema.optional().catch(undefined), - immich_quota_bytes: quotaSizeInBytesSchema.optional().catch(undefined), + immich_storage_label: storageLabelClaimSchema, + immich_quota_bytes: quotaSizeInBytesSchema, immich_role: z - .union([userRoleSchema, z.array(userRoleSchema)]) + .union([z.string(), z.array(z.string())]) + .optional() .transform((value) => { if (!value) { return; @@ -61,7 +80,6 @@ export const OAuthProfileSchema = z.object({ return 'user'; }) - .optional() .catch(undefined), }); export type OAuthProfile = z.infer; diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts index fcfe01c26..dd8c94872 100644 --- a/server/src/services/auth.service.spec.ts +++ b/server/src/services/auth.service.spec.ts @@ -1334,6 +1334,32 @@ describe(AuthService.name, () => { }); }); + it('should sync an empty storage label as null on subsequent logins when a custom claim is configured', async () => { + const user = UserFactory.create({ oauthId: 'oauth-id', storageLabel: 'custom-label' }); + + mocks.systemMetadata.get.mockResolvedValue({ + oauth: { ...systemConfigStub.oauthWithAutoRegister.oauth, storageLabelClaim: 'immich_label' }, + }); + mocks.oauth.getProfileAndOAuthSid.mockResolvedValue({ + rawProfile: OAuthProfileFactory.create({ sub: user.oauthId!, immich_label: '' }), + }); + mocks.user.getByOAuthId.mockResolvedValue(user); + mocks.user.getByStorageLabel.mockResolvedValue(void 0); + mocks.user.update.mockResolvedValue(user); + mocks.session.create.mockResolvedValue(SessionFactory.create()); + + await sut.callback( + { url: 'http://immich/auth/login?code=abc123', state: 'xyz789', codeVerifier: 'foo' }, + {}, + loginDetails, + ); + + expect(mocks.user.update).toHaveBeenCalledWith(user.id, { + storageLabel: null, + updatedAt: expect.any(Date), + }); + }); + it('should promote an existing user to admin if the role claim contains admin on login', async () => { const oauthId = 'oauth-id'; const user = UserFactory.create({ isAdmin: false, oauthId }); @@ -1416,7 +1442,8 @@ describe(AuthService.name, () => { expect(mocks.user.syncUsage).not.toHaveBeenCalled(); }); - it('should update quota to default value on subsequent logins when it is absent from the profile', async () => { + // TODO re-enable in v4 + it.skip('should update quota to default value on subsequent logins when it is absent from the profile', async () => { const user = UserFactory.create({ oauthId: 'oauth-id' }); mocks.systemMetadata.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota); diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts index cdad0a242..dc2858332 100644 --- a/server/src/services/auth.service.ts +++ b/server/src/services/auth.service.ts @@ -623,10 +623,12 @@ export class AuthService extends BaseService { if ( oauth.storageLabelClaim !== defaults.oauth.storageLabelClaim && - profile.immich_storage_label && + profile.immich_storage_label !== undefined && profile.immich_storage_label !== user.storageLabel ) { - const duplicate = await this.userRepository.getByStorageLabel(profile.immich_storage_label); + const duplicate = profile.immich_storage_label + ? await this.userRepository.getByStorageLabel(profile.immich_storage_label) + : undefined; if (duplicate && duplicate.id !== user.id) { this.logger.warn(`Unable to sync OAuth storage label for user ${user.id}: label already in use`); @@ -635,9 +637,7 @@ export class AuthService extends BaseService { } } - if (profile.immich_quota_bytes === undefined && user.quotaSizeInBytes !== profile.defaults.immich_quota_bytes) { - updates.quotaSizeInBytes = profile.defaults.immich_quota_bytes; - } else if (profile.immich_quota_bytes !== undefined && profile.immich_quota_bytes !== user.quotaSizeInBytes) { + if (profile.immich_quota_bytes !== undefined && profile.immich_quota_bytes !== user.quotaSizeInBytes) { updates.quotaSizeInBytes = profile.immich_quota_bytes; }