From 75dd6fb28be2aa2bbc171a48679929a903d01474 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 5 Sep 2025 11:09:32 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=82=20refactor:=20Centralize=20`fileSt?= =?UTF-8?q?rategy`=20Resolution=20for=20OpenID,=20SAML,=20and=20Social=20L?= =?UTF-8?q?ogins=20(#9468)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔑 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 | 22 +++++++++++++++++++--- api/strategies/openidStrategy.js | 6 ++++-- api/strategies/process.js | 11 ++++++----- api/strategies/samlStrategy.js | 6 ++++-- api/strategies/socialLogin.js | 5 ++++- 5 files changed, 37 insertions(+), 13 deletions(-) diff --git a/api/strategies/appleStrategy.test.js b/api/strategies/appleStrategy.test.js index 65a961bd4d..d8ba4616f2 100644 --- a/api/strategies/appleStrategy.test.js +++ b/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 () => { diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index 6bac9734a4..370d502abc 100644 --- a/api/strategies/openidStrategy.js +++ b/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(), diff --git a/api/strategies/process.js b/api/strategies/process.js index f8b01f0544..8f70cd86ce 100644 --- a/api/strategies/process.js +++ b/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} * 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} @@ -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) { diff --git a/api/strategies/samlStrategy.js b/api/strategies/samlStrategy.js index e09d64bce6..93118f6743 100644 --- a/api/strategies/samlStrategy.js +++ b/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(), diff --git a/api/strategies/socialLogin.js b/api/strategies/socialLogin.js index 79ba5b8e9e..5808cbc9b8 100644 --- a/api/strategies/socialLogin.js +++ b/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); }