🔒refactor: social login and remove direct user model access in strategies (#2946)

* refactor: checking `ALLOW_SOCIAL_REGISTRATION` with `isEnabled` * feat: Add findUserByEmail function to UserService This commit adds a new function, , to the module. This function retrieves a user document from the database based on the provided email. It returns the user document if found, otherwise it returns null. If there is a problem during user retrieval, an error is thrown. * refactor: add socialLogin to remove repetitive code
2025-12-16 16:30:15 +01:00 · 2024-06-06 19:23:11 +02:00 · 2024-06-06 19:23:11 +02:00 · b7fef6958b
commit b7fef6958b
parent 5452d4c20c
6 changed files with 104 additions and 143 deletions
--- a/api/server/services/UserService.js
+++ b/api/server/services/UserService.js
@ -166,6 +166,23 @@ const checkUserKeyExpiry = (expiresAt, endpoint) => {
  }
 };

+/**
+ * Retrieves a user document from the database based on the provided email.
+ * @async
+ * @param {string} email - The email of the user to find.
+ * @returns {Promise<Object|null>} The user document if found, otherwise null.
+ * @throws {Error} Throws an error if there is a problem during user retrieval.
+ */
+const findUserByEmail = async (email) => {
+  try {
+    const user = await User.findOne({ email });
+    return user;
+  } catch (error) {
+    logger.error(`[findUserByEmail] Error occurred while finding user by email: ${email}`, error);
+    throw error;
+  }
+};
+
 module.exports = {
  updateUserPluginsService,
  getUserKey,
@ -174,4 +191,5 @@ module.exports = {
  updateUserKey,
  deleteUserKey,
  checkUserKeyExpiry,
+  findUserByEmail,
 };
--- a/api/strategies/discordStrategy.js
+++ b/api/strategies/discordStrategy.js
@ -1,50 +1,27 @@
 const { Strategy: DiscordStrategy } = require('passport-discord');
-const { createNewUser, handleExistingUser } = require('./process');
-const { logger } = require('~/config');
-const User = require('~/models/User');
+const socialLogin = require('./socialLogin');

-const discordLogin = async (accessToken, refreshToken, profile, cb) => {
-  try {
-    const email = profile.email;
-    const discordId = profile.id;
-
-    // TODO: remove direct access of User model
-    const oldUser = await User.findOne({ email });
-    const ALLOW_SOCIAL_REGISTRATION =
-      process.env.ALLOW_SOCIAL_REGISTRATION?.toLowerCase() === 'true';
-    let avatarUrl;
-
-    if (profile.avatar) {
-      const format = profile.avatar.startsWith('a_') ? 'gif' : 'png';
-      avatarUrl = `https://cdn.discordapp.com/avatars/${profile.id}/${profile.avatar}.${format}`;
-    } else {
-      const defaultAvatarNum = Number(profile.discriminator) % 5;
-      avatarUrl = `https://cdn.discordapp.com/embed/avatars/${defaultAvatarNum}.png`;
-    }
-
-    if (oldUser) {
-      await handleExistingUser(oldUser, avatarUrl);
-      return cb(null, oldUser);
-    }
-
-    if (ALLOW_SOCIAL_REGISTRATION) {
-      const newUser = await createNewUser({
-        email,
-        avatarUrl,
-        provider: 'discord',
-        providerKey: 'discordId',
-        providerId: discordId,
-        username: profile.username,
-        name: profile.global_name,
-      });
-      return cb(null, newUser);
-    }
-  } catch (err) {
-    logger.error('[discordLogin]', err);
-    return cb(err);
+const getProfileDetails = (profile) => {
+  let avatarUrl;
+  if (profile.avatar) {
+    const format = profile.avatar.startsWith('a_') ? 'gif' : 'png';
+    avatarUrl = `https://cdn.discordapp.com/avatars/${profile.id}/${profile.avatar}.${format}`;
+  } else {
+    const defaultAvatarNum = Number(profile.discriminator) % 5;
+    avatarUrl = `https://cdn.discordapp.com/embed/avatars/${defaultAvatarNum}.png`;
  }
+
+  return {
+    email: profile.email,
+    id: profile.id,
+    avatarUrl,
+    username: profile.username,
+    name: profile.global_name,
+  };
 };

+const discordLogin = socialLogin('discord', getProfileDetails);
+
 module.exports = () =>
  new DiscordStrategy(
    {
--- a/api/strategies/facebookStrategy.js
+++ b/api/strategies/facebookStrategy.js
@ -1,39 +1,15 @@
 const FacebookStrategy = require('passport-facebook').Strategy;
-const { createNewUser, handleExistingUser } = require('./process');
-const { logger } = require('~/config');
-const User = require('~/models/User');
+const socialLogin = require('./socialLogin');

-const facebookLogin = async (accessToken, refreshToken, profile, cb) => {
-  try {
-    const email = profile.emails[0]?.value;
-    const facebookId = profile.id;
-    const oldUser = await User.findOne({ email });
-    const ALLOW_SOCIAL_REGISTRATION =
-      process.env.ALLOW_SOCIAL_REGISTRATION?.toLowerCase() === 'true';
-    const avatarUrl = profile.photos[0]?.value;
+const getProfileDetails = (profile) => ({
+  email: profile.emails[0]?.value,
+  id: profile.id,
+  avatarUrl: profile.photos[0]?.value,
+  username: profile.displayName,
+  name: profile.name?.givenName + ' ' + profile.name?.familyName,
+});

-    if (oldUser) {
-      await handleExistingUser(oldUser, avatarUrl);
-      return cb(null, oldUser);
-    }
-
-    if (ALLOW_SOCIAL_REGISTRATION) {
-      const newUser = await createNewUser({
-        email,
-        avatarUrl,
-        provider: 'facebook',
-        providerKey: 'facebookId',
-        providerId: facebookId,
-        username: profile.displayName,
-        name: profile.name?.givenName + ' ' + profile.name?.familyName,
-      });
-      return cb(null, newUser);
-    }
-  } catch (err) {
-    logger.error('[facebookLogin]', err);
-    return cb(err);
-  }
-};
+const facebookLogin = socialLogin('facebook', getProfileDetails);

 module.exports = () =>
  new FacebookStrategy(
--- a/api/strategies/githubStrategy.js
+++ b/api/strategies/githubStrategy.js
@ -1,40 +1,16 @@
 const { Strategy: GitHubStrategy } = require('passport-github2');
-const { createNewUser, handleExistingUser } = require('./process');
-const { logger } = require('~/config');
-const User = require('~/models/User');
+const socialLogin = require('./socialLogin');

-const githubLogin = async (accessToken, refreshToken, profile, cb) => {
-  try {
-    const email = profile.emails[0].value;
-    const githubId = profile.id;
-    const oldUser = await User.findOne({ email });
-    const ALLOW_SOCIAL_REGISTRATION =
-      process.env.ALLOW_SOCIAL_REGISTRATION?.toLowerCase() === 'true';
-    const avatarUrl = profile.photos[0].value;
+const getProfileDetails = (profile) => ({
+  email: profile.emails[0].value,
+  id: profile.id,
+  avatarUrl: profile.photos[0].value,
+  username: profile.username,
+  name: profile.displayName,
+  emailVerified: profile.emails[0].verified,
+});

-    if (oldUser) {
-      await handleExistingUser(oldUser, avatarUrl);
-      return cb(null, oldUser);
-    }
-
-    if (ALLOW_SOCIAL_REGISTRATION) {
-      const newUser = await createNewUser({
-        email,
-        avatarUrl,
-        provider: 'github',
-        providerKey: 'githubId',
-        providerId: githubId,
-        username: profile.username,
-        name: profile.displayName,
-        emailVerified: profile.emails[0].verified,
-      });
-      return cb(null, newUser);
-    }
-  } catch (err) {
-    logger.error('[githubLogin]', err);
-    return cb(err);
-  }
-};
+const githubLogin = socialLogin('github', getProfileDetails);

 module.exports = () =>
  new GitHubStrategy(
--- a/api/strategies/googleStrategy.js
+++ b/api/strategies/googleStrategy.js
@ -1,40 +1,16 @@
 const { Strategy: GoogleStrategy } = require('passport-google-oauth20');
-const { createNewUser, handleExistingUser } = require('./process');
-const { logger } = require('~/config');
-const User = require('~/models/User');
+const socialLogin = require('./socialLogin');

-const googleLogin = async (accessToken, refreshToken, profile, cb) => {
-  try {
-    const email = profile.emails[0].value;
-    const googleId = profile.id;
-    const oldUser = await User.findOne({ email });
-    const ALLOW_SOCIAL_REGISTRATION =
-      process.env.ALLOW_SOCIAL_REGISTRATION?.toLowerCase() === 'true';
-    const avatarUrl = profile.photos[0].value;
+const getProfileDetails = (profile) => ({
+  email: profile.emails[0].value,
+  id: profile.id,
+  avatarUrl: profile.photos[0].value,
+  username: profile.name.givenName,
+  name: `${profile.name.givenName} ${profile.name.familyName}`,
+  emailVerified: profile.emails[0].verified,
+});

-    if (oldUser) {
-      await handleExistingUser(oldUser, avatarUrl);
-      return cb(null, oldUser);
-    }
-
-    if (ALLOW_SOCIAL_REGISTRATION) {
-      const newUser = await createNewUser({
-        email,
-        avatarUrl,
-        provider: 'google',
-        providerKey: 'googleId',
-        providerId: googleId,
-        username: profile.name.givenName,
-        name: `${profile.name.givenName} ${profile.name.familyName}`,
-        emailVerified: profile.emails[0].verified,
-      });
-      return cb(null, newUser);
-    }
-  } catch (err) {
-    logger.error('[googleLogin]', err);
-    return cb(err);
-  }
-};
+const googleLogin = socialLogin('google', getProfileDetails);

 module.exports = () =>
  new GoogleStrategy(
--- a/api/strategies/socialLogin.js
+++ b/api/strategies/socialLogin.js
@ -0,0 +1,38 @@
+const { createNewUser, handleExistingUser } = require('./process');
+const { logger } = require('~/config');
+const { findUserByEmail } = require('~/server/services/UserService');
+const { isEnabled } = require('~/server/utils');
+
+const socialLogin =
+  (provider, getProfileDetails) => async (accessToken, refreshToken, profile, cb) => {
+    try {
+      const { email, id, avatarUrl, username, name, emailVerified } = getProfileDetails(profile);
+
+      const oldUser = await findUserByEmail(email);
+      const ALLOW_SOCIAL_REGISTRATION = isEnabled(process.env.ALLOW_SOCIAL_REGISTRATION);
+
+      if (oldUser) {
+        await handleExistingUser(oldUser, avatarUrl);
+        return cb(null, oldUser);
+      }
+
+      if (ALLOW_SOCIAL_REGISTRATION) {
+        const newUser = await createNewUser({
+          email,
+          avatarUrl,
+          provider,
+          providerKey: `${provider}Id`,
+          providerId: id,
+          username,
+          name,
+          emailVerified,
+        });
+        return cb(null, newUser);
+      }
+    } catch (err) {
+      logger.error(`[${provider}Login]`, err);
+      return cb(err);
+    }
+  };
+
+module.exports = socialLogin;