From 795a56c39ba2446f242e278ba5dc1019822a5cb5 Mon Sep 17 00:00:00 2001 From: "Chris (ChrisJr404)" <11917633+ChrisJr404@users.noreply.github.com> Date: Tue, 12 May 2026 15:58:58 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8D=AA=20feat(auth):=20make=20cookie=20Sa?= =?UTF-8?q?meSite=20configurable=20via=20COOKIE=5FSAMESITE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auth cookies set by AuthService (refreshToken, token_provider, openid_access_token, openid_id_token, openid_user_id) were hardcoded to SameSite=Strict, which breaks deployments where the API is loaded cross-origin (embedded iframe, separate SPA origin, OAuth redirect with non-same-site landing page). Adds a `COOKIE_SAMESITE` env var accepting `strict | lax | none` (case-insensitive). When unset or set to an unrecognized value it falls back to `strict`, preserving existing behavior. The decision is centralized in a new `getAuthCookieOptions()` helper in `packages/api/src/oauth/csrf.ts` so every cookie-setting site stays in sync. Browsers require Secure for SameSite=None or they silently drop the cookie. `getAuthCookieOptions()` enforces that invariant by forcing `secure: true` whenever `COOKIE_SAMESITE=none`, regardless of the local `shouldUseSecureCookie()` heuristic. Out of scope: the OAuth CSRF cookies in `csrf.ts` and the CloudFront signed cookies in `cdn/cloudfront-cookies.ts` keep their existing explicit values (`lax` and `none` respectively) since those are required by their respective flows. Closes #4610 Signed-off-by: Chris (ChrisJr404) <11917633+ChrisJr404@users.noreply.github.com> --- .env.example | 8 ++ api/server/services/AuthService.js | 25 +++--- api/server/services/AuthService.spec.js | 77 +++++++++++++--- packages/api/src/oauth/csrf.spec.ts | 111 +++++++++++++++++++++++- packages/api/src/oauth/csrf.ts | 37 ++++++++ 5 files changed, 229 insertions(+), 29 deletions(-) diff --git a/.env.example b/.env.example index 5d7f69a9de..ed6b94dd67 100644 --- a/.env.example +++ b/.env.example @@ -472,6 +472,14 @@ ALLOW_UNVERIFIED_EMAIL_LOGIN=true SESSION_EXPIRY=1000 * 60 * 15 REFRESH_TOKEN_EXPIRY=(1000 * 60 * 60 * 24) * 7 +# Controls the SameSite attribute for authentication cookies (refreshToken, +# token_provider, openid_*). Accepts: strict | lax | none. Defaults to strict +# when unset or set to an unrecognized value. Use "none" only when the API is +# embedded cross-origin (e.g. an iframe on another domain or a separate SPA +# origin); the browser requires Secure for SameSite=None, so the API will +# force Secure=true in that case regardless of NODE_ENV/DOMAIN_SERVER. +# COOKIE_SAMESITE=strict + JWT_SECRET=16f8c0ef4a5d391b26034086c628469d3f9f497f08163ab9b40137092f2909ef JWT_REFRESH_SECRET=eaa5191f2914e30b9387fd84e254e4ba6fc51b4654968a9b0803b456a54b8418 diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index 943c5a81f6..50cde5b85d 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -16,7 +16,7 @@ const { parseCloudFrontCookieScope, CLOUDFRONT_SCOPE_COOKIE, isEmailDomainAllowed, - shouldUseSecureCookie, + getAuthCookieOptions, resolveAppConfigForUser, } = require('@librechat/api'); const { @@ -532,17 +532,16 @@ const setAuthTokens = async (userId, res, _session = null, req = null) => { const sessionExpiry = math(process.env.SESSION_EXPIRY, DEFAULT_SESSION_EXPIRY); const token = await generateToken(user, sessionExpiry); + const cookieOptions = getAuthCookieOptions(); res.cookie('refreshToken', refreshToken, { expires: new Date(refreshTokenExpires), httpOnly: true, - secure: shouldUseSecureCookie(), - sameSite: 'strict', + ...cookieOptions, }); res.cookie('token_provider', 'librechat', { expires: new Date(refreshTokenExpires), httpOnly: true, - secure: shouldUseSecureCookie(), - sameSite: 'strict', + ...cookieOptions, }); setCloudFrontAuthCookies(req, res, user, { userId: user?._id ?? userId }); @@ -644,11 +643,11 @@ const setOpenIDAuthTokens = ( * The refresh token is small (opaque string) so it doesn't hit the HTTP/2 header * size limits that motivated session storage for the larger access_token/id_token. */ + const cookieOptions = getAuthCookieOptions(); res.cookie('refreshToken', refreshToken, { expires: expirationDate, httpOnly: true, - secure: shouldUseSecureCookie(), - sameSite: 'strict', + ...cookieOptions, }); /** Store tokens server-side in session to avoid large cookies */ @@ -665,15 +664,13 @@ const setOpenIDAuthTokens = ( res.cookie('openid_access_token', tokenset.access_token, { expires: expirationDate, httpOnly: true, - secure: shouldUseSecureCookie(), - sameSite: 'strict', + ...cookieOptions, }); if (tokenset.id_token) { res.cookie('openid_id_token', tokenset.id_token, { expires: expirationDate, httpOnly: true, - secure: shouldUseSecureCookie(), - sameSite: 'strict', + ...cookieOptions, }); } } @@ -682,8 +679,7 @@ const setOpenIDAuthTokens = ( res.cookie('token_provider', 'openid', { expires: expirationDate, httpOnly: true, - secure: shouldUseSecureCookie(), - sameSite: 'strict', + ...cookieOptions, }); if (userId && isEnabled(process.env.OPENID_REUSE_TOKENS)) { /** JWT-signed user ID cookie for image path validation when OPENID_REUSE_TOKENS is enabled */ @@ -693,8 +689,7 @@ const setOpenIDAuthTokens = ( res.cookie('openid_user_id', signedUserId, { expires: expirationDate, httpOnly: true, - secure: shouldUseSecureCookie(), - sameSite: 'strict', + ...cookieOptions, }); } diff --git a/api/server/services/AuthService.spec.js b/api/server/services/AuthService.spec.js index c81b78e8e1..3a7adb9fc3 100644 --- a/api/server/services/AuthService.spec.js +++ b/api/server/services/AuthService.spec.js @@ -13,7 +13,7 @@ jest.mock('@librechat/api', () => ({ checkEmailConfig: jest.fn(), isEmailDomainAllowed: jest.fn(), math: jest.fn((val, fallback) => (val ? Number(val) : fallback)), - shouldUseSecureCookie: jest.fn(() => false), + getAuthCookieOptions: jest.fn(() => ({ secure: false, sameSite: 'strict' })), resolveAppConfigForUser: jest.fn(async (_getAppConfig, _user) => ({})), setCloudFrontCookies: jest.fn(() => true), getCloudFrontConfig: jest.fn(() => ({ @@ -47,7 +47,7 @@ jest.mock('~/server/services/Config', () => ({ getAppConfig: jest.fn() })); jest.mock('~/server/utils', () => ({ sendEmail: jest.fn() })); const { - shouldUseSecureCookie, + getAuthCookieOptions, isEmailDomainAllowed, resolveAppConfigForUser, setCloudFrontCookies, @@ -271,7 +271,7 @@ describe('setOpenIDAuthTokens', () => { }); describe('cookie secure flag', () => { - it('should call shouldUseSecureCookie for every cookie set', () => { + it('should apply getAuthCookieOptions to every cookie set', () => { const tokenset = { id_token: 'the-id-token', access_token: 'the-access-token', @@ -283,17 +283,17 @@ describe('setOpenIDAuthTokens', () => { 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); + expect(getAuthCookieOptions.mock.calls.length).toBeGreaterThanOrEqual(1); - // Verify all cookies use the result of shouldUseSecureCookie + // Default mock returns { secure: false, sameSite: 'strict' }; every cookie should carry it. for (const [, cookie] of Object.entries(res._cookies)) { expect(cookie.options.secure).toBe(false); + expect(cookie.options.sameSite).toBe('strict'); } }); - it('should set secure: true when shouldUseSecureCookie returns true', () => { - shouldUseSecureCookie.mockReturnValue(true); + it('should set secure: true when getAuthCookieOptions returns secure=true', () => { + getAuthCookieOptions.mockReturnValue({ secure: true, sameSite: 'strict' }); const tokenset = { id_token: 'the-id-token', @@ -307,11 +307,12 @@ describe('setOpenIDAuthTokens', () => { for (const [, cookie] of Object.entries(res._cookies)) { expect(cookie.options.secure).toBe(true); + expect(cookie.options.sameSite).toBe('strict'); } }); - it('should use shouldUseSecureCookie for cookie fallback path (no session)', () => { - shouldUseSecureCookie.mockReturnValue(false); + it('should propagate getAuthCookieOptions to the cookie fallback path (no session)', () => { + getAuthCookieOptions.mockReturnValue({ secure: false, sameSite: 'strict' }); const tokenset = { id_token: 'the-id-token', @@ -327,19 +328,44 @@ describe('setOpenIDAuthTokens', () => { expect(res.cookie).toHaveBeenCalledWith( 'refreshToken', expect.any(String), - expect.objectContaining({ secure: false }), + expect.objectContaining({ secure: false, sameSite: 'strict' }), ); expect(res.cookie).toHaveBeenCalledWith( 'openid_access_token', expect.any(String), - expect.objectContaining({ secure: false }), + expect.objectContaining({ secure: false, sameSite: 'strict' }), ); expect(res.cookie).toHaveBeenCalledWith( 'token_provider', 'openid', - expect.objectContaining({ secure: false }), + expect.objectContaining({ secure: false, sameSite: 'strict' }), ); }); + + it.each([ + ['strict', false], + ['lax', false], + ['none', true], + ])('should apply sameSite=%s (secure=%s) to every cookie set', (sameSite, secure) => { + getAuthCookieOptions.mockReturnValue({ secure, sameSite }); + + 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'); + + // 4 cookies on the fallback path. Each must carry the configured options. + expect(Object.keys(res._cookies).length).toBeGreaterThanOrEqual(4); + for (const [, cookie] of Object.entries(res._cookies)) { + expect(cookie.options.secure).toBe(secure); + expect(cookie.options.sameSite).toBe(sameSite); + } + }); }); describe('edge cases', () => { @@ -818,5 +844,30 @@ describe('CloudFront cookie integration', () => { expect(result).toBe('mock-access-token'); }); + + it.each([ + ['strict', false], + ['lax', false], + ['none', true], + ])( + 'applies sameSite=%s (secure=%s) to refreshToken and token_provider', + async (sameSite, secure) => { + getAuthCookieOptions.mockReturnValue({ secure, sameSite }); + const res = mockResponse(); + + await setAuthTokens('user-123', res); + + expect(res.cookie).toHaveBeenCalledWith( + 'refreshToken', + expect.any(String), + expect.objectContaining({ secure, sameSite }), + ); + expect(res.cookie).toHaveBeenCalledWith( + 'token_provider', + 'librechat', + expect.objectContaining({ secure, sameSite }), + ); + }, + ); }); }); diff --git a/packages/api/src/oauth/csrf.spec.ts b/packages/api/src/oauth/csrf.spec.ts index b56f1fd38f..e1c3a1ce80 100644 --- a/packages/api/src/oauth/csrf.spec.ts +++ b/packages/api/src/oauth/csrf.spec.ts @@ -1,4 +1,4 @@ -import { shouldUseSecureCookie } from './csrf'; +import { shouldUseSecureCookie, getAuthCookieSameSite, getAuthCookieOptions } from './csrf'; describe('shouldUseSecureCookie', () => { const originalEnv = process.env; @@ -97,3 +97,112 @@ describe('shouldUseSecureCookie', () => { }); }); }); + +describe('getAuthCookieSameSite', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + }); + + afterAll(() => { + process.env = originalEnv; + }); + + it('should default to strict when COOKIE_SAMESITE is unset', () => { + delete process.env.COOKIE_SAMESITE; + expect(getAuthCookieSameSite()).toBe('strict'); + }); + + it('should default to strict when COOKIE_SAMESITE is empty', () => { + process.env.COOKIE_SAMESITE = ''; + expect(getAuthCookieSameSite()).toBe('strict'); + }); + + it('should return strict when COOKIE_SAMESITE=strict', () => { + process.env.COOKIE_SAMESITE = 'strict'; + expect(getAuthCookieSameSite()).toBe('strict'); + }); + + it('should return lax when COOKIE_SAMESITE=lax', () => { + process.env.COOKIE_SAMESITE = 'lax'; + expect(getAuthCookieSameSite()).toBe('lax'); + }); + + it('should return none when COOKIE_SAMESITE=none', () => { + process.env.COOKIE_SAMESITE = 'none'; + expect(getAuthCookieSameSite()).toBe('none'); + }); + + it('should accept case-insensitive values', () => { + process.env.COOKIE_SAMESITE = 'LAX'; + expect(getAuthCookieSameSite()).toBe('lax'); + process.env.COOKIE_SAMESITE = 'None'; + expect(getAuthCookieSameSite()).toBe('none'); + process.env.COOKIE_SAMESITE = 'Strict'; + expect(getAuthCookieSameSite()).toBe('strict'); + }); + + it('should trim whitespace before matching', () => { + process.env.COOKIE_SAMESITE = ' lax '; + expect(getAuthCookieSameSite()).toBe('lax'); + }); + + it('should fall back to strict on unrecognized values', () => { + process.env.COOKIE_SAMESITE = 'nope'; + expect(getAuthCookieSameSite()).toBe('strict'); + process.env.COOKIE_SAMESITE = 'true'; + expect(getAuthCookieSameSite()).toBe('strict'); + process.env.COOKIE_SAMESITE = 'STRICTLY'; + expect(getAuthCookieSameSite()).toBe('strict'); + }); +}); + +describe('getAuthCookieOptions', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { ...originalEnv }; + process.env.NODE_ENV = 'development'; + process.env.DOMAIN_SERVER = 'http://localhost:3080'; + delete process.env.COOKIE_SAMESITE; + }); + + afterAll(() => { + process.env = originalEnv; + }); + + it('should default to { secure: shouldUseSecureCookie(), sameSite: "strict" }', () => { + expect(getAuthCookieOptions()).toEqual({ secure: false, sameSite: 'strict' }); + }); + + it('should pass sameSite=lax through and use the secure heuristic', () => { + process.env.COOKIE_SAMESITE = 'lax'; + expect(getAuthCookieOptions()).toEqual({ secure: false, sameSite: 'lax' }); + }); + + it('should force secure=true when sameSite=none even on localhost', () => { + process.env.COOKIE_SAMESITE = 'none'; + // shouldUseSecureCookie() would return false here, but SameSite=None + // requires Secure or browsers will drop the cookie. + expect(getAuthCookieOptions()).toEqual({ secure: true, sameSite: 'none' }); + }); + + it('should keep secure=true when sameSite=none in production', () => { + process.env.NODE_ENV = 'production'; + process.env.DOMAIN_SERVER = 'https://chat.example.com'; + process.env.COOKIE_SAMESITE = 'none'; + expect(getAuthCookieOptions()).toEqual({ secure: true, sameSite: 'none' }); + }); + + it('should use shouldUseSecureCookie() result when sameSite=strict in production', () => { + process.env.NODE_ENV = 'production'; + process.env.DOMAIN_SERVER = 'https://chat.example.com'; + expect(getAuthCookieOptions()).toEqual({ secure: true, sameSite: 'strict' }); + }); + + it('should fall back to strict for unrecognized sameSite values', () => { + process.env.COOKIE_SAMESITE = 'bogus'; + expect(getAuthCookieOptions()).toEqual({ secure: false, sameSite: 'strict' }); + }); +}); diff --git a/packages/api/src/oauth/csrf.ts b/packages/api/src/oauth/csrf.ts index 6ed63968d1..3775877e73 100644 --- a/packages/api/src/oauth/csrf.ts +++ b/packages/api/src/oauth/csrf.ts @@ -40,6 +40,43 @@ export function shouldUseSecureCookie(): boolean { return isProduction && !isLocalhost; } +export type AuthCookieSameSite = 'strict' | 'lax' | 'none'; + +/** + * Resolves the SameSite attribute for authentication cookies from the + * `COOKIE_SAMESITE` environment variable. Accepts `strict`, `lax`, or `none` + * (case-insensitive). Anything else (including unset) falls back to `strict`, + * which preserves the historical behavior. + * + * Use `getAuthCookieOptions()` instead of calling this directly when setting + * cookies, so the `SameSite=None` requires `Secure` invariant is enforced. + */ +export function getAuthCookieSameSite(): AuthCookieSameSite { + const raw = (process.env.COOKIE_SAMESITE || '').trim().toLowerCase(); + if (raw === 'lax' || raw === 'none' || raw === 'strict') { + return raw; + } + return 'strict'; +} + +/** + * Returns the `{ secure, sameSite }` pair that should be applied to every + * authentication cookie set by the API. Centralizing the choice keeps all + * cookie-setting sites in sync and enforces the browser rule that + * `SameSite=None` cookies must also be `Secure`. When the user opts into + * `SameSite=None`, `secure` is forced to `true` regardless of the local + * `shouldUseSecureCookie()` heuristic, because non-secure SameSite=None + * cookies are silently dropped by modern browsers. + */ +export function getAuthCookieOptions(): { + secure: boolean; + sameSite: AuthCookieSameSite; +} { + const sameSite = getAuthCookieSameSite(); + const secure = sameSite === 'none' ? true : shouldUseSecureCookie(); + return { secure, sameSite }; +} + /** Generates an HMAC-based token for OAuth CSRF protection */ export function generateOAuthCsrfToken(flowId: string, secret?: string): string { const key = secret || process.env.JWT_SECRET;