🛂 refactor: Centralize fileStrategy Resolution for OpenID, SAML, and Social Logins (#9468)

* 🔑 refactor: `fileStrategy` for OpenID, SAML, and Social logins

* ci: Update Apple strategy tests to use correct isEnabled import and enhance handleExistingUser call

api/strategies/appleStrategy.test.js

@@ -1,10 +1,10 @@
 const jwt = require('jsonwebtoken');
 const mongoose = require('mongoose');
+const { isEnabled } = require('@librechat/api');
 const { logger } = require('@librechat/data-schemas');
 const { Strategy: AppleStrategy } = require('passport-apple');
 const { MongoMemoryServer } = require('mongodb-memory-server');
 const { createSocialUser, handleExistingUser } = require('./process');
-const { isEnabled } = require('~/server/utils');
 const socialLogin = require('./socialLogin');
 const { findUser } = require('~/models');
 const { User } = require('~/db/models');
@@ -17,6 +17,8 @@ jest.mock('@librechat/data-schemas', () => {
     logger: {
       error: jest.fn(),
       debug: jest.fn(),
+      info: jest.fn(),
+      warn: jest.fn(),
     },
   };
 });
@@ -24,12 +26,19 @@ jest.mock('./process', () => ({
   createSocialUser: jest.fn(),
   handleExistingUser: jest.fn(),
 }));
