CERBERUS/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2026-08-03 22:32:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

🪪 fix: Use Shared IdP Avatar Processing (#13422)

Browse source 
* fix: Harden IdP avatar processing

* fix: Preserve trusted OpenID avatar auth
This commit is contained in:
Danny Avila 2026-05-30 19:51:58 -04:00 • committed by GitHub
parent 68d5958fe7
commit de760f6b51
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 197 additions and 69 deletions
Download patch file Download diff file Expand all files Collapse all files

8
api/server/services/Files/images/avatar.js
View file

@ -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<string, string> }} [params.fetchOptions] - Optional headers for trusted avatar URLs.
*
* @returns {Promise<any>}
* 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) {

20
api/server/services/Files/images/avatar.spec.js
View file

@ -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', () => {

86
api/strategies/openidStrategy.js
View file

@ -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<Buffer | string>} 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,
}),

66
api/strategies/openidStrategy.spec.js
View file

@ -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.
});

29
api/strategies/samlStrategy.js
View file

@ -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<Buffer>}
*/
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,
}),

57
api/strategies/samlStrategy.spec.js
View file

@ -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 () => {