fix: oauth regressions (#32934)

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<typeof OAuthProfileSchema>;

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

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;
     }