From 7b9a57a4677c233a1156fbf0cc7b66f186cf0d55 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 11 May 2026 23:29:01 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20fix:=20Harden=20OpenID?= =?UTF-8?q?=20Session=20Token=20Reuse=20(#13086)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Harden OpenID Session Token Reuse * fix: Preserve OpenID Session Token On Forced Refresh * fix: Gate Preserved OpenID Id Token By Expiry * test: Cover OpenID Id Token Expiry Buffer --- api/server/controllers/AuthController.js | 32 +- api/server/controllers/AuthController.spec.js | 340 +++++++++++++++++- api/server/services/AuthService.js | 32 +- api/server/services/AuthService.spec.js | 84 ++++- 4 files changed, 457 insertions(+), 31 deletions(-) diff --git a/api/server/controllers/AuthController.js b/api/server/controllers/AuthController.js index 3f24e56abe..527728e98c 100644 --- a/api/server/controllers/AuthController.js +++ b/api/server/controllers/AuthController.js @@ -27,6 +27,9 @@ const { getGraphApiToken } = require('~/server/services/GraphTokenService'); const { getOpenIdConfig, getOpenIdEmail } = require('~/strategies'); const AUTH_REFRESH_USER_PROJECTION = '-password -__v -totpSecret -backupCodes -federatedTokens'; +const OPENID_REUSE_EXPIRY_BUFFER_SECONDS = 30; +/** Mirrors the default SESSION_EXPIRY to bound IdP revocation lag for session-token reuse. */ +const OPENID_REUSE_MAX_SESSION_AGE_MS = 15 * 60 * 1000; const registrationController = async (req, res) => { try { @@ -68,7 +71,19 @@ const getValidOpenIDReuseUserId = (parsedCookies) => { } }; +const isRecentOpenIDSessionRefresh = (openidTokens) => { + const lastRefreshedAt = Number(openidTokens?.lastRefreshedAt); + const elapsed = Date.now() - lastRefreshedAt; + return ( + Number.isFinite(lastRefreshedAt) && elapsed >= 0 && elapsed <= OPENID_REUSE_MAX_SESSION_AGE_MS + ); +}; + const getReusableOpenIDSessionToken = (openidTokens) => { + if (!isRecentOpenIDSessionRefresh(openidTokens)) { + return null; + } + const candidates = [ { token: openidTokens?.idToken, type: 'id_token' }, { token: openidTokens?.accessToken, type: 'access_token' }, @@ -79,8 +94,13 @@ const getReusableOpenIDSessionToken = (openidTokens) => { if (!candidate.token) { continue; } + /** Decode only: tokens are from the trusted server-side session; expiry gates reuse. */ const decoded = jwt.decode(candidate.token); - if (decoded && typeof decoded === 'object' && decoded.exp > now) { + if ( + decoded && + typeof decoded === 'object' && + decoded.exp > now + OPENID_REUSE_EXPIRY_BUFFER_SECONDS + ) { return candidate; } } @@ -134,6 +154,11 @@ const refreshController = async (req, res) => { } try { + /** + * Reuse skips an IdP refresh only for recently-refreshed server-side tokens. + * Stale, missing, or near-expiry tokens fall through to refreshTokenGrant so + * upstream revocations and cookie/session extension are checked regularly. + */ const reusableSessionToken = getReusableOpenIDSessionToken(req.session?.openidTokens); const reuseUserId = reusableSessionToken ? getValidOpenIDReuseUserId(parsedCookies) : null; if (reuseUserId) { @@ -144,7 +169,6 @@ const refreshController = async (req, res) => { token_type: reusableSessionToken.type, has_id_token: Boolean(req.session?.openidTokens?.idToken), has_access_token: Boolean(req.session?.openidTokens?.accessToken), - cloudfront_cookies_attempted: true, cloudfront_cookies_set: cloudFrontCookiesSet, }); return res.status(200).send({ @@ -237,7 +261,7 @@ const refreshController = async (req, res) => { if (process.env.NODE_ENV === 'CI') { const token = await setAuthTokens(userId, res, null, req); - return res.status(200).send({ token, user }); + return res.status(200).send({ token, user: sanitizeUserForAuthResponse(user) }); } /** Session with the hashed refresh token */ @@ -252,7 +276,7 @@ const refreshController = async (req, res) => { if (session && session.expiration > new Date()) { const token = await setAuthTokens(userId, res, session, req); - res.status(200).send({ token, user }); + res.status(200).send({ token, user: sanitizeUserForAuthResponse(user) }); } else if (req?.query?.retry) { // Retrying from a refresh token request that failed (401) res.status(403).send('No session found'); diff --git a/api/server/controllers/AuthController.spec.js b/api/server/controllers/AuthController.spec.js index cbfd483e33..7bed1da33b 100644 --- a/api/server/controllers/AuthController.spec.js +++ b/api/server/controllers/AuthController.spec.js @@ -43,13 +43,18 @@ const { logger } = require('@librechat/data-schemas'); const { isEnabled, findOpenIDUser, buildOpenIDRefreshParams } = require('@librechat/api'); const { graphTokenController, refreshController } = require('./AuthController'); const { getGraphApiToken } = require('~/server/services/GraphTokenService'); -const { setOpenIDAuthTokens, setCloudFrontAuthCookies } = require('~/server/services/AuthService'); +const { + setOpenIDAuthTokens, + setCloudFrontAuthCookies, + setAuthTokens, +} = require('~/server/services/AuthService'); const { getOpenIdConfig, getOpenIdEmail } = require('~/strategies'); -const { getUserById, updateUser } = require('~/models'); +const { getUserById, findSession, updateUser } = require('~/models'); const ORIGINAL_OPENID_SCOPE = process.env.OPENID_SCOPE; const ORIGINAL_OPENID_REFRESH_AUDIENCE = process.env.OPENID_REFRESH_AUDIENCE; const ORIGINAL_JWT_REFRESH_SECRET = process.env.JWT_REFRESH_SECRET; +const ORIGINAL_NODE_ENV = process.env.NODE_ENV; describe('graphTokenController', () => { let req, res; @@ -194,6 +199,28 @@ describe('refreshController – OpenID path', () => { }; let req, res; + const idpSigningSecret = 'idp-signing-secret'; + + const makeSessionToken = (claims = {}) => + jwt.sign( + { + sub: baseClaims.sub, + exp: Math.floor(Date.now() / 1000) + 3600, + ...claims, + }, + idpSigningSecret, + ); + + const makeSignedUserId = (id = 'user-db-id', options = { expiresIn: '1h' }) => + jwt.sign({ id }, process.env.JWT_REFRESH_SECRET, options); + + const setOpenIDReuseCookies = (signedUserId = makeSignedUserId()) => { + req.headers.cookie = [ + 'token_provider=openid', + 'refreshToken=stored-refresh', + `openid_user_id=${signedUserId}`, + ].join('; '); + }; beforeEach(() => { jest.clearAllMocks(); @@ -248,6 +275,20 @@ describe('refreshController – OpenID path', () => { } }); + /** Asserts the full OpenID refresh grant was triggered using default mock state. */ + const expectOpenIDRefreshGrant = () => { + expect(openIdClient.refreshTokenGrant).toHaveBeenCalledWith( + { some: 'config' }, + 'stored-refresh', + {}, + ); + expect(setOpenIDAuthTokens).toHaveBeenCalledWith(mockTokenset, req, res, { + userId: 'user-db-id', + existingRefreshToken: 'stored-refresh', + tenantId: undefined, + }); + }; + it('should call getOpenIdEmail with token claims and use result for findOpenIDUser', async () => { await refreshController(req, res); @@ -263,23 +304,15 @@ describe('refreshController – OpenID path', () => { }); it('reuses valid OpenID session tokens and refreshes CloudFront cookies', async () => { - const reusableIdToken = jwt.sign( - { sub: baseClaims.sub, exp: Math.floor(Date.now() / 1000) + 3600 }, - 'idp-signing-secret', - ); - const signedUserId = jwt.sign({ id: 'user-db-id' }, process.env.JWT_REFRESH_SECRET, { - expiresIn: '1h', - }); - req.headers.cookie = [ - 'token_provider=openid', - 'refreshToken=stored-refresh', - `openid_user_id=${signedUserId}`, - ].join('; '); + const reusableIdToken = makeSessionToken(); + const signedUserId = makeSignedUserId(); + setOpenIDReuseCookies(signedUserId); req.session = { openidTokens: { accessToken: 'session-access-token', idToken: reusableIdToken, refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now(), }, }; const user = { @@ -316,7 +349,6 @@ describe('refreshController – OpenID path', () => { '[refreshController] OpenID session token reused', expect.objectContaining({ token_type: 'id_token', - cloudfront_cookies_attempted: true, cloudfront_cookies_set: true, }), ); @@ -326,6 +358,184 @@ describe('refreshController – OpenID path', () => { expect(debugOutput).not.toContain('session-access-token'); }); + it('falls through to full OpenID refresh when session tokens are expired', async () => { + const expiredToken = makeSessionToken({ exp: Math.floor(Date.now() / 1000) - 60 }); + setOpenIDReuseCookies(); + req.session = { + openidTokens: { + accessToken: expiredToken, + idToken: expiredToken, + refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now(), + }, + }; + + await refreshController(req, res); + + expect(getUserById).not.toHaveBeenCalled(); + expect(setCloudFrontAuthCookies).not.toHaveBeenCalled(); + expectOpenIDRefreshGrant(); + }); + + it('falls through to full OpenID refresh when session tokens are near expiry', async () => { + const nearExpiryToken = makeSessionToken({ exp: Math.floor(Date.now() / 1000) + 5 }); + setOpenIDReuseCookies(); + req.session = { + openidTokens: { + accessToken: nearExpiryToken, + idToken: nearExpiryToken, + refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now(), + }, + }; + + await refreshController(req, res); + + expect(getUserById).not.toHaveBeenCalled(); + expectOpenIDRefreshGrant(); + }); + + it('falls through to full OpenID refresh when session tokens have no exp claim', async () => { + const tokenWithoutExp = jwt.sign({ sub: baseClaims.sub }, idpSigningSecret); + setOpenIDReuseCookies(); + req.session = { + openidTokens: { + accessToken: tokenWithoutExp, + idToken: tokenWithoutExp, + refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now(), + }, + }; + + await refreshController(req, res); + + expect(getUserById).not.toHaveBeenCalled(); + expectOpenIDRefreshGrant(); + }); + + it('falls through to full OpenID refresh when the signed reuse user cookie is invalid', async () => { + setOpenIDReuseCookies('tampered-cookie'); + req.session = { + openidTokens: { + accessToken: 'session-access-token', + idToken: makeSessionToken(), + refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now(), + }, + }; + + await refreshController(req, res); + + expect(getUserById).not.toHaveBeenCalled(); + expectOpenIDRefreshGrant(); + }); + + it('falls through to full OpenID refresh when the reuse user no longer exists', async () => { + setOpenIDReuseCookies(); + req.session = { + openidTokens: { + accessToken: 'session-access-token', + idToken: makeSessionToken(), + refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now(), + }, + }; + getUserById.mockResolvedValueOnce(null); + + await refreshController(req, res); + + expect(getUserById).toHaveBeenCalledWith( + 'user-db-id', + '-password -__v -totpSecret -backupCodes -federatedTokens', + ); + expect(setCloudFrontAuthCookies).not.toHaveBeenCalled(); + expectOpenIDRefreshGrant(); + }); + + it('falls through to full OpenID refresh when session tokens are stale', async () => { + setOpenIDReuseCookies(); + req.session = { + openidTokens: { + accessToken: 'session-access-token', + idToken: makeSessionToken(), + refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now() - 16 * 60 * 1000, + }, + }; + + await refreshController(req, res); + + expect(getUserById).not.toHaveBeenCalled(); + expectOpenIDRefreshGrant(); + }); + + it('falls through to full OpenID refresh when session refresh timestamp is in the future', async () => { + setOpenIDReuseCookies(); + req.session = { + openidTokens: { + accessToken: 'session-access-token', + idToken: makeSessionToken(), + refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now() + 60 * 1000, + }, + }; + + await refreshController(req, res); + + expect(getUserById).not.toHaveBeenCalled(); + expectOpenIDRefreshGrant(); + }); + + it('falls through to full OpenID refresh for pre-upgrade sessions without lastRefreshedAt', async () => { + setOpenIDReuseCookies(); + req.session = { + openidTokens: { + accessToken: 'session-access-token', + idToken: makeSessionToken(), + refreshToken: 'stored-refresh', + }, + }; + + await refreshController(req, res); + + expect(getUserById).not.toHaveBeenCalled(); + expectOpenIDRefreshGrant(); + }); + + it('sanitizes Mongoose-style user documents on the OpenID reuse path', async () => { + const reusableIdToken = makeSessionToken(); + setOpenIDReuseCookies(); + req.session = { + openidTokens: { + accessToken: 'session-access-token', + idToken: reusableIdToken, + refreshToken: 'stored-refresh', + lastRefreshedAt: Date.now(), + }, + }; + const userDocument = { + toObject: () => ({ + ...defaultUser, + federatedTokens: { access_token: 'do-not-return' }, + }), + }; + getUserById.mockResolvedValue(userDocument); + + await refreshController(req, res); + + const sentPayload = res.send.mock.calls[0][0]; + expect(setCloudFrontAuthCookies).toHaveBeenCalledWith(req, res, userDocument); + expect(sentPayload).toEqual({ + token: reusableIdToken, + user: expect.objectContaining({ + _id: 'user-db-id', + email: baseClaims.email, + }), + }); + expect(sentPayload.user).not.toHaveProperty('password'); + expect(sentPayload.user).not.toHaveProperty('federatedTokens'); + }); + it('should pass scope-only OpenID refresh params when OPENID_SCOPE is set', async () => { process.env.OPENID_SCOPE = 'openid profile email'; @@ -529,3 +739,103 @@ describe('refreshController – OpenID path', () => { expect(res.send).toHaveBeenCalledWith('Refresh token not provided'); }); }); + +describe('refreshController – LibreChat path', () => { + let req, res; + const refreshSecret = 'test-refresh-secret'; + + beforeEach(() => { + jest.clearAllMocks(); + process.env.JWT_REFRESH_SECRET = refreshSecret; + process.env.NODE_ENV = 'test'; + setAuthTokens.mockResolvedValue('local-app-token'); + findSession.mockResolvedValue({ expiration: new Date(Date.now() + 60_000) }); + + const refreshToken = jwt.sign({ id: 'local-user-id' }, refreshSecret, { + expiresIn: '1h', + }); + req = { + headers: { cookie: `refreshToken=${refreshToken}` }, + query: {}, + session: {}, + }; + res = { + status: jest.fn().mockReturnThis(), + send: jest.fn().mockReturnThis(), + redirect: jest.fn(), + }; + }); + + afterAll(() => { + if (ORIGINAL_JWT_REFRESH_SECRET === undefined) { + delete process.env.JWT_REFRESH_SECRET; + } else { + process.env.JWT_REFRESH_SECRET = ORIGINAL_JWT_REFRESH_SECRET; + } + + if (ORIGINAL_NODE_ENV === undefined) { + delete process.env.NODE_ENV; + } else { + process.env.NODE_ENV = ORIGINAL_NODE_ENV; + } + }); + + it('sanitizes user documents before returning local refresh responses', async () => { + getUserById.mockResolvedValue({ + toObject: () => ({ + _id: 'local-user-id', + email: 'local@example.com', + password: 'hashed-password', + __v: 1, + totpSecret: 'totp-secret', + backupCodes: ['backup-code'], + federatedTokens: { access_token: 'do-not-return' }, + }), + }); + + await refreshController(req, res); + + const sentPayload = res.send.mock.calls[0][0]; + expect(setAuthTokens).toHaveBeenCalledWith( + 'local-user-id', + res, + { expiration: expect.any(Date) }, + req, + ); + expect(sentPayload).toEqual({ + token: 'local-app-token', + user: { + _id: 'local-user-id', + email: 'local@example.com', + }, + }); + }); + + it('sanitizes user documents before returning CI refresh responses', async () => { + process.env.NODE_ENV = 'CI'; + getUserById.mockResolvedValue({ + toObject: () => ({ + _id: 'local-user-id', + email: 'local@example.com', + password: 'hashed-password', + __v: 1, + totpSecret: 'totp-secret', + backupCodes: ['backup-code'], + federatedTokens: { access_token: 'do-not-return' }, + }), + }); + + await refreshController(req, res); + + const sentPayload = res.send.mock.calls[0][0]; + expect(findSession).not.toHaveBeenCalled(); + expect(setAuthTokens).toHaveBeenCalledWith('local-user-id', res, null, req); + expect(sentPayload).toEqual({ + token: 'local-app-token', + user: { + _id: 'local-user-id', + email: 'local@example.com', + }, + }); + }); +}); diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index 7197002873..943c5a81f6 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -45,6 +45,23 @@ const domains = { }; const genericVerificationMessage = 'Please check your email to verify your email address.'; +const OPENID_SESSION_ID_TOKEN_EXPIRY_BUFFER_SECONDS = 30; + +const getUnexpiredOpenIDSessionIdToken = (idToken) => { + if (!idToken) { + return; + } + + const decoded = jwt.decode(idToken); + const now = Math.floor(Date.now() / 1000); + if ( + decoded && + typeof decoded === 'object' && + decoded.exp > now + OPENID_SESSION_ID_TOKEN_EXPIRY_BUFFER_SECONDS + ) { + return idToken; + } +}; /** * Logout user @@ -412,12 +429,7 @@ const resetPassword = async (userId, token, password) => { const getPreviousCloudFrontScope = (req) => parseCloudFrontCookieScope(req?.cookies?.[CLOUDFRONT_SCOPE_COOKIE]); -const normalizeCloudFrontScopeValue = (value) => { - if (value == null) { - return value; - } - return value.toString?.() ?? value; -}; +const normalizeCloudFrontScopeValue = (value) => (value == null ? undefined : String(value)); const getCloudFrontScopeValue = (optionsValue, userValue, requestValue) => normalizeCloudFrontScopeValue(optionsValue ?? userValue ?? requestValue); @@ -533,7 +545,7 @@ const setAuthTokens = async (userId, res, _session = null, req = null) => { sameSite: 'strict', }); - setCloudFrontAuthCookies(req, res, user, { userId }); + setCloudFrontAuthCookies(req, res, user, { userId: user?._id ?? userId }); return token; } catch (error) { @@ -616,8 +628,11 @@ const setOpenIDAuthTokens = ( * 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; const sessionIdToken = req.session?.openidTokens?.idToken; + const appAuthToken = + tokenset.id_token || + getUnexpiredOpenIDSessionIdToken(sessionIdToken) || + tokenset.access_token; const logoutIdToken = tokenset.id_token || sessionIdToken; /** @@ -643,6 +658,7 @@ const setOpenIDAuthTokens = ( idToken: logoutIdToken, refreshToken: refreshToken, expiresAt: expirationDate.getTime(), + lastRefreshedAt: Date.now(), }; } else { logger.warn('[setOpenIDAuthTokens] No session available, falling back to cookies'); diff --git a/api/server/services/AuthService.spec.js b/api/server/services/AuthService.spec.js index ab2be3a57d..c81b78e8e1 100644 --- a/api/server/services/AuthService.spec.js +++ b/api/server/services/AuthService.spec.js @@ -54,6 +54,7 @@ const { getCloudFrontConfig, parseCloudFrontCookieScope, } = require('@librechat/api'); +const jwt = require('jsonwebtoken'); const { logger } = require('@librechat/data-schemas'); const { findUser, @@ -188,9 +189,14 @@ describe('setOpenIDAuthTokens', () => { expect(req.session.openidTokens.accessToken).toBe('the-access-token'); expect(req.session.openidTokens.idToken).toBe('the-id-token'); expect(req.session.openidTokens.refreshToken).toBe('the-refresh-token'); + expect(req.session.openidTokens.lastRefreshedAt).toEqual(expect.any(Number)); }); - it('should preserve the existing session id_token when refresh omits one', () => { + it('should return the existing unexpired session id_token when refresh omits one', () => { + const existingIdToken = jwt.sign( + { sub: 'user-123', exp: Math.floor(Date.now() / 1000) + 3600 }, + 'idp-signing-secret', + ); const tokenset = { access_token: 'new-access-token', refresh_token: 'new-refresh-token', @@ -198,7 +204,34 @@ describe('setOpenIDAuthTokens', () => { const req = mockRequest({ openidTokens: { accessToken: 'old-access-token', - idToken: 'existing-id-token', + idToken: existingIdToken, + refreshToken: 'old-refresh-token', + }, + }); + const res = mockResponse(); + + const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123'); + + expect(result).toBe(existingIdToken); + expect(req.session.openidTokens.accessToken).toBe('new-access-token'); + expect(req.session.openidTokens.idToken).toBe(existingIdToken); + expect(req.session.openidTokens.refreshToken).toBe('new-refresh-token'); + expect(req.session.openidTokens.lastRefreshedAt).toEqual(expect.any(Number)); + }); + + it('should fall back to access_token when the existing session id_token is expired', () => { + const expiredIdToken = jwt.sign( + { sub: 'user-123', exp: Math.floor(Date.now() / 1000) - 60 }, + 'idp-signing-secret', + ); + const tokenset = { + access_token: 'new-access-token', + refresh_token: 'new-refresh-token', + }; + const req = mockRequest({ + openidTokens: { + accessToken: 'old-access-token', + idToken: expiredIdToken, refreshToken: 'old-refresh-token', }, }); @@ -207,9 +240,33 @@ describe('setOpenIDAuthTokens', () => { const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123'); expect(result).toBe('new-access-token'); + expect(req.session.openidTokens.idToken).toBe(expiredIdToken); + expect(req.session.openidTokens.accessToken).toBe('new-access-token'); + }); + + it('should fall back to access_token when the existing session id_token is near expiry', () => { + const nearExpiryIdToken = jwt.sign( + { sub: 'user-123', exp: Math.floor(Date.now() / 1000) + 10 }, + 'idp-signing-secret', + ); + const tokenset = { + access_token: 'new-access-token', + refresh_token: 'new-refresh-token', + }; + const req = mockRequest({ + openidTokens: { + accessToken: 'old-access-token', + idToken: nearExpiryIdToken, + refreshToken: 'old-refresh-token', + }, + }); + const res = mockResponse(); + + const result = setOpenIDAuthTokens(tokenset, req, res, 'user-123'); + + expect(result).toBe('new-access-token'); + expect(req.session.openidTokens.idToken).toBe(nearExpiryIdToken); expect(req.session.openidTokens.accessToken).toBe('new-access-token'); - expect(req.session.openidTokens.idToken).toBe('existing-id-token'); - expect(req.session.openidTokens.refreshToken).toBe('new-refresh-token'); }); }); @@ -715,6 +772,25 @@ describe('CloudFront cookie integration', () => { ); }); + it('uses the fetched user id as the canonical CloudFront user scope', async () => { + getUserById.mockResolvedValueOnce({ + _id: { toString: () => 'canonical-user' }, + tenantId: 'tenantA', + }); + const res = mockResponse(); + + await setAuthTokens('input-user-id', res); + + expect(setCloudFrontCookies).toHaveBeenCalledWith( + res, + { + userId: 'canonical-user', + tenantId: 'tenantA', + }, + null, + ); + }); + it('passes the previous CloudFront cookie scope when present', async () => { parseCloudFrontCookieScope.mockReturnValue({ userId: 'old-user', tenantId: 'old-tenant' }); const res = mockResponse();