mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-02-14 05:24:24 +01:00
🔒 fix: Secure Cookie Localhost Bypass and OpenID Token Selection in AuthService (#11782)
* 🔒 fix: Secure Cookie Localhost Bypass and OpenID Token Selection in AuthService Two independent bugs in `api/server/services/AuthService.js` cause complete authentication failure when using `OPENID_REUSE_TOKENS=true` with Microsoft Entra ID (or Auth0) on `http://localhost` with `NODE_ENV=production`: Bug 1: `secure: isProduction` prevents auth cookies on localhost PR #11518 introduced `shouldUseSecureCookie()` in `socialLogins.js` to handle the case where `NODE_ENV=production` but the server runs on `http://localhost`. However, `AuthService.js` was not updated — it still used `secure: isProduction` in 6 cookie locations across `setAuthTokens()` and `setOpenIDAuthTokens()`. The `token_provider` cookie being dropped is critical: without it, `requireJwtAuth` middleware defaults to the `jwt` strategy instead of `openidJwt`, causing all authenticated requests to return 401. Bug 2: `setOpenIDAuthTokens()` returns `access_token` instead of `id_token` The `openIdJwtStrategy` validates the Bearer token via JWKS. For Entra ID without `OPENID_AUDIENCE`, the `access_token` is a Microsoft Graph API token (opaque or signed for a different audience), which fails JWKS validation. The `id_token` is always a standard JWT signed by the IdP's JWKS keys with the app's `client_id` as audience — which is what the strategy expects. This is the same root cause as issue #8796 (Auth0 encrypted access tokens). Changes: - Consolidate `shouldUseSecureCookie()` into `packages/api/src/oauth/csrf.ts` as a shared, typed utility exported from `@librechat/api`, replacing the duplicate definitions in `AuthService.js` and `socialLogins.js` - Move `isProduction` check inside the function body so it is evaluated at call time rather than module load time - Fix `packages/api/src/oauth/csrf.ts` which also used bare `secure: isProduction` for CSRF and session cookies (same localhost bug) - Return `tokenset.id_token || tokenset.access_token` from `setOpenIDAuthTokens()` so JWKS validation works with standard OIDC providers; falls back to `access_token` for backward compatibility - Add 15 tests for `shouldUseSecureCookie()` covering production/dev modes, localhost variants, edge cases, and a documented IPv6 bracket limitation - Add 13 tests for `setOpenIDAuthTokens()` covering token selection, session storage, cookie secure flag delegation, and edge cases Refs: #8796, #11518, #11236, #9931 * chore: Adjust Import Order and Type Definitions in AgentPanel Component - Reordered imports in `AgentPanel.tsx` for better organization and clarity. - Updated type imports to ensure proper usage of `FieldNamesMarkedBoolean` and `TranslationKeys`. - Removed redundant imports to streamline the codebase.
This commit is contained in:
parent
3888dfa489
commit
2e42378b16
6 changed files with 431 additions and 50 deletions
|
|
@ -7,7 +7,13 @@ const {
|
|||
DEFAULT_REFRESH_TOKEN_EXPIRY,
|
||||
} = require('@librechat/data-schemas');
|
||||
const { ErrorTypes, SystemRoles, errorsToString } = require('librechat-data-provider');
|
||||
const { isEnabled, checkEmailConfig, isEmailDomainAllowed, math } = require('@librechat/api');
|
||||
const {
|
||||
math,
|
||||
isEnabled,
|
||||
checkEmailConfig,
|
||||
isEmailDomainAllowed,
|
||||
shouldUseSecureCookie,
|
||||
} = require('@librechat/api');
|
||||
const {
|
||||
findUser,
|
||||
findToken,
|
||||
|
|
@ -33,7 +39,6 @@ const domains = {
|
|||
server: process.env.DOMAIN_SERVER,
|
||||
};
|
||||
|
||||
const isProduction = process.env.NODE_ENV === 'production';
|
||||
const genericVerificationMessage = 'Please check your email to verify your email address.';
|
||||
|
||||
/**
|
||||
|
|
@ -392,13 +397,13 @@ const setAuthTokens = async (userId, res, _session = null) => {
|
|||
res.cookie('refreshToken', refreshToken, {
|
||||
expires: new Date(refreshTokenExpires),
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: shouldUseSecureCookie(),
|
||||
sameSite: 'strict',
|
||||
});
|
||||
res.cookie('token_provider', 'librechat', {
|
||||
expires: new Date(refreshTokenExpires),
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: shouldUseSecureCookie(),
|
||||
sameSite: 'strict',
|
||||
});
|
||||
return token;
|
||||
|
|
@ -419,7 +424,7 @@ const setAuthTokens = async (userId, res, _session = null) => {
|
|||
* @param {Object} req - request object (for session access)
|
||||
* @param {Object} res - response object
|
||||
* @param {string} [userId] - Optional MongoDB user ID for image path validation
|
||||
* @returns {String} - access token
|
||||
* @returns {String} - id_token (preferred) or access_token as the app auth token
|
||||
*/
|
||||
const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) => {
|
||||
try {
|
||||
|
|
@ -448,6 +453,15 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) =
|
|||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* Use id_token as the app authentication token (Bearer token for JWKS validation).
|
||||
* The id_token is always a standard JWT signed by the IdP's JWKS keys with the app's
|
||||
* client_id as audience. The access_token may be opaque or intended for a different
|
||||
* audience (e.g., Microsoft Graph API), which fails JWKS validation.
|
||||
* Falls back to access_token for providers where id_token is not available.
|
||||
*/
|
||||
const appAuthToken = tokenset.id_token || tokenset.access_token;
|
||||
|
||||
/** Store tokens server-side in session to avoid large cookies */
|
||||
if (req.session) {
|
||||
req.session.openidTokens = {
|
||||
|
|
@ -460,13 +474,13 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) =
|
|||
res.cookie('refreshToken', refreshToken, {
|
||||
expires: expirationDate,
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: shouldUseSecureCookie(),
|
||||
sameSite: 'strict',
|
||||
});
|
||||
res.cookie('openid_access_token', tokenset.access_token, {
|
||||
expires: expirationDate,
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: shouldUseSecureCookie(),
|
||||
sameSite: 'strict',
|
||||
});
|
||||
}
|
||||
|
|
@ -475,7 +489,7 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) =
|
|||
res.cookie('token_provider', 'openid', {
|
||||
expires: expirationDate,
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: shouldUseSecureCookie(),
|
||||
sameSite: 'strict',
|
||||
});
|
||||
if (userId && isEnabled(process.env.OPENID_REUSE_TOKENS)) {
|
||||
|
|
@ -486,11 +500,11 @@ const setOpenIDAuthTokens = (tokenset, req, res, userId, existingRefreshToken) =
|
|||
res.cookie('openid_user_id', signedUserId, {
|
||||
expires: expirationDate,
|
||||
httpOnly: true,
|
||||
secure: isProduction,
|
||||
secure: shouldUseSecureCookie(),
|
||||
sameSite: 'strict',
|
||||
});
|
||||
}
|
||||
return tokenset.access_token;
|
||||
return appAuthToken;
|
||||
} catch (error) {
|
||||
logger.error('[setOpenIDAuthTokens] Error in setting authentication tokens:', error);
|
||||
throw error;
|
||||
|
|
|
|||
269
api/server/services/AuthService.spec.js
Normal file
269
api/server/services/AuthService.spec.js
Normal file
|
|
@ -0,0 +1,269 @@
|
|||
jest.mock('@librechat/data-schemas', () => ({
|
||||
logger: { info: jest.fn(), warn: jest.fn(), debug: jest.fn(), error: jest.fn() },
|
||||
DEFAULT_SESSION_EXPIRY: 900000,
|
||||
DEFAULT_REFRESH_TOKEN_EXPIRY: 604800000,
|
||||
}));
|
||||
jest.mock('librechat-data-provider', () => ({
|
||||
ErrorTypes: {},
|
||||
SystemRoles: { USER: 'USER', ADMIN: 'ADMIN' },
|
||||
errorsToString: jest.fn(),
|
||||
}));
|
||||
jest.mock('@librechat/api', () => ({
|
||||
isEnabled: jest.fn((val) => val === 'true' || val === true),
|
||||
checkEmailConfig: jest.fn(),
|
||||
isEmailDomainAllowed: jest.fn(),
|
||||
math: jest.fn((val, fallback) => (val ? Number(val) : fallback)),
|
||||
shouldUseSecureCookie: jest.fn(() => false),
|
||||
}));
|
||||
jest.mock('~/models', () => ({
|
||||
findUser: jest.fn(),
|
||||
findToken: jest.fn(),
|
||||
createUser: jest.fn(),
|
||||
updateUser: jest.fn(),
|
||||
countUsers: jest.fn(),
|
||||
getUserById: jest.fn(),
|
||||
findSession: jest.fn(),
|
||||
createToken: jest.fn(),
|
||||
deleteTokens: jest.fn(),
|
||||
deleteSession: jest.fn(),
|
||||
createSession: jest.fn(),
|
||||
generateToken: jest.fn(),
|
||||
deleteUserById: jest.fn(),
|
||||
generateRefreshToken: jest.fn(),
|
||||
}));
|
||||
jest.mock('~/strategies/validators', () => ({ registerSchema: { parse: jest.fn() } }));
|
||||
jest.mock('~/server/services/Config', () => ({ getAppConfig: jest.fn() }));
|
||||
jest.mock('~/server/utils', () => ({ sendEmail: jest.fn() }));
|
||||
|
||||
const { shouldUseSecureCookie } = require('@librechat/api');
|
||||
const { setOpenIDAuthTokens } = require('./AuthService');
|
||||
|
||||
/** Helper to build a mock Express response */
|
||||
function mockResponse() {
|
||||
const cookies = {};
|
||||
const res = {
|
||||
cookie: jest.fn((name, value, options) => {
|
||||
cookies[name] = { value, options };
|
||||
}),
|
||||
_cookies: cookies,
|
||||
};
|
||||
return res;
|
||||
}
|
||||
|
||||
/** Helper to build a mock Express request with session */
|
||||
function mockRequest(sessionData = {}) {
|
||||
return {
|
||||
session: { openidTokens: null, ...sessionData },
|
||||
};
|
||||
}
|
||||
|
||||
describe('setOpenIDAuthTokens', () => {
|
||||
const env = process.env;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
process.env = {
|
||||
...env,
|
||||
JWT_REFRESH_SECRET: 'test-refresh-secret',
|
||||
OPENID_REUSE_TOKENS: 'true',
|
||||
};
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
process.env = env;
|
||||
});
|
||||
|
||||
describe('token selection (id_token vs access_token)', () => {
|
||||
it('should return id_token when both id_token and access_token are present', () => {
|
||||
const tokenset = {
|
||||
id_token: 'the-id-token',
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
expect(result).toBe('the-id-token');
|
||||
});
|
||||
|
||||
it('should return access_token when id_token is not available', () => {
|
||||
const tokenset = {
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
expect(result).toBe('the-access-token');
|
||||
});
|
||||
|
||||
it('should return access_token when id_token is undefined', () => {
|
||||
const tokenset = {
|
||||
id_token: undefined,
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
expect(result).toBe('the-access-token');
|
||||
});
|
||||
|
||||
it('should return access_token when id_token is null', () => {
|
||||
const tokenset = {
|
||||
id_token: null,
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
expect(result).toBe('the-access-token');
|
||||
});
|
||||
|
||||
it('should return id_token even when id_token and access_token differ', () => {
|
||||
const tokenset = {
|
||||
id_token: 'id-token-jwt-signed-by-idp',
|
||||
access_token: 'opaque-graph-api-token',
|
||||
refresh_token: 'refresh-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
expect(result).toBe('id-token-jwt-signed-by-idp');
|
||||
expect(result).not.toBe('opaque-graph-api-token');
|
||||
});
|
||||
});
|
||||
|
||||
describe('session token storage', () => {
|
||||
it('should store the original access_token in session (not id_token)', () => {
|
||||
const tokenset = {
|
||||
id_token: 'the-id-token',
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
|
||||
expect(req.session.openidTokens.accessToken).toBe('the-access-token');
|
||||
expect(req.session.openidTokens.refreshToken).toBe('the-refresh-token');
|
||||
});
|
||||
});
|
||||
|
||||
describe('cookie secure flag', () => {
|
||||
it('should call shouldUseSecureCookie for every cookie set', () => {
|
||||
const tokenset = {
|
||||
id_token: 'the-id-token',
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
|
||||
// token_provider + openid_user_id (session path, so no refreshToken/openid_access_token cookies)
|
||||
const secureCalls = shouldUseSecureCookie.mock.calls.length;
|
||||
expect(secureCalls).toBeGreaterThanOrEqual(2);
|
||||
|
||||
// Verify all cookies use the result of shouldUseSecureCookie
|
||||
for (const [, cookie] of Object.entries(res._cookies)) {
|
||||
expect(cookie.options.secure).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it('should set secure: true when shouldUseSecureCookie returns true', () => {
|
||||
shouldUseSecureCookie.mockReturnValue(true);
|
||||
|
||||
const tokenset = {
|
||||
id_token: 'the-id-token',
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
|
||||
for (const [, cookie] of Object.entries(res._cookies)) {
|
||||
expect(cookie.options.secure).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('should use shouldUseSecureCookie for cookie fallback path (no session)', () => {
|
||||
shouldUseSecureCookie.mockReturnValue(false);
|
||||
|
||||
const tokenset = {
|
||||
id_token: 'the-id-token',
|
||||
access_token: 'the-access-token',
|
||||
refresh_token: 'the-refresh-token',
|
||||
};
|
||||
const req = { session: null };
|
||||
const res = mockResponse();
|
||||
|
||||
setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
|
||||
// In the cookie fallback path, we get: refreshToken, openid_access_token, token_provider, openid_user_id
|
||||
expect(res.cookie).toHaveBeenCalledWith(
|
||||
'refreshToken',
|
||||
expect.any(String),
|
||||
expect.objectContaining({ secure: false }),
|
||||
);
|
||||
expect(res.cookie).toHaveBeenCalledWith(
|
||||
'openid_access_token',
|
||||
expect.any(String),
|
||||
expect.objectContaining({ secure: false }),
|
||||
);
|
||||
expect(res.cookie).toHaveBeenCalledWith(
|
||||
'token_provider',
|
||||
'openid',
|
||||
expect.objectContaining({ secure: false }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('edge cases', () => {
|
||||
it('should return undefined when tokenset is null', () => {
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
const result = setOpenIDAuthTokens(null, req, res, 'user-123');
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should return undefined when access_token is missing', () => {
|
||||
const tokenset = { refresh_token: 'refresh' };
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should return undefined when no refresh token is available', () => {
|
||||
const tokenset = { access_token: 'access', id_token: 'id' };
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123');
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should use existingRefreshToken when tokenset has no refresh_token', () => {
|
||||
const tokenset = {
|
||||
id_token: 'the-id-token',
|
||||
access_token: 'the-access-token',
|
||||
};
|
||||
const req = mockRequest();
|
||||
const res = mockResponse();
|
||||
|
||||
const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123', 'existing-refresh');
|
||||
expect(result).toBe('the-id-token');
|
||||
expect(req.session.openidTokens.refreshToken).toBe('existing-refresh');
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue