🛂 feat: Social Login by Provider ID First then Email (danny-avila#10358)

Author: danny-avila

api/strategies/appleStrategy.test.js

@@ -304,6 +304,7 @@ describe('Apple Login Strategy', () => {
           fileStrategy: 'local',
           balance: { enabled: false },
         }),
+        'jane.doe@example.com',
       );
     });
 

api/strategies/process.js

@@ -5,22 +5,25 @@ const { resizeAvatar } = require('~/server/services/Files/images/avatar');
 const { updateUser, createUser, getUserById } = require('~/models');
 
 /**
- * Updates the avatar URL of an existing user. If the user's avatar URL does not include the query parameter
+ * Updates the avatar URL and email of an existing user. If the user's avatar URL does not include the query parameter
  * '?manual=true', it updates the user's avatar with the provided URL. For local file storage, it directly updates
  * the avatar URL, while for other storage types, it processes the avatar URL using the specified file strategy.
+ * Also updates the email if it has changed (e.g., when a Google Workspace email is updated).
  *
  * @param {IUser} oldUser - The existing user object that needs to be updated.
  * @param {string} avatarUrl - The new avatar URL to be set for the user.
  * @param {AppConfig} appConfig - The application configuration object.
+ * @param {string} [email] - Optional. The new email address to update if it has changed.
  *
  * @returns {Promise<void>}
- *          The function updates the user's avatar and saves the user object. It does not return any value.
+ *          The function updates the user's avatar and/or email and saves the user object. It does not return any value.
  *
  * @throws {Error} Throws an error if there's an issue saving the updated user object.
  */
-const handleExistingUser = async (oldUser, avatarUrl, appConfig) => {
+const handleExistingUser = async (oldUser, avatarUrl, appConfig, email) => {
   const fileStrategy = appConfig?.fileStrategy ?? process.env.CDN_PROVIDER;
   const isLocal = fileStrategy === FileSources.local;
+  const updates = {};
 
   let updatedAvatar = false;
   const hasManualFlag =
@@ -39,7 +42,16 @@ const handleExistingUser = async (oldUser, avatarUrl, appConfig) => {
   }
 
   if (updatedAvatar) {
-    await updateUser(oldUser._id, { avatar: updatedAvatar });
+    updates.avatar = updatedAvatar;
+  }
+
+  /** Update email if it has changed */
+  if (email && email.trim() !== oldUser.email) {
+    updates.email = email.trim();
+  }
+
+  if (Object.keys(updates).length > 0) {
+    await updateUser(oldUser._id, updates);
   }
 };
 

api/strategies/process.test.js

@@ -167,4 +167,76 @@ describe('handleExistingUser', () => {
     // This should throw an error when trying to access oldUser._id
     await expect(handleExistingUser(null, avatarUrl)).rejects.toThrow();
   });
+
+  it('should update email when it has changed', async () => {
+    const oldUser = {
+      _id: 'user123',
+      email: 'old@example.com',
+      avatar: 'https://example.com/avatar.png?manual=true',
+    };
+    const avatarUrl = 'https://example.com/avatar.png';
+    const newEmail = 'new@example.com';
+
+    await handleExistingUser(oldUser, avatarUrl, {}, newEmail);
+
+    expect(updateUser).toHaveBeenCalledWith('user123', { email: 'new@example.com' });
+  });
+
+  it('should update both avatar and email when both have changed', async () => {
+    const oldUser = {
+      _id: 'user123',
+      email: 'old@example.com',
+      avatar: null,
+    };
+    const avatarUrl = 'https://example.com/new-avatar.png';
+    const newEmail = 'new@example.com';
+
+    await handleExistingUser(oldUser, avatarUrl, {}, newEmail);
+
+    expect(updateUser).toHaveBeenCalledWith('user123', {
+      avatar: avatarUrl,
+      email: 'new@example.com',
+    });
+  });
+
+  it('should not update email when it has not changed', async () => {
+    const oldUser = {
+      _id: 'user123',
+      email: 'same@example.com',
+      avatar: 'https://example.com/avatar.png?manual=true',
+    };
+    const avatarUrl = 'https://example.com/avatar.png';
+    const sameEmail = 'same@example.com';
+
+    await handleExistingUser(oldUser, avatarUrl, {}, sameEmail);
+
+    expect(updateUser).not.toHaveBeenCalled();
+  });
+
+  it('should trim email before comparison and update', async () => {
+    const oldUser = {
+      _id: 'user123',
+      email: 'test@example.com',
+      avatar: 'https://example.com/avatar.png?manual=true',
+    };
+    const avatarUrl = 'https://example.com/avatar.png';
+    const newEmailWithSpaces = '  newemail@example.com  ';
+
+    await handleExistingUser(oldUser, avatarUrl, {}, newEmailWithSpaces);
+
+    expect(updateUser).toHaveBeenCalledWith('user123', { email: 'newemail@example.com' });
+  });
+
+  it('should not update when email parameter is not provided', async () => {
+    const oldUser = {
+      _id: 'user123',
+      email: 'test@example.com',
+      avatar: 'https://example.com/avatar.png?manual=true',
+    };
+    const avatarUrl = 'https://example.com/avatar.png';
+
+    await handleExistingUser(oldUser, avatarUrl, {});
+
+    expect(updateUser).not.toHaveBeenCalled();
+  });
 });

api/strategies/socialLogin.js

@@ -25,10 +25,24 @@ const socialLogin =
         return cb(error);
       }
 
-      const existingUser = await findUser({ email: email.trim() });
+      const providerKey = `${provider}Id`;
+      let existingUser = null;
+
+      /** First try to find user by provider ID (e.g., googleId, facebookId) */
+      if (id && typeof id === 'string') {
+        existingUser = await findUser({ [providerKey]: id });
+      }
+
+      /** If not found by provider ID, try finding by email */
+      if (!existingUser) {
+        existingUser = await findUser({ email: email?.trim() });
+        if (existingUser) {
+          logger.warn(`[${provider}Login] User found by email: ${email} but not by ${providerKey}`);
+        }
+      }
 
       if (existingUser?.provider === provider) {
-        await handleExistingUser(existingUser, avatarUrl, appConfig);
+        await handleExistingUser(existingUser, avatarUrl, appConfig, email);
         return cb(null, existingUser);
       } else if (existingUser) {
         logger.info(