diff --git a/api/server/services/Files/images/avatar.js b/api/server/services/Files/images/avatar.js index 640aed293f..72e038cbec 100644 --- a/api/server/services/Files/images/avatar.js +++ b/api/server/services/Files/images/avatar.js @@ -26,7 +26,7 @@ const MAX_AVATAR_BYTES = 10 * 1024 * 1024; * measurable benefit on this path. If this ever becomes a hot path, hoist * the agents to module scope. */ -async function fetchAvatarBuffer(input) { +async function fetchAvatarBuffer(input, fetchOptions = {}) { let parsed; try { parsed = new URL(input); @@ -44,6 +44,7 @@ async function fetchAvatarBuffer(input) { * stronger of the two for this path — bounds total slow-loris exposure. */ const response = await fetch(parsed.href, { + headers: fetchOptions.headers, agent: (urlObj) => (urlObj.protocol === 'https:' ? httpsAgent : httpAgent), redirect: 'error', timeout: 5000, @@ -80,6 +81,7 @@ async function fetchAvatarBuffer(input) { * @param {string} options.desiredFormat - The desired output format of the image. * @param {(string|Buffer|File)} params.input - The input representing the avatar image. Can be a URL (string), * a Buffer, or a File object. + * @param {{ headers?: Record }} [params.fetchOptions] - Optional headers for trusted avatar URLs. * * @returns {Promise} * A promise that resolves to a resized buffer. @@ -87,7 +89,7 @@ async function fetchAvatarBuffer(input) { * @throws {Error} Throws an error if the user ID is undefined, the input type is invalid, the image fetching fails, * or any other error occurs during the processing. */ -async function resizeAvatar({ userId, input, desiredFormat = EImageOutputType.PNG }) { +async function resizeAvatar({ userId, input, desiredFormat = EImageOutputType.PNG, fetchOptions }) { try { if (userId === undefined) { throw new Error('User ID is undefined'); @@ -95,7 +97,7 @@ async function resizeAvatar({ userId, input, desiredFormat = EImageOutputType.PN let imageBuffer; if (typeof input === 'string') { - imageBuffer = await fetchAvatarBuffer(input); + imageBuffer = await fetchAvatarBuffer(input, fetchOptions); } else if (input instanceof Buffer) { imageBuffer = input; } else if (typeof input === 'object' && input instanceof File) { diff --git a/api/server/services/Files/images/avatar.spec.js b/api/server/services/Files/images/avatar.spec.js index cc3e0b1ad1..2eb2240468 100644 --- a/api/server/services/Files/images/avatar.spec.js +++ b/api/server/services/Files/images/avatar.spec.js @@ -117,6 +117,26 @@ describe('resizeAvatar — fetchAvatarBuffer', () => { expect(agentFn(new URL('http://anything'))).toEqual({ __kind: 'http' }); expect(createSSRFSafeAgents).toHaveBeenCalledTimes(1); }); + + it('passes configured fetch headers while preserving shared fetch controls', async () => { + fetch.mockResolvedValueOnce(makeResponse({ body: Buffer.from('rawimg') })); + await resizeAvatar({ + userId: 'u1', + input: 'https://cdn.example.com/avatar.png', + fetchOptions: { + headers: { + Authorization: 'Bearer avatar-token', + }, + }, + }); + + const opts = fetch.mock.calls[0][1]; + expect(opts.headers).toEqual({ Authorization: 'Bearer avatar-token' }); + expect(opts.redirect).toBe('error'); + expect(opts.timeout).toBe(5000); + expect(opts.size).toBe(10 * 1024 * 1024); + expect(typeof opts.agent).toBe('function'); + }); }); describe('rejects unsafe responses', () => { diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index 85dc400e7f..86e09480ef 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -1,10 +1,8 @@ const undici = require('undici'); const { get } = require('lodash'); -const fetch = require('node-fetch'); const passport = require('passport'); const client = require('openid-client'); const jwtDecode = require('jsonwebtoken/decode'); -const { HttpsProxyAgent } = require('https-proxy-agent'); const { hashToken, logger } = require('@librechat/data-schemas'); const { Strategy: OpenIDStrategy } = require('openid-client/passport'); const { CacheKeys, ErrorTypes, SystemRoles } = require('librechat-data-provider'); @@ -22,6 +20,7 @@ const { resolveAppConfigForUser, } = require('@librechat/api'); const { getStrategyFunctions } = require('~/server/services/Files/strategies'); +const { resizeAvatar } = require('~/server/services/Files/images/avatar'); const { findUser, createUser, updateUser } = require('~/models'); const { getAppConfig } = require('~/server/services/Config'); const getLogStores = require('~/cache/getLogStores'); @@ -200,43 +199,64 @@ const getUserInfo = async (config, accessToken, sub) => { } }; -/** - * Downloads an image from a URL using an access token. - * @param {string} url - * @param {Configuration} config - * @param {string} accessToken access token - * @param {string} sub - The subject identifier of the user. usually found as "sub" in the claims of the token - * @returns {Promise} The image buffer or an empty string if the download fails. - */ -const downloadImage = async (url, config, accessToken, sub) => { +function getUrlOrigin(value) { + try { + return new URL(value).origin; + } catch { + return null; + } +} + +function getOpenIDAvatarAuthorizedOrigins(config) { + const metadata = config?.serverMetadata?.() ?? {}; + const metadataOrigins = [metadata.issuer, metadata.userinfo_endpoint] + .map(getUrlOrigin) + .filter(Boolean); + const configuredOrigins = (process.env.OPENID_AVATAR_AUTHORIZED_ORIGINS ?? '') + .split(/[\s,]+/) + .map(getUrlOrigin) + .filter(Boolean); + + return new Set([...metadataOrigins, ...configuredOrigins]); +} + +function shouldAuthorizeOpenIDAvatar(url, config) { + const origin = getUrlOrigin(url); + if (!origin) { + return false; + } + + return getOpenIDAvatarAuthorizedOrigins(config).has(origin); +} + +async function getOpenIDAvatarFetchOptions(url, config, accessToken, sub) { + if (!shouldAuthorizeOpenIDAvatar(url, config)) { + return undefined; + } + const exchangedAccessToken = await exchangeAccessTokenIfNeeded(config, accessToken, sub, true); + return { + headers: { + Authorization: `Bearer ${exchangedAccessToken}`, + }, + }; +} + +const resizeIdentityProviderAvatar = async (url, userId, config, accessToken, sub) => { if (!url) { return ''; } try { - const options = { - method: 'GET', - headers: { - Authorization: `Bearer ${exchangedAccessToken}`, - }, - }; - - if (process.env.PROXY) { - options.agent = new HttpsProxyAgent(process.env.PROXY); - } - - const response = await fetch(url, options); - - if (response.ok) { - const buffer = await response.buffer(); - return buffer; - } else { - throw new Error(`${response.statusText} (HTTP ${response.status})`); + const fetchOptions = await getOpenIDAvatarFetchOptions(url, config, accessToken, sub); + const avatarParams = { userId, input: url }; + if (fetchOptions) { + avatarParams.fetchOptions = fetchOptions; } + return await resizeAvatar(avatarParams); } catch (error) { logger.error( - `[openidStrategy] downloadImage: Error downloading image at URL "${url}": ${error}`, + `[openidStrategy] resizeIdentityProviderAvatar: Error processing avatar at URL "${url}": ${error}`, ); return ''; } @@ -662,8 +682,10 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { fileName = userinfo.sub + '.png'; } - const imageBuffer = await downloadImage( + const userId = user._id.toString(); + const imageBuffer = await resizeIdentityProviderAvatar( imageUrl, + userId, openidConfig, tokenset.access_token, userinfo.sub, @@ -674,7 +696,7 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { const imagePath = await saveBuffer( getAvatarSaveParams(fileStrategy, { fileName, - userId: user._id.toString(), + userId, buffer: imageBuffer, tenantId: user.tenantId, }), diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index 086376306d..12056f2530 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -4,6 +4,7 @@ const jwtDecode = require('jsonwebtoken/decode'); const { ErrorTypes, FileSources } = require('librechat-data-provider'); const { findUser, createUser, updateUser } = require('~/models'); const { getOpenIdIssuer, resolveAppConfigForUser, isEnabled } = require('@librechat/api'); +const { resizeAvatar } = require('~/server/services/Files/images/avatar'); const { getAppConfig } = require('~/server/services/Config'); const { setupOpenId } = require('./openidStrategy'); @@ -21,6 +22,9 @@ jest.mock('~/server/services/Files/strategies', () => ({ saveBuffer: jest.fn().mockResolvedValue('/fake/path/to/avatar.png'), })), })); +jest.mock('~/server/services/Files/images/avatar', () => ({ + resizeAvatar: jest.fn().mockResolvedValue(Buffer.from('safe avatar')), +})); jest.mock('~/server/services/Config', () => ({ getAppConfig: jest.fn().mockResolvedValue({}), })); @@ -215,6 +219,7 @@ describe('setupOpenId', () => { delete process.env.OPENID_USERNAME_CLAIM; delete process.env.OPENID_NAME_CLAIM; delete process.env.OPENID_EMAIL_CLAIM; + delete process.env.OPENID_AVATAR_AUTHORIZED_ORIGINS; delete process.env.PROXY; delete process.env.OPENID_USE_PKCE; delete process.env.OPENID_GENERATE_NONCE; @@ -235,13 +240,7 @@ describe('setupOpenId', () => { return { _id: id, ...userData }; }); - // For image download, simulate a successful response - const fakeBuffer = Buffer.from('fake image'); - const fakeResponse = { - ok: true, - buffer: jest.fn().mockResolvedValue(fakeBuffer), - }; - fetch.mockResolvedValue(fakeResponse); + resizeAvatar.mockResolvedValue(Buffer.from('safe avatar')); // Call the setup function and capture the verify callback for the regular 'openid' strategy // (not 'openidAdmin' which requires existing users) @@ -1281,7 +1280,7 @@ describe('setupOpenId', () => { }); }); - it('should attempt to download and save the avatar if picture is provided', async () => { + it('should process and save the avatar through the shared avatar path if picture is provided', async () => { const { getStrategyFunctions } = require('~/server/services/Files/strategies'); // Act @@ -1291,8 +1290,11 @@ describe('setupOpenId', () => { const { saveBuffer } = strategyResult.value; const [saveParams] = saveBuffer.mock.calls[0]; - // Assert – verify that download was attempted and the avatar field was set via updateUser - expect(fetch).toHaveBeenCalled(); + expect(resizeAvatar).toHaveBeenCalledWith({ + userId: 'newUserId', + input: 'https://example.com/avatar.png', + }); + expect(fetch).not.toHaveBeenCalled(); expect(saveParams).toEqual( expect.objectContaining({ fileName: 'hashed-token.png', @@ -1305,6 +1307,44 @@ describe('setupOpenId', () => { expect(user.avatar).toBe('/fake/path/to/avatar.png'); }); + it('uses only the shared avatar processor for OpenID picture URLs', async () => { + await validate(tokenset); + + expect(resizeAvatar).toHaveBeenCalledWith({ + userId: 'newUserId', + input: 'https://example.com/avatar.png', + }); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('adds auth headers for configured OpenID avatar origins', async () => { + process.env.OPENID_AVATAR_AUTHORIZED_ORIGINS = 'https://example.com'; + + await validate(tokenset); + + expect(resizeAvatar).toHaveBeenCalledWith({ + userId: 'newUserId', + input: 'https://example.com/avatar.png', + fetchOptions: { + headers: { + Authorization: 'Bearer fake_access_token', + }, + }, + }); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('continues login when shared avatar processing rejects the picture URL', async () => { + const { getStrategyFunctions } = require('~/server/services/Files/strategies'); + resizeAvatar.mockRejectedValueOnce(new Error('avatar processing failed')); + + const { user } = await validate(tokenset); + + expect(user).toBeTruthy(); + expect(user.avatar).toBeUndefined(); + expect(getStrategyFunctions).not.toHaveBeenCalled(); + }); + it('should save CloudFront IdP avatars under the shared avatar prefix', async () => { const { getStrategyFunctions } = require('~/server/services/Files/strategies'); getAppConfig.mockResolvedValueOnce({ fileStrategy: mockCloudfrontFileSource }); @@ -1316,6 +1356,11 @@ describe('setupOpenId', () => { const [saveParams] = saveBuffer.mock.calls[0]; expect(getStrategyFunctions).toHaveBeenLastCalledWith(mockCloudfrontFileSource); + expect(resizeAvatar).toHaveBeenCalledWith({ + userId: 'newUserId', + input: 'https://example.com/avatar.png', + }); + expect(fetch).not.toHaveBeenCalled(); expect(saveParams).toEqual( expect.objectContaining({ basePath: 'avatars', @@ -1336,6 +1381,7 @@ describe('setupOpenId', () => { // Assert – fetch should not be called and avatar should remain undefined or empty expect(fetch).not.toHaveBeenCalled(); + expect(resizeAvatar).not.toHaveBeenCalled(); // Depending on your implementation, user.avatar may be undefined or an empty string. }); diff --git a/api/strategies/samlStrategy.js b/api/strategies/samlStrategy.js index b21a4a482b..cab43044b9 100644 --- a/api/strategies/samlStrategy.js +++ b/api/strategies/samlStrategy.js @@ -1,6 +1,5 @@ const fs = require('fs'); const path = require('path'); -const fetch = require('node-fetch'); const passport = require('passport'); const { ErrorTypes } = require('librechat-data-provider'); const { hashToken, logger } = require('@librechat/data-schemas'); @@ -13,6 +12,7 @@ const { resolveAppConfigForUser, } = require('@librechat/api'); const { getStrategyFunctions } = require('~/server/services/Files/strategies'); +const { resizeAvatar } = require('~/server/services/Files/images/avatar'); const { findUser, createUser, updateUser } = require('~/models'); const { getAppConfig } = require('~/server/services/Config'); const paths = require('~/config/paths'); @@ -110,21 +110,17 @@ function getPicture(profile) { return getSamlClaim(profile, 'SAML_PICTURE_CLAIM', 'picture'); } -/** - * Downloads an image from a URL using an access token. - * @param {string} url - * @returns {Promise} - */ -const downloadImage = async (url) => { +const resizeIdentityProviderAvatar = async (url, userId) => { + if (!url) { + return null; + } + try { - const response = await fetch(url); - if (response.ok) { - return await response.buffer(); - } else { - throw new Error(`${response.statusText} (HTTP ${response.status})`); - } + return await resizeAvatar({ userId, input: url }); } catch (error) { - logger.error(`[samlStrategy] Error downloading image at URL "${url}": ${error}`); + logger.error( + `[samlStrategy] resizeIdentityProviderAvatar: Error processing avatar at URL "${url}": ${error}`, + ); return null; } }; @@ -264,7 +260,8 @@ function createSamlCallback(existingUsersOnly = false) { const picture = getPicture(profile); if (picture && !user.avatar?.includes('manual=true')) { - const imageBuffer = await downloadImage(profile.picture); + const userId = user._id.toString(); + const imageBuffer = await resizeIdentityProviderAvatar(picture, userId); if (imageBuffer) { let fileName; if (crypto) { @@ -278,7 +275,7 @@ function createSamlCallback(existingUsersOnly = false) { const imagePath = await saveBuffer( getAvatarSaveParams(fileStrategy, { fileName, - userId: user._id.toString(), + userId, buffer: imageBuffer, tenantId: user.tenantId, }), diff --git a/api/strategies/samlStrategy.spec.js b/api/strategies/samlStrategy.spec.js index dbf003e58c..301fee3a87 100644 --- a/api/strategies/samlStrategy.spec.js +++ b/api/strategies/samlStrategy.spec.js @@ -53,6 +53,9 @@ jest.mock('~/server/services/Files/strategies', () => ({ saveBuffer: jest.fn().mockResolvedValue('/fake/path/to/avatar.png'), })), })); +jest.mock('~/server/services/Files/images/avatar', () => ({ + resizeAvatar: jest.fn().mockResolvedValue(Buffer.from('safe avatar')), +})); jest.mock('~/config/paths', () => ({ root: '/fake/root/path', })); @@ -64,6 +67,7 @@ const { Strategy: SamlStrategy } = require('@node-saml/passport-saml'); const { FileSources } = require('librechat-data-provider'); const { findUser } = require('~/models'); const { resolveAppConfigForUser } = require('@librechat/api'); +const { resizeAvatar } = require('~/server/services/Files/images/avatar'); const { getAppConfig } = require('~/server/services/Config'); const { setupSaml, getCertificateContent } = require('./samlStrategy'); @@ -288,12 +292,7 @@ u7wlOSk+oFzDIO/UILIA delete process.env.SAML_PICTURE_CLAIM; delete process.env.SAML_NAME_CLAIM; - // Simulate image download - const fakeBuffer = Buffer.from('fake image'); - fetch.mockResolvedValue({ - ok: true, - buffer: jest.fn().mockResolvedValue(fakeBuffer), - }); + resizeAvatar.mockResolvedValue(Buffer.from('safe avatar')); await setupSaml(); }); @@ -447,7 +446,7 @@ u7wlOSk+oFzDIO/UILIA expect(result.details.message).toBe(require('librechat-data-provider').ErrorTypes.AUTH_FAILED); }); - it('should attempt to download and save the avatar if picture is provided', async () => { + it('should process and save the avatar through the shared avatar path if picture is provided', async () => { const { getStrategyFunctions } = require('~/server/services/Files/strategies'); const profile = { ...baseProfile }; @@ -457,7 +456,11 @@ u7wlOSk+oFzDIO/UILIA const { saveBuffer } = strategyResult.value; const [saveParams] = saveBuffer.mock.calls[0]; - expect(fetch).toHaveBeenCalled(); + expect(resizeAvatar).toHaveBeenCalledWith({ + userId: 'mock-user-id', + input: 'https://example.com/avatar.png', + }); + expect(fetch).not.toHaveBeenCalled(); expect(saveParams).toEqual( expect.objectContaining({ fileName: 'hashed-token.png', @@ -469,6 +472,38 @@ u7wlOSk+oFzDIO/UILIA expect(user.avatar).toBe('/fake/path/to/avatar.png'); }); + it('continues login when shared avatar processing rejects the picture URL', async () => { + const { getStrategyFunctions } = require('~/server/services/Files/strategies'); + const profile = { ...baseProfile }; + resizeAvatar.mockRejectedValueOnce(new Error('avatar processing failed')); + + const { user } = await validate(profile); + + expect(user).toBeTruthy(); + expect(user.avatar).toBeUndefined(); + expect(getStrategyFunctions).not.toHaveBeenCalled(); + }); + + it('uses the configured SAML picture claim for shared avatar processing', async () => { + process.env.SAML_PICTURE_CLAIM = 'avatar_url'; + verifyCallback = null; + await setupSaml(); + + const profile = { + ...baseProfile, + picture: 'https://example.com/ignored.png', + avatar_url: 'https://idp.example.com/custom-avatar.png', + }; + + await validate(profile); + + expect(resizeAvatar).toHaveBeenCalledWith({ + userId: 'mock-user-id', + input: 'https://idp.example.com/custom-avatar.png', + }); + expect(fetch).not.toHaveBeenCalled(); + }); + it('should save CloudFront SAML avatars under the shared avatar prefix', async () => { const { getStrategyFunctions } = require('~/server/services/Files/strategies'); getAppConfig.mockResolvedValueOnce({ fileStrategies: { avatar: FileSources.cloudfront } }); @@ -481,6 +516,11 @@ u7wlOSk+oFzDIO/UILIA const [saveParams] = saveBuffer.mock.calls[0]; expect(getStrategyFunctions).toHaveBeenLastCalledWith(FileSources.cloudfront); + expect(resizeAvatar).toHaveBeenCalledWith({ + userId: 'mock-user-id', + input: 'https://example.com/avatar.png', + }); + expect(fetch).not.toHaveBeenCalled(); expect(saveParams).toEqual( expect.objectContaining({ basePath: 'avatars', @@ -498,6 +538,7 @@ u7wlOSk+oFzDIO/UILIA await validate(profile); expect(fetch).not.toHaveBeenCalled(); + expect(resizeAvatar).not.toHaveBeenCalled(); }); it('should pass the found user to resolveAppConfigForUser', async () => {