Andreas/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2025-09-22 06:00:56 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

🔐 fix: Image Validation when Reusing OpenID Token (#9458)

Browse source 
* 🔧 fix: Enhance OpenID token handling with user ID for image path validation

* 🔧 fix: Change logger level to error for user info fetch failure and remove redundant info log in OpenID user lookup

* 🔧 refactor: Remove validateImageRequest from middleware exports and enhance validation logic in validateImageRequest.js

* Removed validateImageRequest from the middleware index.
* Improved error handling and validation checks in validateImageRequest.js, including handling of OpenID tokens, URL length, and malformed URLs.
* Updated tests in validateImages.spec.js to cover new validation scenarios and edge cases.
This commit is contained in:
Danny Avila 2025-09-05 03:12:17 -04:00 • committed by GitHub
parent e705b09280
commit cd73cb0b3e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 401 additions and 152 deletions
Download patch file Download diff file Expand all files Collapse all files

2
api/server/controllers/AuthController.js
View file

@ -75,7 +75,7 @@ const refreshController = async (req, res) => {
if (!user) {
return res.status(401).redirect('/login');
}
const token = setOpenIDAuthTokens(tokenset, res);
const token = setOpenIDAuthTokens(tokenset, res, user._id.toString());
return res.status(200).send({ token, user });
} catch (error) {
logger.error('[refreshController] OpenID token refresh error', error);

4
api/server/index.js
View file

@ -12,7 +12,7 @@ const { logger } = require('@librechat/data-schemas');
const mongoSanitize = require('express-mongo-sanitize');
const { isEnabled, ErrorController } = require('@librechat/api');
const { connectDb, indexSync } = require('~/db');
const validateImageRequest = require('./middleware/validateImageRequest');
const createValidateImageRequest = require('./middleware/validateImageRequest');
const { jwtLogin, ldapLogin, passportLogin } = require('~/strategies');
const { updateInterfacePermissions } = require('~/models/interface');
const { checkMigrations } = require('./services/start/migration');
@ -126,7 +126,7 @@ const startServer = async () => {
app.use('/api/config', routes.config);
app.use('/api/assistants', routes.assistants);
app.use('/api/files', await routes.files.initialize());
app.use('/images/', validateImageRequest, routes.staticRoute);
app.use('/images/', createValidateImageRequest(appConfig.secureImageLinks), routes.staticRoute);
app.use('/api/share', routes.share);
app.use('/api/roles', routes.roles);
app.use('/api/agents', routes.agents);

2
api/server/middleware/index.js
View file

@ -1,6 +1,5 @@
const validatePasswordReset = require('./validatePasswordReset');
const validateRegistration = require('./validateRegistration');
const validateImageRequest = require('./validateImageRequest');
const buildEndpointOption = require('./buildEndpointOption');
const validateMessageReq = require('./validateMessageReq');
const checkDomainAllowed = require('./checkDomainAllowed');
@ -50,6 +49,5 @@ module.exports = {
validateMessageReq,
buildEndpointOption,
validateRegistration,
validateImageRequest,
validatePasswordReset,
};

200
api/server/middleware/spec/validateImages.spec.js
View file

@ -1,14 +1,14 @@
const jwt = require('jsonwebtoken');
const validateImageRequest = require('~/server/middleware/validateImageRequest');
const { isEnabled } = require('@librechat/api');
const createValidateImageRequest = require('~/server/middleware/validateImageRequest');
jest.mock('~/server/services/Config/app', () => ({
getAppConfig: jest.fn(),
jest.mock('@librechat/api', () => ({
isEnabled: jest.fn(),
}));
describe('validateImageRequest middleware', () => {
let req, res, next;
let req, res, next, validateImageRequest;
const validObjectId = '65cfb246f7ecadb8b1e8036b';
const { getAppConfig } = require('~/server/services/Config/app');
beforeEach(() => {
jest.clearAllMocks();
@ -22,23 +22,35 @@ describe('validateImageRequest middleware', () => {
};
next = jest.fn();
process.env.JWT_REFRESH_SECRET = 'test-secret';
process.env.OPENID_REUSE_TOKENS = 'false';
// Mock getAppConfig to return secureImageLinks: true by default
getAppConfig.mockResolvedValue({
secureImageLinks: true,
});
// Default: OpenID token reuse disabled
isEnabled.mockReturnValue(false);
});
afterEach(() => {
jest.clearAllMocks();
});
test('should call next() if secureImageLinks is false', async () => {
getAppConfig.mockResolvedValue({
secureImageLinks: false,
});
await validateImageRequest(req, res, next);
describe('Factory function', () => {
test('should return a pass-through middleware if secureImageLinks is false', async () => {
const middleware = createValidateImageRequest(false);
await middleware(req, res, next);
expect(next).toHaveBeenCalled();
expect(res.status).not.toHaveBeenCalled();
});
test('should return validation middleware if secureImageLinks is true', async () => {
validateImageRequest = createValidateImageRequest(true);
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(401);
expect(res.send).toHaveBeenCalledWith('Unauthorized');
});
});
describe('Standard LibreChat token flow', () => {
beforeEach(() => {
validateImageRequest = createValidateImageRequest(true);
});
test('should return 401 if refresh token is not provided', async () => {
@ -88,19 +100,17 @@ describe('validateImageRequest middleware', () => {
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
test('should return 403 for invalid ObjectId format', async () => {
test('should allow agent avatar pattern for any valid ObjectId', async () => {
const validToken = jwt.sign(
{ id: validObjectId, exp: Math.floor(Date.now() / 1000) + 3600 },
process.env.JWT_REFRESH_SECRET,
);
req.headers.cookie = `refreshToken=${validToken}`;
req.originalUrl = '/images/123/example.jpg'; // Invalid ObjectId
req.originalUrl = '/images/65cfb246f7ecadb8b1e8036c/agent-avatar-12345.png';
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
expect(next).toHaveBeenCalled();
});
// File traversal tests
test('should prevent file traversal attempts', async () => {
const validToken = jwt.sign(
{ id: validObjectId, exp: Math.floor(Date.now() / 1000) + 3600 },
@ -121,6 +131,9 @@ describe('validateImageRequest middleware', () => {
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
jest.clearAllMocks();
// Reset mocks for next iteration
res.status = jest.fn().mockReturnThis();
res.send = jest.fn();
}
});
@ -135,3 +148,152 @@ describe('validateImageRequest middleware', () => {
expect(next).toHaveBeenCalled();
});
});
describe('OpenID token flow', () => {
beforeEach(() => {
validateImageRequest = createValidateImageRequest(true);
// Enable OpenID token reuse
isEnabled.mockReturnValue(true);
process.env.OPENID_REUSE_TOKENS = 'true';
});
test('should return 403 if no OpenID user ID cookie when token_provider is openid', async () => {
req.headers.cookie = 'refreshToken=dummy-token; token_provider=openid';
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
test('should validate JWT-signed user ID for OpenID flow', async () => {
const signedUserId = jwt.sign(
{ id: validObjectId, exp: Math.floor(Date.now() / 1000) + 3600 },
process.env.JWT_REFRESH_SECRET,
);
req.headers.cookie = `refreshToken=dummy-token; token_provider=openid; openid_user_id=${signedUserId}`;
req.originalUrl = `/images/${validObjectId}/example.jpg`;
await validateImageRequest(req, res, next);
expect(next).toHaveBeenCalled();
});
test('should return 403 for invalid JWT-signed user ID', async () => {
req.headers.cookie =
'refreshToken=dummy-token; token_provider=openid; openid_user_id=invalid-jwt';
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
test('should return 403 for expired JWT-signed user ID', async () => {
const expiredSignedUserId = jwt.sign(
{ id: validObjectId, exp: Math.floor(Date.now() / 1000) - 3600 },
process.env.JWT_REFRESH_SECRET,
);
req.headers.cookie = `refreshToken=dummy-token; token_provider=openid; openid_user_id=${expiredSignedUserId}`;
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
test('should validate image path against JWT-signed user ID', async () => {
const signedUserId = jwt.sign(
{ id: validObjectId, exp: Math.floor(Date.now() / 1000) + 3600 },
process.env.JWT_REFRESH_SECRET,
);
const differentObjectId = '65cfb246f7ecadb8b1e8036c';
req.headers.cookie = `refreshToken=dummy-token; token_provider=openid; openid_user_id=${signedUserId}`;
req.originalUrl = `/images/${differentObjectId}/example.jpg`;
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
test('should allow agent avatars in OpenID flow', async () => {
const signedUserId = jwt.sign(
{ id: validObjectId, exp: Math.floor(Date.now() / 1000) + 3600 },
process.env.JWT_REFRESH_SECRET,
);
req.headers.cookie = `refreshToken=dummy-token; token_provider=openid; openid_user_id=${signedUserId}`;
req.originalUrl = '/images/65cfb246f7ecadb8b1e8036c/agent-avatar-12345.png';
await validateImageRequest(req, res, next);
expect(next).toHaveBeenCalled();
});
});
describe('Security edge cases', () => {
let validToken;
beforeEach(() => {
validateImageRequest = createValidateImageRequest(true);
validToken = jwt.sign(
{ id: validObjectId, exp: Math.floor(Date.now() / 1000) + 3600 },
process.env.JWT_REFRESH_SECRET,
);
});
test('should handle very long image filenames', async () => {
const longFilename = 'a'.repeat(1000) + '.jpg';
req.headers.cookie = `refreshToken=${validToken}`;
req.originalUrl = `/images/${validObjectId}/${longFilename}`;
await validateImageRequest(req, res, next);
expect(next).toHaveBeenCalled();
});
test('should handle URLs with maximum practical length', async () => {
// Most browsers support URLs up to ~2000 characters
const longFilename = 'x'.repeat(1900) + '.jpg';
req.headers.cookie = `refreshToken=${validToken}`;
req.originalUrl = `/images/${validObjectId}/${longFilename}`;
await validateImageRequest(req, res, next);
expect(next).toHaveBeenCalled();
});
test('should accept URLs just under the 2048 limit', async () => {
// Create a URL exactly 2047 characters long
const baseLength = `/images/${validObjectId}/`.length + '.jpg'.length;
const filenameLength = 2047 - baseLength;
const filename = 'a'.repeat(filenameLength) + '.jpg';
req.headers.cookie = `refreshToken=${validToken}`;
req.originalUrl = `/images/${validObjectId}/${filename}`;
await validateImageRequest(req, res, next);
expect(next).toHaveBeenCalled();
});
test('should handle malformed URL encoding gracefully', async () => {
req.headers.cookie = `refreshToken=${validToken}`;
req.originalUrl = `/images/${validObjectId}/test%ZZinvalid.jpg`;
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
test('should reject URLs with null bytes', async () => {
req.headers.cookie = `refreshToken=${validToken}`;
req.originalUrl = `/images/${validObjectId}/test\x00.jpg`;
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
test('should handle URLs with repeated slashes', async () => {
req.headers.cookie = `refreshToken=${validToken}`;
req.originalUrl = `/images/${validObjectId}//test.jpg`;
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
test('should reject extremely long URLs as potential DoS', async () => {
// Create a URL longer than 2048 characters
const baseLength = `/images/${validObjectId}/`.length + '.jpg'.length;
const filenameLength = 2049 - baseLength; // Ensure total length exceeds 2048
const extremelyLongFilename = 'x'.repeat(filenameLength) + '.jpg';
req.headers.cookie = `refreshToken=${validToken}`;
req.originalUrl = `/images/${validObjectId}/${extremelyLongFilename}`;
// Verify our test URL is actually too long
expect(req.originalUrl.length).toBeGreaterThan(2048);
await validateImageRequest(req, res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith('Access Denied');
});
});
});

133
api/server/middleware/validateImageRequest.js
View file

@ -1,7 +1,7 @@
const cookies = require('cookie');
const jwt = require('jsonwebtoken');
const { isEnabled } = require('@librechat/api');
const { logger } = require('@librechat/data-schemas');
const { getAppConfig } = require('~/server/services/Config/app');
const OBJECT_ID_LENGTH = 24;
const OBJECT_ID_PATTERN = /^[0-9a-f]{24}$/i;
@ -22,42 +22,116 @@ function isValidObjectId(id) {
}
/**
* Middleware to validate image request.
* Must be set by `secureImageLinks` via custom config file.
* Validates a LibreChat refresh token
* @param {string} refreshToken - The refresh token to validate
* @returns {{valid: boolean, userId?: string, error?: string}} - Validation result
*/
async function validateImageRequest(req, res, next) {
const appConfig = await getAppConfig({ role: req.user?.role });
if (!appConfig.secureImageLinks) {
return next();
}
const refreshToken = req.headers.cookie ? cookies.parse(req.headers.cookie).refreshToken : null;
if (!refreshToken) {
logger.warn('[validateImageRequest] Refresh token not provided');
return res.status(401).send('Unauthorized');
}
let payload;
function validateToken(refreshToken) {
try {
payload = jwt.verify(refreshToken, process.env.JWT_REFRESH_SECRET);
} catch (err) {
logger.warn('[validateImageRequest]', err);
return res.status(403).send('Access Denied');
}
const payload = jwt.verify(refreshToken, process.env.JWT_REFRESH_SECRET);
if (!isValidObjectId(payload.id)) {
logger.warn('[validateImageRequest] Invalid User ID');
return res.status(403).send('Access Denied');
return { valid: false, error: 'Invalid User ID' };
}
const currentTimeInSeconds = Math.floor(Date.now() / 1000);
if (payload.exp < currentTimeInSeconds) {
logger.warn('[validateImageRequest] Refresh token expired');
return { valid: false, error: 'Refresh token expired' };
}
return { valid: true, userId: payload.id };
} catch (err) {
logger.warn('[validateToken]', err);
return { valid: false, error: 'Invalid token' };
}
}
/**
* Factory to create the `validateImageRequest` middleware with configured secureImageLinks
* @param {boolean} [secureImageLinks] - Whether secure image links are enabled
*/
function createValidateImageRequest(secureImageLinks) {
if (!secureImageLinks) {
return (_req, _res, next) => next();
}
/**
* Middleware to validate image request.
* Supports both LibreChat refresh tokens and OpenID JWT tokens.
* Must be set by `secureImageLinks` via custom config file.
*/
return async function validateImageRequest(req, res, next) {
try {
const cookieHeader = req.headers.cookie;
if (!cookieHeader) {
logger.warn('[validateImageRequest] No cookies provided');
return res.status(401).send('Unauthorized');
}
const parsedCookies = cookies.parse(cookieHeader);
const refreshToken = parsedCookies.refreshToken;
if (!refreshToken) {
logger.warn('[validateImageRequest] Token not provided');
return res.status(401).send('Unauthorized');
}
const tokenProvider = parsedCookies.token_provider;
let userIdForPath;
if (tokenProvider === 'openid' && isEnabled(process.env.OPENID_REUSE_TOKENS)) {
const openidUserId = parsedCookies.openid_user_id;
if (!openidUserId) {
logger.warn('[validateImageRequest] No OpenID user ID cookie found');
return res.status(403).send('Access Denied');
}
const fullPath = decodeURIComponent(req.originalUrl);
const pathPattern = new RegExp(`^/images/${payload.id}/[^/]+$`);
const validationResult = validateToken(openidUserId);
if (!validationResult.valid) {
logger.warn(`[validateImageRequest] ${validationResult.error}`);
return res.status(403).send('Access Denied');
}
userIdForPath = validationResult.userId;
} else {
const validationResult = validateToken(refreshToken);
if (!validationResult.valid) {
logger.warn(`[validateImageRequest] ${validationResult.error}`);
return res.status(403).send('Access Denied');
}
userIdForPath = validationResult.userId;
}
if (!userIdForPath) {
logger.warn('[validateImageRequest] No user ID available for path validation');
return res.status(403).send('Access Denied');
}
const MAX_URL_LENGTH = 2048;
if (req.originalUrl.length > MAX_URL_LENGTH) {
logger.warn('[validateImageRequest] URL too long');
return res.status(403).send('Access Denied');
}
if (req.originalUrl.includes('\x00')) {
logger.warn('[validateImageRequest] URL contains null byte');
return res.status(403).send('Access Denied');
}
let fullPath;
try {
fullPath = decodeURIComponent(req.originalUrl);
} catch {
logger.warn('[validateImageRequest] Invalid URL encoding');
return res.status(403).send('Access Denied');
}
const agentAvatarPattern = /^\/images\/[a-f0-9]{24}\/agent-[^/]*$/;
if (agentAvatarPattern.test(fullPath)) {
logger.debug('[validateImageRequest] Image request validated');
return next();
}
const escapedUserId = userIdForPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const pathPattern = new RegExp(`^/images/${escapedUserId}/[^/]+$`);
if (pathPattern.test(fullPath)) {
logger.debug('[validateImageRequest] Image request validated');
@ -66,6 +140,11 @@ async function validateImageRequest(req, res, next) {
logger.warn('[validateImageRequest] Invalid image path');
res.status(403).send('Access Denied');
}
} catch (error) {
logger.error('[validateImageRequest] Error:', error);
res.status(500).send('Internal Server Error');
}
};
}
module.exports = validateImageRequest;
module.exports = createValidateImageRequest;

2
api/server/routes/oauth.js
View file

@ -39,7 +39,7 @@ const oauthHandler = async (req, res) => {
isEnabled(process.env.OPENID_REUSE_TOKENS) === true
) {
await syncUserEntraGroupMemberships(req.user, req.user.tokenset.access_token);
setOpenIDAuthTokens(req.user.tokenset, res);
setOpenIDAuthTokens(req.user.tokenset, res, req.user._id.toString());
} else {
await setAuthTokens(req.user._id, res);
}

15
api/server/services/AuthService.js
View file

@ -402,9 +402,10 @@ const setAuthTokens = async (userId, res, sessionId = null) => {
* @param {import('openid-client').TokenEndpointResponse & import('openid-client').TokenEndpointResponseHelpers} tokenset
* - The tokenset object containing access and refresh tokens
* @param {Object} res - response object
* @param {string} [userId] - Optional MongoDB user ID for image path validation
* @returns {String} - access token
*/
const setOpenIDAuthTokens = (tokenset, res) => {
const setOpenIDAuthTokens = (tokenset, res, userId) => {
try {
if (!tokenset) {
logger.error('[setOpenIDAuthTokens] No tokenset found in request');
@ -435,6 +436,18 @@ const setOpenIDAuthTokens = (tokenset, res) => {
secure: isProduction,
sameSite: 'strict',
});
if (userId && isEnabled(process.env.OPENID_REUSE_TOKENS)) {
/** JWT-signed user ID cookie for image path validation when OPENID_REUSE_TOKENS is enabled */
const signedUserId = jwt.sign({ id: userId }, process.env.JWT_REFRESH_SECRET, {
expiresIn: expiryInMilliseconds / 1000,
});
res.cookie('openid_user_id', signedUserId, {
expires: expirationDate,
httpOnly: true,
secure: isProduction,
sameSite: 'strict',
});
}
return tokenset.access_token;
} catch (error) {
logger.error('[setOpenIDAuthTokens] Error in setting authentication tokens:', error);

2
api/strategies/openidStrategy.js
View file

@ -183,7 +183,7 @@ const getUserInfo = async (config, accessToken, sub) => {
const exchangedAccessToken = await exchangeAccessTokenIfNeeded(config, accessToken, sub);
return await client.fetchUserInfo(config, exchangedAccessToken, sub);
} catch (error) {
logger.warn(`[openidStrategy] getUserInfo: Error fetching user info: ${error}`);
logger.error('[openidStrategy] getUserInfo: Error fetching user info:', error);
return null;
}
};

3
packages/api/src/auth/openid.ts
View file

@ -17,9 +17,6 @@ export async function findOpenIDUser({
strategyName?: string;
}): Promise<{ user: IUser | null; error: string | null; migration: boolean }> {
let user = await findUser({ openidId });
logger.info(`[${strategyName}] user ${user ? 'found' : 'not found'} with openidId: ${openidId}`);
// If user not found by openidId, try to find by email
if (!user && email) {
user = await findUser({ email });
logger.warn(