-jest.mock('~/server/utils', () => ({
+jest.mock('@librechat/api', () => ({
+  ...jest.requireActual('@librechat/api'),
   isEnabled: jest.fn(),
 }));
 jest.mock('~/models', () => ({
   findUser: jest.fn(),
 }));
+jest.mock('~/server/services/Config', () => ({
+  getAppConfig: jest.fn().mockResolvedValue({
+    fileStrategy: 'local',
+    balance: { enabled: false },
+  }),
+}));
 
 describe('Apple Login Strategy', () => {
   let mongoServer;
@@ -288,7 +297,14 @@ describe('Apple Login Strategy', () => {
 
       expect(mockVerifyCallback).toHaveBeenCalledWith(null, existingUser);
       expect(existingUser.avatarUrl).toBeNull(); // As per getProfileDetails
-      expect(handleExistingUser).toHaveBeenCalledWith(existingUser, null);
+      expect(handleExistingUser).toHaveBeenCalledWith(
+        existingUser,
+        null,
+        expect.objectContaining({
+          fileStrategy: 'local',
+          balance: { enabled: false },
+        }),
+      );
     });
 
     it('should handle missing idToken gracefully', async () => {

api/strategies/openidStrategy.js

@@ -398,6 +398,7 @@ async function setupOpenId() {
             );
           }
 
+          const appConfig = await getAppConfig();
           if (!user) {
             user = {
               provider: 'openid',
@@ -409,7 +410,6 @@ async function setupOpenId() {
               idOnTheSource: userinfo.oid,
             };
 
-            const appConfig = await getAppConfig();
             const balanceConfig = getBalanceConfig(appConfig);
             user = await createUser(user, balanceConfig, true, true);
           } else {
@@ -438,7 +438,9 @@ async function setupOpenId() {
               userinfo.sub,
             );
             if (imageBuffer) {
-              const { saveBuffer } = getStrategyFunctions(process.env.CDN_PROVIDER);
+              const { saveBuffer } = getStrategyFunctions(
+                appConfig?.fileStrategy ?? process.env.CDN_PROVIDER,
+              );
               const imagePath = await saveBuffer({
                 fileName,
                 userId: user._id.toString(),

api/strategies/process.js

@@ -3,7 +3,6 @@ const { FileSources } = require('librechat-data-provider');
 const { getStrategyFunctions } = require('~/server/services/Files/strategies');
 const { resizeAvatar } = require('~/server/services/Files/images/avatar');
 const { updateUser, createUser, getUserById } = require('~/models');
-const { getAppConfig } = require('~/server/services/Config');
 
 /**
  * Updates the avatar URL of an existing user. If the user's avatar URL does not include the query parameter
@@ -12,14 +11,15 @@ const { getAppConfig } = require('~/server/services/Config');
  *
  * @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.
  *
  * @returns {Promise<void>}
  *          The function updates the user's avatar 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) => {
-  const fileStrategy = process.env.CDN_PROVIDER;
+const handleExistingUser = async (oldUser, avatarUrl, appConfig) => {
+  const fileStrategy = appConfig?.fileStrategy ?? process.env.CDN_PROVIDER;
   const isLocal = fileStrategy === FileSources.local;
 
   let updatedAvatar = false;
@@ -56,6 +56,7 @@ const handleExistingUser = async (oldUser, avatarUrl) => {
  * @param {string} params.providerId - The provider-specific ID of the user.
  * @param {string} params.username - The username of the new user.
  * @param {string} params.name - The name of the new user.
+ * @param {AppConfig} appConfig - The application configuration object.
  * @param {boolean} [params.emailVerified=false] - Optional. Indicates whether the user's email is verified. Defaults to false.
  *
  * @returns {Promise<User>}
@@ -71,6 +72,7 @@ const createSocialUser = async ({
   providerId,
   username,
   name,
+  appConfig,
   emailVerified,
 }) => {
   const update = {
@@ -83,10 +85,9 @@ const createSocialUser = async ({
     emailVerified,
   };
 
-  const appConfig = await getAppConfig();
   const balanceConfig = getBalanceConfig(appConfig);
   const newUserId = await createUser(update, balanceConfig);
-  const fileStrategy = process.env.CDN_PROVIDER;
+  const fileStrategy = appConfig?.fileStrategy ?? process.env.CDN_PROVIDER;
   const isLocal = fileStrategy === FileSources.local;
 
   if (!isLocal) {

api/strategies/samlStrategy.js

@@ -220,6 +220,7 @@ async function setupSaml() {
             getUserName(profile) || getGivenName(profile) || getEmail(profile),
           );
 
+          const appConfig = await getAppConfig();
           if (!user) {
             user = {
               provider: 'saml',
@@ -229,7 +230,6 @@ async function setupSaml() {
               emailVerified: true,
               name: fullName,
             };
-            const appConfig = await getAppConfig();
             const balanceConfig = getBalanceConfig(appConfig);
             user = await createUser(user, balanceConfig, true, true);
           } else {
@@ -250,7 +250,9 @@ async function setupSaml() {
                 fileName = profile.nameID + '.png';
               }
 
-              const { saveBuffer } = getStrategyFunctions(process.env.CDN_PROVIDER);
+              const { saveBuffer } = getStrategyFunctions(
+                appConfig?.fileStrategy ?? process.env.CDN_PROVIDER,
+              );
               const imagePath = await saveBuffer({
                 fileName,
                 userId: user._id.toString(),

api/strategies/socialLogin.js

@@ -2,6 +2,7 @@ const { isEnabled } = require('@librechat/api');
 const { logger } = require('@librechat/data-schemas');
 const { ErrorTypes } = require('librechat-data-provider');
 const { createSocialUser, handleExistingUser } = require('./process');
+const { getAppConfig } = require('~/server/services/Config');
 const { findUser } = require('~/models');
 
 const socialLogin =
@@ -12,11 +13,12 @@ const socialLogin =
         profile,
       });
 
+      const appConfig = await getAppConfig();
       const existingUser = await findUser({ email: email.trim() });
       const ALLOW_SOCIAL_REGISTRATION = isEnabled(process.env.ALLOW_SOCIAL_REGISTRATION);
 
       if (existingUser?.provider === provider) {
-        await handleExistingUser(existingUser, avatarUrl);
+        await handleExistingUser(existingUser, avatarUrl, appConfig);
         return cb(null, existingUser);
       } else if (existingUser) {
         logger.info(
@@ -38,6 +40,7 @@ const socialLogin =
           username,
           name,
           emailVerified,
+          appConfig,
         });
         return cb(null, newUser);
       }