diff --git a/api/server/controllers/auth/oauth.js b/api/server/controllers/auth/oauth.js index 80c2ced002..917e9e2bef 100644 --- a/api/server/controllers/auth/oauth.js +++ b/api/server/controllers/auth/oauth.js @@ -47,9 +47,15 @@ function createOAuthHandler(redirectUri = domains.client) { const refreshToken = req.user.tokenset?.refresh_token || req.user.federatedTokens?.refresh_token; - const exchangeCode = await generateAdminExchangeCode(cache, req.user, token, refreshToken); - const callbackUrl = new URL(redirectUri); + const exchangeCode = await generateAdminExchangeCode( + cache, + req.user, + token, + refreshToken, + callbackUrl.origin, + req.pkceChallenge, + ); callbackUrl.searchParams.set('code', exchangeCode); logger.info(`[OAuth] Admin panel redirect with exchange code for user: ${req.user.email}`); return res.redirect(callbackUrl.toString()); diff --git a/api/server/routes/admin/auth.js b/api/server/routes/admin/auth.js index 530764852b..3e614c88ed 100644 --- a/api/server/routes/admin/auth.js +++ b/api/server/routes/admin/auth.js @@ -24,6 +24,28 @@ const setBalanceConfig = createSetBalanceConfig({ const router = express.Router(); +function resolveRequestOrigin(req) { + const originHeader = req.get('origin'); + if (originHeader) { + try { + return new URL(originHeader).origin; + } catch { + return undefined; + } + } + + const refererHeader = req.get('referer'); + if (!refererHeader) { + return undefined; + } + + try { + return new URL(refererHeader).origin; + } catch { + return undefined; + } +} + router.post( '/login/local', middleware.logHeaders, @@ -52,20 +74,67 @@ router.get('/oauth/openid/check', (req, res) => { res.status(200).json({ message: 'OpenID check successful' }); }); -router.get('/oauth/openid', (req, res, next) => { +/** PKCE challenge cache TTL: 5 minutes (enough for user to authenticate with IdP) */ +const PKCE_CHALLENGE_TTL = 5 * 60 * 1000; +/** Regex pattern for valid PKCE challenges: 64 hex characters (SHA-256 hex digest) */ +const PKCE_CHALLENGE_PATTERN = /^[a-f0-9]{64}$/; + +router.get('/oauth/openid', async (req, res, next) => { + const state = randomState(); + const codeChallenge = req.query.code_challenge; + + if (typeof codeChallenge === 'string' && PKCE_CHALLENGE_PATTERN.test(codeChallenge)) { + try { + const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE); + await cache.set(`pkce:${state}`, codeChallenge, PKCE_CHALLENGE_TTL); + } catch (err) { + logger.error('[admin/oauth/openid] Failed to store PKCE challenge:', err); + return res.redirect( + `${getAdminPanelUrl()}/auth/openid/callback?error=pkce_store_failed&error_description=Failed+to+store+PKCE+challenge`, + ); + } + } + return passport.authenticate('openidAdmin', { session: false, - state: randomState(), + state, })(req, res, next); }); router.get( '/oauth/openid/callback', + (req, res, next) => { + req.oauthState = typeof req.query.state === 'string' ? req.query.state : undefined; + next(); + }, passport.authenticate('openidAdmin', { failureRedirect: `${getAdminPanelUrl()}/auth/openid/callback?error=auth_failed&error_description=Authentication+failed`, failureMessage: true, session: false, }), + async (req, res, next) => { + if (!req.oauthState) { + return next(); + } + try { + const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE); + const challenge = await cache.get(`pkce:${req.oauthState}`); + if (challenge) { + req.pkceChallenge = challenge; + await cache.delete(`pkce:${req.oauthState}`); + } else { + logger.warn( + '[admin/oauth/callback] State present but no PKCE challenge found; PKCE will not be enforced for this request', + ); + } + } catch (err) { + logger.error('[admin/oauth/callback] Failed to retrieve PKCE challenge, aborting:', err); + return res.redirect( + `${getAdminPanelUrl()}/auth/openid/callback?error=pkce_retrieval_failed&error_description=Failed+to+retrieve+PKCE+challenge`, + ); + } + next(); + }, requireAdminAccess, setBalanceConfig, middleware.checkDomainAllowed, @@ -73,7 +142,7 @@ router.get( ); /** Regex pattern for valid exchange codes: 64 hex characters */ -const EXCHANGE_CODE_PATTERN = /^[a-f0-9]{64}$/i; +const EXCHANGE_CODE_PATTERN = /^[a-f0-9]{64}$/; /** * Exchange OAuth authorization code for tokens. @@ -81,12 +150,12 @@ const EXCHANGE_CODE_PATTERN = /^[a-f0-9]{64}$/i; * The code is one-time-use and expires in 30 seconds. * * POST /api/admin/oauth/exchange - * Body: { code: string } + * Body: { code: string, code_verifier?: string } * Response: { token: string, refreshToken: string, user: object } */ router.post('/oauth/exchange', middleware.loginLimiter, async (req, res) => { try { - const { code } = req.body; + const { code, code_verifier: codeVerifier } = req.body; if (!code) { logger.warn('[admin/oauth/exchange] Missing authorization code'); @@ -104,8 +173,20 @@ router.post('/oauth/exchange', middleware.loginLimiter, async (req, res) => { }); } + if ( + codeVerifier !== undefined && + (typeof codeVerifier !== 'string' || codeVerifier.length < 1 || codeVerifier.length > 512) + ) { + logger.warn('[admin/oauth/exchange] Invalid code_verifier format'); + return res.status(400).json({ + error: 'Invalid code_verifier', + error_code: 'INVALID_VERIFIER', + }); + } + const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE); - const result = await exchangeAdminCode(cache, code); + const requestOrigin = resolveRequestOrigin(req); + const result = await exchangeAdminCode(cache, code, requestOrigin, codeVerifier); if (!result) { return res.status(401).json({ diff --git a/packages/api/src/auth/exchange.spec.ts b/packages/api/src/auth/exchange.spec.ts new file mode 100644 index 0000000000..05865197a8 --- /dev/null +++ b/packages/api/src/auth/exchange.spec.ts @@ -0,0 +1,217 @@ +import crypto from 'crypto'; +import { Keyv } from 'keyv'; +import type { IUser } from '@librechat/data-schemas'; + +jest.mock( + '@librechat/data-schemas', + () => ({ + logger: { + info: jest.fn(), + warn: jest.fn(), + }, + }), + { virtual: true }, +); + +import { exchangeAdminCode, generateAdminExchangeCode, verifyCodeChallenge } from './exchange'; + +describe('admin OAuth code exchange', () => { + const user = { + _id: 'user123', + email: 'admin@example.com', + name: 'Admin User', + username: 'admin', + role: 'ADMIN', + provider: 'openid', + } as unknown as IUser; + + const createCache = () => { + const cache = new Keyv(); + const deleteSpy = jest.spyOn(cache, 'delete'); + return { cache, deleteSpy }; + }; + + describe('origin binding', () => { + it('exchanges code when request origin matches generated origin', async () => { + const { cache } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + 'https://admin.example.com', + ); + + const result = await exchangeAdminCode(cache, exchangeCode, 'https://admin.example.com'); + + expect(result).not.toBeNull(); + expect(result!.token).toBe('jwt-token'); + expect(result!.refreshToken).toBe('refresh-token'); + expect(result!.user.email).toBe('admin@example.com'); + }); + + it('rejects code exchange when request origin does not match generated origin', async () => { + const { cache, deleteSpy } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + 'https://admin.example.com', + ); + + const result = await exchangeAdminCode(cache, exchangeCode, 'https://evil.example.com'); + + expect(result).toBeNull(); + expect(deleteSpy).toHaveBeenCalledWith(exchangeCode); + await expect(cache.get(exchangeCode)).resolves.toBeUndefined(); + }); + + it('rejects code exchange when origin is stored but request has no origin', async () => { + const { cache } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + 'https://admin.example.com', + ); + + const result = await exchangeAdminCode(cache, exchangeCode, undefined); + + expect(result).toBeNull(); + }); + + it('allows exchange when no origin was stored (backward compat)', async () => { + const { cache } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + ); + + const result = await exchangeAdminCode(cache, exchangeCode, 'https://any-origin.com'); + + expect(result).not.toBeNull(); + expect(result!.token).toBe('jwt-token'); + }); + }); + + describe('one-time use', () => { + it('rejects code that has already been used', async () => { + const { cache } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + 'https://admin.example.com', + ); + + await exchangeAdminCode(cache, exchangeCode, 'https://admin.example.com'); + const secondAttempt = await exchangeAdminCode( + cache, + exchangeCode, + 'https://admin.example.com', + ); + + expect(secondAttempt).toBeNull(); + }); + }); + + describe('PKCE verification', () => { + const codeVerifier = crypto.randomBytes(32).toString('hex'); + const codeChallenge = crypto.createHash('sha256').update(codeVerifier).digest('hex'); + + it('verifyCodeChallenge returns true for matching verifier', () => { + expect(verifyCodeChallenge(codeVerifier, codeChallenge)).toBe(true); + }); + + it('verifyCodeChallenge returns false for wrong verifier', () => { + expect(verifyCodeChallenge('wrong-verifier', codeChallenge)).toBe(false); + }); + + it('verifyCodeChallenge handles hex case insensitively (input gate rejects uppercase)', () => { + const uppercaseChallenge = codeChallenge.toUpperCase(); + // Buffer.from(hex) is case-insensitive, so verification passes at this layer. + // Uppercase challenges are rejected earlier by PKCE_CHALLENGE_PATTERN (no /i flag). + expect(verifyCodeChallenge(codeVerifier, uppercaseChallenge)).toBe(true); + }); + + it('exchanges code when valid code_verifier is provided', async () => { + const { cache } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + 'https://admin.example.com', + codeChallenge, + ); + + const result = await exchangeAdminCode( + cache, + exchangeCode, + 'https://admin.example.com', + codeVerifier, + ); + + expect(result).not.toBeNull(); + expect(result!.token).toBe('jwt-token'); + }); + + it('rejects exchange when code_verifier does not match challenge', async () => { + const { cache } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + 'https://admin.example.com', + codeChallenge, + ); + + const result = await exchangeAdminCode( + cache, + exchangeCode, + 'https://admin.example.com', + 'wrong-verifier', + ); + + expect(result).toBeNull(); + }); + + it('rejects exchange when challenge stored but no verifier provided', async () => { + const { cache } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + 'https://admin.example.com', + codeChallenge, + ); + + const result = await exchangeAdminCode(cache, exchangeCode, 'https://admin.example.com'); + + expect(result).toBeNull(); + }); + + it('allows exchange when no challenge stored and no verifier sent (backward compat)', async () => { + const { cache } = createCache(); + const exchangeCode = await generateAdminExchangeCode( + cache, + user, + 'jwt-token', + 'refresh-token', + 'https://admin.example.com', + ); + + const result = await exchangeAdminCode(cache, exchangeCode, 'https://admin.example.com'); + + expect(result).not.toBeNull(); + expect(result!.token).toBe('jwt-token'); + }); + }); +}); diff --git a/packages/api/src/auth/exchange.ts b/packages/api/src/auth/exchange.ts index c919974523..f304ac6532 100644 --- a/packages/api/src/auth/exchange.ts +++ b/packages/api/src/auth/exchange.ts @@ -37,6 +37,8 @@ export interface AdminExchangeData { user: AdminExchangeUser; token: string; refreshToken?: string; + origin?: string; + codeChallenge?: string; } /** @@ -68,12 +70,30 @@ export function serializeUserForExchange(user: IUser): AdminExchangeUser { }; } +/** + * Verifies a PKCE code_verifier against a stored code_challenge. + * Uses hex-encoded SHA-256 comparison (not RFC 7636 S256 which uses base64url). + * @param verifier - The code_verifier provided during exchange + * @param challenge - The hex-encoded SHA-256 code_challenge stored during code generation + * @returns True if the verifier matches the challenge + */ +export function verifyCodeChallenge(verifier: string, challenge: string): boolean { + const computed = crypto.createHash('sha256').update(verifier).digest(); + const storedBuf = Buffer.from(challenge, 'hex'); + if (computed.length !== storedBuf.length) { + return false; + } + return crypto.timingSafeEqual(computed, storedBuf); +} + /** * Generates an exchange code and stores user data for admin panel OAuth flow. * @param cache - The Keyv cache instance for storing exchange data * @param user - The authenticated user object * @param token - The JWT access token * @param refreshToken - Optional refresh token for OpenID users + * @param origin - The admin panel origin (scheme://host:port) for origin binding + * @param codeChallenge - PKCE code_challenge (hex-encoded SHA-256 of code_verifier) * @returns The generated exchange code */ export async function generateAdminExchangeCode( @@ -81,6 +101,8 @@ export async function generateAdminExchangeCode( user: IUser, token: string, refreshToken?: string, + origin?: string, + codeChallenge?: string, ): Promise { const exchangeCode = crypto.randomBytes(32).toString('hex'); @@ -89,6 +111,8 @@ export async function generateAdminExchangeCode( user: serializeUserForExchange(user), token, refreshToken, + origin, + codeChallenge, }; await cache.set(exchangeCode, data); @@ -103,15 +127,19 @@ export async function generateAdminExchangeCode( * The code is deleted immediately after retrieval (one-time use). * @param cache - The Keyv cache instance for retrieving exchange data * @param code - The authorization code to exchange + * @param requestOrigin - The origin of the requesting client for origin binding + * @param codeVerifier - PKCE code_verifier to verify against the stored code_challenge * @returns The exchange response with token, refreshToken, and user data, or null if invalid/expired */ export async function exchangeAdminCode( cache: Keyv, code: string, + requestOrigin?: string, + codeVerifier?: string, ): Promise { const data = (await cache.get(code)) as AdminExchangeData | undefined; - /** Delete immediately - one-time use */ + /** Delete before validation — ensures one-time use even if subsequent checks throw */ await cache.delete(code); if (!data) { @@ -119,6 +147,18 @@ export async function exchangeAdminCode( return null; } + if (data.origin && data.origin !== requestOrigin) { + logger.warn('[adminExchange] Authorization code origin mismatch'); + return null; + } + + if (data.codeChallenge) { + if (!codeVerifier || !verifyCodeChallenge(codeVerifier, data.codeChallenge)) { + logger.warn('[adminExchange] PKCE code_verifier mismatch or missing'); + return null; + } + } + logger.info(`[adminExchange] Exchanged code for user: ${data.user?.email}`); return {