mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-01-22 02:06:12 +01:00
🧮 refactor: Replace Eval with Safe Math Expression Parser (#11098)
* chore: Add mathjs dependency * refactor: Replace eval with mathjs for safer expression evaluation and improve session expiry handling to not environment variables from data-schemas package * test: Add integration tests for math function with environment variable expressions * refactor: Update test description for clarity on expiresIn behavior * refactor: Update test cases to clarify default expiration behavior for token generation * refactor: Improve error handling in math function for clearer evaluation errors
This commit is contained in:
parent
d0863de8d4
commit
6ffb176056
14 changed files with 602 additions and 85 deletions
|
|
@ -1,7 +1,9 @@
|
|||
import { createSessionMethods, type SessionMethods } from './session';
|
||||
import { createSessionMethods, DEFAULT_REFRESH_TOKEN_EXPIRY, type SessionMethods } from './session';
|
||||
import { createTokenMethods, type TokenMethods } from './token';
|
||||
import { createRoleMethods, type RoleMethods } from './role';
|
||||
import { createUserMethods, type UserMethods } from './user';
|
||||
import { createUserMethods, DEFAULT_SESSION_EXPIRY, type UserMethods } from './user';
|
||||
|
||||
export { DEFAULT_REFRESH_TOKEN_EXPIRY, DEFAULT_SESSION_EXPIRY };
|
||||
import { createKeyMethods, type KeyMethods } from './key';
|
||||
import { createFileMethods, type FileMethods } from './file';
|
||||
/* Memories */
|
||||
|
|
|
|||
|
|
@ -12,8 +12,8 @@ export class SessionError extends Error {
|
|||
}
|
||||
}
|
||||
|
||||
const { REFRESH_TOKEN_EXPIRY } = process.env ?? {};
|
||||
const expires = REFRESH_TOKEN_EXPIRY ? eval(REFRESH_TOKEN_EXPIRY) : 1000 * 60 * 60 * 24 * 7; // 7 days default
|
||||
/** Default refresh token expiry: 7 days in milliseconds */
|
||||
export const DEFAULT_REFRESH_TOKEN_EXPIRY = 1000 * 60 * 60 * 24 * 7;
|
||||
|
||||
// Factory function that takes mongoose instance and returns the methods
|
||||
export function createSessionMethods(mongoose: typeof import('mongoose')) {
|
||||
|
|
@ -28,11 +28,13 @@ export function createSessionMethods(mongoose: typeof import('mongoose')) {
|
|||
throw new SessionError('User ID is required', 'INVALID_USER_ID');
|
||||
}
|
||||
|
||||
const expiresIn = options.expiresIn ?? DEFAULT_REFRESH_TOKEN_EXPIRY;
|
||||
|
||||
try {
|
||||
const Session = mongoose.models.Session;
|
||||
const currentSession = new Session({
|
||||
user: userId,
|
||||
expiration: options.expiration || new Date(Date.now() + expires),
|
||||
expiration: options.expiration || new Date(Date.now() + expiresIn),
|
||||
});
|
||||
const refreshToken = await generateRefreshToken(currentSession);
|
||||
|
||||
|
|
@ -105,7 +107,10 @@ export function createSessionMethods(mongoose: typeof import('mongoose')) {
|
|||
async function updateExpiration(
|
||||
session: t.ISession | string,
|
||||
newExpiration?: Date,
|
||||
options: t.UpdateExpirationOptions = {},
|
||||
): Promise<t.ISession> {
|
||||
const expiresIn = options.expiresIn ?? DEFAULT_REFRESH_TOKEN_EXPIRY;
|
||||
|
||||
try {
|
||||
const Session = mongoose.models.Session;
|
||||
const sessionDoc = typeof session === 'string' ? await Session.findById(session) : session;
|
||||
|
|
@ -114,7 +119,7 @@ export function createSessionMethods(mongoose: typeof import('mongoose')) {
|
|||
throw new SessionError('Session not found', 'SESSION_NOT_FOUND');
|
||||
}
|
||||
|
||||
sessionDoc.expiration = newExpiration || new Date(Date.now() + expires);
|
||||
sessionDoc.expiration = newExpiration || new Date(Date.now() + expiresIn);
|
||||
return await sessionDoc.save();
|
||||
} catch (error) {
|
||||
logger.error('[updateExpiration] Error updating session:', error);
|
||||
|
|
@ -208,7 +213,9 @@ export function createSessionMethods(mongoose: typeof import('mongoose')) {
|
|||
}
|
||||
|
||||
try {
|
||||
const expiresIn = session.expiration ? session.expiration.getTime() : Date.now() + expires;
|
||||
const expiresIn = session.expiration
|
||||
? session.expiration.getTime()
|
||||
: Date.now() + DEFAULT_REFRESH_TOKEN_EXPIRY;
|
||||
|
||||
if (!session.expiration) {
|
||||
session.expiration = new Date(expiresIn);
|
||||
|
|
|
|||
|
|
@ -31,11 +31,10 @@ describe('User Methods', () => {
|
|||
} as IUser;
|
||||
|
||||
afterEach(() => {
|
||||
delete process.env.SESSION_EXPIRY;
|
||||
delete process.env.JWT_SECRET;
|
||||
});
|
||||
|
||||
it('should default to 15 minutes when SESSION_EXPIRY is not set', async () => {
|
||||
it('should default to 15 minutes when expiresIn is not provided', async () => {
|
||||
process.env.JWT_SECRET = 'test-secret';
|
||||
mockSignPayload.mockResolvedValue('mocked-token');
|
||||
|
||||
|
|
@ -49,16 +48,15 @@ describe('User Methods', () => {
|
|||
email: mockUser.email,
|
||||
},
|
||||
secret: 'test-secret',
|
||||
expirationTime: 900, // 15 minutes in seconds
|
||||
expirationTime: 900, // 15 minutes in seconds (DEFAULT_SESSION_EXPIRY / 1000)
|
||||
});
|
||||
});
|
||||
|
||||
it('should default to 15 minutes when SESSION_EXPIRY is empty string', async () => {
|
||||
process.env.SESSION_EXPIRY = '';
|
||||
it('should default to 15 minutes when expiresIn is undefined', async () => {
|
||||
process.env.JWT_SECRET = 'test-secret';
|
||||
mockSignPayload.mockResolvedValue('mocked-token');
|
||||
|
||||
await userMethods.generateToken(mockUser);
|
||||
await userMethods.generateToken(mockUser, undefined);
|
||||
|
||||
expect(mockSignPayload).toHaveBeenCalledWith({
|
||||
payload: {
|
||||
|
|
@ -68,16 +66,15 @@ describe('User Methods', () => {
|
|||
email: mockUser.email,
|
||||
},
|
||||
secret: 'test-secret',
|
||||
expirationTime: 900, // 15 minutes in seconds
|
||||
expirationTime: 900, // 15 minutes in seconds (DEFAULT_SESSION_EXPIRY / 1000)
|
||||
});
|
||||
});
|
||||
|
||||
it('should use custom expiry when SESSION_EXPIRY is set to a valid expression', async () => {
|
||||
process.env.SESSION_EXPIRY = '1000 * 60 * 30'; // 30 minutes
|
||||
it('should use custom expiry when expiresIn is provided', async () => {
|
||||
process.env.JWT_SECRET = 'test-secret';
|
||||
mockSignPayload.mockResolvedValue('mocked-token');
|
||||
|
||||
await userMethods.generateToken(mockUser);
|
||||
await userMethods.generateToken(mockUser, 1000 * 60 * 30); // 30 minutes
|
||||
|
||||
expect(mockSignPayload).toHaveBeenCalledWith({
|
||||
payload: {
|
||||
|
|
@ -91,12 +88,12 @@ describe('User Methods', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should default to 15 minutes when SESSION_EXPIRY evaluates to falsy value', async () => {
|
||||
process.env.SESSION_EXPIRY = '0'; // This will evaluate to 0, which is falsy
|
||||
it('should use 0 when expiresIn is 0', async () => {
|
||||
process.env.JWT_SECRET = 'test-secret';
|
||||
mockSignPayload.mockResolvedValue('mocked-token');
|
||||
|
||||
await userMethods.generateToken(mockUser);
|
||||
// When 0 is passed, it should use 0 (caller's responsibility to pass valid value)
|
||||
await userMethods.generateToken(mockUser, 0);
|
||||
|
||||
expect(mockSignPayload).toHaveBeenCalledWith({
|
||||
payload: {
|
||||
|
|
@ -106,7 +103,7 @@ describe('User Methods', () => {
|
|||
email: mockUser.email,
|
||||
},
|
||||
secret: 'test-secret',
|
||||
expirationTime: 900, // 15 minutes in seconds
|
||||
expirationTime: 0, // 0 seconds
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -119,45 +116,13 @@ describe('User Methods', () => {
|
|||
});
|
||||
|
||||
it('should return the token from signPayload', async () => {
|
||||
process.env.SESSION_EXPIRY = '1000 * 60 * 60'; // 1 hour
|
||||
process.env.JWT_SECRET = 'test-secret';
|
||||
const expectedToken = 'generated-jwt-token';
|
||||
mockSignPayload.mockResolvedValue(expectedToken);
|
||||
|
||||
const token = await userMethods.generateToken(mockUser);
|
||||
const token = await userMethods.generateToken(mockUser, 1000 * 60 * 60); // 1 hour
|
||||
|
||||
expect(token).toBe(expectedToken);
|
||||
});
|
||||
|
||||
it('should handle invalid SESSION_EXPIRY expressions gracefully', async () => {
|
||||
process.env.SESSION_EXPIRY = 'invalid expression';
|
||||
process.env.JWT_SECRET = 'test-secret';
|
||||
mockSignPayload.mockResolvedValue('mocked-token');
|
||||
|
||||
// Mock console.warn to verify it's called
|
||||
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
|
||||
|
||||
await userMethods.generateToken(mockUser);
|
||||
|
||||
// Should use default value when eval fails
|
||||
expect(mockSignPayload).toHaveBeenCalledWith({
|
||||
payload: {
|
||||
id: mockUser._id,
|
||||
username: mockUser.username,
|
||||
provider: mockUser.provider,
|
||||
email: mockUser.email,
|
||||
},
|
||||
secret: 'test-secret',
|
||||
expirationTime: 900, // 15 minutes in seconds (default)
|
||||
});
|
||||
|
||||
// Verify warning was logged
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(
|
||||
'Invalid SESSION_EXPIRY expression, using default:',
|
||||
expect.any(SyntaxError),
|
||||
);
|
||||
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,6 +2,9 @@ import mongoose, { FilterQuery } from 'mongoose';
|
|||
import type { IUser, BalanceConfig, CreateUserRequest, UserDeleteResult } from '~/types';
|
||||
import { signPayload } from '~/crypto';
|
||||
|
||||
/** Default JWT session expiry: 15 minutes in milliseconds */
|
||||
export const DEFAULT_SESSION_EXPIRY = 1000 * 60 * 15;
|
||||
|
||||
/** Factory function that takes mongoose instance and returns the methods */
|
||||
export function createUserMethods(mongoose: typeof import('mongoose')) {
|
||||
/**
|
||||
|
|
@ -161,24 +164,15 @@ export function createUserMethods(mongoose: typeof import('mongoose')) {
|
|||
|
||||
/**
|
||||
* Generates a JWT token for a given user.
|
||||
* @param user - The user object
|
||||
* @param expiresIn - Optional expiry time in milliseconds. Default: 15 minutes
|
||||
*/
|
||||
async function generateToken(user: IUser): Promise<string> {
|
||||
async function generateToken(user: IUser, expiresIn?: number): Promise<string> {
|
||||
if (!user) {
|
||||
throw new Error('No user provided');
|
||||
}
|
||||
|
||||
let expires = 1000 * 60 * 15;
|
||||
|
||||
if (process.env.SESSION_EXPIRY !== undefined && process.env.SESSION_EXPIRY !== '') {
|
||||
try {
|
||||
const evaluated = eval(process.env.SESSION_EXPIRY);
|
||||
if (evaluated) {
|
||||
expires = evaluated;
|
||||
}
|
||||
} catch (error) {
|
||||
console.warn('Invalid SESSION_EXPIRY expression, using default:', error);
|
||||
}
|
||||
}
|
||||
const expires = expiresIn ?? DEFAULT_SESSION_EXPIRY;
|
||||
|
||||
return await signPayload({
|
||||
payload: {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue