diff --git a/api/server/controllers/auth/oauth.js b/api/server/controllers/auth/oauth.js index 2f265ba227..4d40a2f04c 100644 --- a/api/server/controllers/auth/oauth.js +++ b/api/server/controllers/auth/oauth.js @@ -49,7 +49,7 @@ function createOAuthHandler(redirectUri = domains.client) { refreshToken = req.user.tokenset?.refresh_token || req.user.federatedTokens?.refresh_token; } - } else { + } else if (req.user.provider === 'google') { refreshToken = req.authInfo?.refreshToken; } const expiresAt = Date.now() + sessionExpiry; diff --git a/api/server/controllers/auth/oauth.spec.js b/api/server/controllers/auth/oauth.spec.js index d579e7524e..d9984a8dd5 100644 --- a/api/server/controllers/auth/oauth.spec.js +++ b/api/server/controllers/auth/oauth.spec.js @@ -191,4 +191,26 @@ describe('createOAuthHandler', () => { expect.any(Number), ); }); + + it('does not forward refresh tokens for admin providers other than google or openid', async () => { + const handler = createOAuthHandler('http://admin.example.com/auth/discord/callback'); + const req = buildReq({ + user: { _id: 'user-9', email: 'd@example.com', provider: 'discord' }, + authInfo: { refreshToken: 'discord-refresh-token' }, + }); + const res = buildRes(); + const next = jest.fn(); + + await handler(req, res, next); + + expect(mockGenerateAdminExchangeCode).toHaveBeenCalledWith( + {}, + req.user, + 'jwt-token', + undefined, + 'http://admin.example.com', + 'pkce-challenge', + expect.any(Number), + ); + }); }); diff --git a/api/server/routes/admin/auth.js b/api/server/routes/admin/auth.js index a8b6775ce6..02da392795 100644 --- a/api/server/routes/admin/auth.js +++ b/api/server/routes/admin/auth.js @@ -20,6 +20,7 @@ const { applyAdminRefresh, AdminRefreshError, buildOpenIDRefreshParams, + serializeUserForExchange, } = require('@librechat/api'); const { loginController } = require('~/server/controllers/auth/LoginController'); const { hasCapability, requireCapability } = require('~/server/middleware/roles/capabilities'); @@ -37,6 +38,7 @@ const { getOpenIdConfig } = require('~/strategies'); const middleware = require('~/server/middleware'); const GOOGLE_TOKEN_ENDPOINT = 'https://oauth2.googleapis.com/token'; +const GOOGLE_USERINFO_ENDPOINT = 'https://openidconnect.googleapis.com/v1/userinfo'; const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN); @@ -51,6 +53,28 @@ function decodeJwtPayload(token) { } } +async function resolveGoogleSubFromUserinfo(accessToken) { + try { + const response = await fetch(GOOGLE_USERINFO_ENDPOINT, { + headers: { Authorization: `Bearer ${accessToken}` }, + }); + if (!response.ok) { + logger.warn('[admin/oauth/refresh] Google userinfo fallback returned non-OK', { + status: response.status, + }); + return undefined; + } + const body = await response.json().catch(() => undefined); + return typeof body?.sub === 'string' ? body.sub : undefined; + } catch (err) { + logger.warn('[admin/oauth/refresh] Google userinfo fallback failed', { + name: err?.name, + message: err?.message, + }); + return undefined; + } +} + async function refreshGoogleAdminSession({ refreshToken, userId, tenantId, sessionExpiry }) { if (!process.env.GOOGLE_CLIENT_ID || !process.env.GOOGLE_CLIENT_SECRET) { throw new AdminRefreshError( @@ -87,25 +111,42 @@ async function refreshGoogleAdminSession({ refreshToken, userId, tenantId, sessi throw new AdminRefreshError('REFRESH_FAILED', 401, 'Refresh failed'); } - const tokenset = await tokenResponse.json(); - if (!tokenset.access_token || !tokenset.id_token) { + let tokenset; + try { + tokenset = await tokenResponse.json(); + } catch (err) { + logger.warn('[admin/oauth/refresh] Google returned non-JSON body', { + name: err?.name, + message: err?.message, + }); + throw new AdminRefreshError('IDP_INCOMPLETE', 502, 'Google returned a non-JSON token response'); + } + if (typeof tokenset?.access_token !== 'string') { throw new AdminRefreshError( 'IDP_INCOMPLETE', 502, - 'Google returned a tokenset missing access_token or id_token', + 'Google returned a tokenset missing access_token', ); } - const claims = decodeJwtPayload(tokenset.id_token); - if (!claims?.sub) { + let googleId; + if (typeof tokenset.id_token === 'string') { + const claims = decodeJwtPayload(tokenset.id_token); + if (typeof claims?.sub === 'string') { + googleId = claims.sub; + } + } + if (!googleId) { + googleId = await resolveGoogleSubFromUserinfo(tokenset.access_token); + } + if (!googleId) { throw new AdminRefreshError( 'CLAIMS_INCOMPLETE', 502, - 'Google id_token has no readable claims or no sub', + 'Could not resolve google sub from refresh response', ); } - const googleId = claims.sub; const SAFE_USER_PROJECTION = '-password -__v -totpSecret -backupCodes'; let user; @@ -162,17 +203,11 @@ async function refreshGoogleAdminSession({ refreshToken, userId, tenantId, sessi const token = await generateToken(user, sessionExpiry); const expiresAt = Date.now() + sessionExpiry; - const responseUser = { - id: user.id ?? user._id?.toString(), - email: user.email, - name: user.name, - role: user.role, - }; return { token, refreshToken: tokenset.refresh_token ?? refreshToken, - user: responseUser, + user: serializeUserForExchange(user), expiresAt, }; } diff --git a/api/server/routes/admin/auth.refresh.test.js b/api/server/routes/admin/auth.refresh.test.js index 292789dcc7..e15225ccaf 100644 --- a/api/server/routes/admin/auth.refresh.test.js +++ b/api/server/routes/admin/auth.refresh.test.js @@ -54,6 +54,20 @@ jest.mock('@librechat/api', () => { } return params; }), + serializeUserForExchange: jest.fn((user) => { + const userId = String(user._id); + return { + _id: userId, + id: userId, + email: user.email, + name: user.name ?? '', + username: user.username ?? '', + role: user.role ?? 'USER', + avatar: user.avatar, + provider: user.provider, + openidId: user.openidId, + }; + }), }; }); @@ -291,11 +305,13 @@ describe('admin auth Google refresh route', () => { findUsers.mockResolvedValue([ { - _id: { toString: () => 'user-id' }, + _id: 'user-id', id: 'user-id', email: 'admin@example.com', name: 'Admin', + username: 'admin', role: 'ADMIN', + provider: 'google', googleId: 'google-admin-id', }, ]); @@ -331,10 +347,13 @@ describe('admin auth Google refresh route', () => { token: 'admin-jwt', refreshToken: 'incoming-google-refresh', user: { + _id: 'user-id', id: 'user-id', email: 'admin@example.com', name: 'Admin', + username: 'admin', role: 'ADMIN', + provider: 'google', }, expiresAt: expect.any(Number), }); @@ -388,12 +407,12 @@ describe('admin auth Google refresh route', () => { expect(response.body.error_code).toBe('REFRESH_FAILED'); }); - it('returns IDP_INCOMPLETE when Google omits access_token or id_token', async () => { + it('returns IDP_INCOMPLETE when Google omits access_token', async () => { global.fetch = jest.fn(() => Promise.resolve({ ok: true, status: 200, - json: () => Promise.resolve({ access_token: 'only-access' }), + json: () => Promise.resolve({ id_token: validIdToken() }), }), ); @@ -405,6 +424,71 @@ describe('admin auth Google refresh route', () => { expect(response.body.error_code).toBe('IDP_INCOMPLETE'); }); + it('returns IDP_INCOMPLETE when Google returns a non-JSON token body', async () => { + global.fetch = jest.fn(() => + Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.reject(new SyntaxError('Unexpected token < in JSON at position 0')), + }), + ); + + const response = await request(app) + .post('/api/admin/oauth/refresh') + .send({ refresh_token: 'incoming-google-refresh', provider: 'google' }); + + expect(response.status).toBe(502); + expect(response.body.error_code).toBe('IDP_INCOMPLETE'); + }); + + it('falls back to Google userinfo when the refresh response omits id_token', async () => { + const tokenCall = jest.fn(() => + Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve({ access_token: 'google-new-access' }), + }), + ); + const userinfoCall = jest.fn(() => + Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve({ sub: 'google-admin-id' }), + }), + ); + global.fetch = jest.fn((url) => + url === 'https://openidconnect.googleapis.com/v1/userinfo' ? userinfoCall() : tokenCall(), + ); + + const response = await request(app) + .post('/api/admin/oauth/refresh') + .send({ refresh_token: 'incoming-google-refresh', provider: 'google' }); + + expect(response.status).toBe(200); + expect(response.body.user._id).toBe('user-id'); + expect(userinfoCall).toHaveBeenCalled(); + }); + + it('returns CLAIMS_INCOMPLETE when id_token is absent and userinfo also fails', async () => { + global.fetch = jest.fn((url) => { + if (url === 'https://openidconnect.googleapis.com/v1/userinfo') { + return Promise.resolve({ ok: false, status: 401, json: () => Promise.resolve({}) }); + } + return Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve({ access_token: 'google-new-access' }), + }); + }); + + const response = await request(app) + .post('/api/admin/oauth/refresh') + .send({ refresh_token: 'incoming-google-refresh', provider: 'google' }); + + expect(response.status).toBe(502); + expect(response.body.error_code).toBe('CLAIMS_INCOMPLETE'); + }); + it('returns CLAIMS_INCOMPLETE when Google id_token has no sub', async () => { const header = Buffer.from(JSON.stringify({ alg: 'RS256' })).toString('base64url'); const payload = Buffer.from(JSON.stringify({})).toString('base64url'); diff --git a/api/strategies/socialLogin.js b/api/strategies/socialLogin.js index 9eac62fb9a..dd8241e72a 100644 --- a/api/strategies/socialLogin.js +++ b/api/strategies/socialLogin.js @@ -3,7 +3,7 @@ const { ErrorTypes } = require('librechat-data-provider'); const { isEnabled, isEmailDomainAllowed, resolveAppConfigForUser } = require('@librechat/api'); const { createSocialUser, handleExistingUser } = require('./process'); const { getAppConfig } = require('~/server/services/Config'); -const { findUser } = require('~/models'); +const { findUser, updateUser } = require('~/models'); const socialLogin = (provider, getProfileDetails, options = {}) => @@ -59,6 +59,18 @@ const socialLogin = refreshToken ? cb(null, user, { refreshToken }) : cb(null, user); if (existingUser?.provider === provider) { + if (id && existingUser[providerKey] && existingUser[providerKey] !== id) { + logger.warn( + `[${provider}Login] Rejected email fallback for ${email}: stored ${providerKey} does not match`, + ); + const error = new Error(ErrorTypes.AUTH_FAILED); + error.code = ErrorTypes.AUTH_FAILED; + return cb(error); + } + if (id && !existingUser[providerKey]) { + await updateUser(existingUser._id, { [providerKey]: id }); + existingUser[providerKey] = id; + } await handleExistingUser(existingUser, avatarUrl, appConfig, email); return passResult(existingUser); } else if (existingUser) { diff --git a/api/strategies/socialLogin.test.js b/api/strategies/socialLogin.test.js index a24f13e511..d5c96c6f3f 100644 --- a/api/strategies/socialLogin.test.js +++ b/api/strategies/socialLogin.test.js @@ -35,6 +35,7 @@ jest.mock('@librechat/api', () => ({ jest.mock('~/models', () => ({ findUser: jest.fn(), + updateUser: jest.fn(), })); jest.mock('~/server/services/Config', () => ({ @@ -138,7 +139,8 @@ describe('socialLogin', () => { expect(callback).toHaveBeenCalledWith(null, existingUser); }); - it('should fallback to finding user by email if not found by provider ID', async () => { + it('migrates the missing provider id when finding by email fallback', async () => { + const { updateUser } = require('~/models'); const provider = 'google'; const googleId = 'google-user-789'; const email = 'user@example.com'; @@ -147,7 +149,6 @@ describe('socialLogin', () => { _id: 'user789', email: email, provider: 'google', - googleId: 'old-google-id', }; findUser.mockResolvedValueOnce(null).mockResolvedValueOnce(existingUser); @@ -172,10 +173,47 @@ describe('socialLogin', () => { `[${provider}Login] User found by email: ${email} but not by ${provider}Id`, ); + expect(updateUser).toHaveBeenCalledWith('user789', { googleId }); + expect(existingUser.googleId).toBe(googleId); expect(handleExistingUser).toHaveBeenCalled(); expect(callback).toHaveBeenCalledWith(null, existingUser); }); + it('rejects the email fallback when the stored provider id differs from the current sub', async () => { + const provider = 'google'; + const googleId = 'google-user-new'; + const email = 'user@example.com'; + + const existingUser = { + _id: 'user789', + email: email, + provider: 'google', + googleId: 'google-user-old', + }; + + findUser.mockResolvedValueOnce(null).mockResolvedValueOnce(existingUser); + + const mockProfile = { + id: googleId, + emails: [{ value: email, verified: true }], + photos: [{ value: 'https://example.com/avatar.png' }], + name: { givenName: 'Bob', familyName: 'Johnson' }, + }; + + const loginFn = socialLogin(provider, mockGetProfileDetails); + const callback = jest.fn(); + + await loginFn(null, null, null, mockProfile, callback); + + expect(logger.warn).toHaveBeenCalledWith( + `[${provider}Login] Rejected email fallback for ${email}: stored ${provider}Id does not match`, + ); + expect(handleExistingUser).not.toHaveBeenCalled(); + expect(callback).toHaveBeenCalledWith( + expect.objectContaining({ code: ErrorTypes.AUTH_FAILED }), + ); + }); + it('should create new user if not found by provider ID or email', async () => { const provider = 'google'; const googleId = 'google-new-user';