diff --git a/api/server/routes/admin/auth.js b/api/server/routes/admin/auth.js index ee4af2a2b8..6c56e04938 100644 --- a/api/server/routes/admin/auth.js +++ b/api/server/routes/admin/auth.js @@ -41,46 +41,6 @@ const middleware = require('~/server/middleware'); const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN); -function buildGoogleAdminRefreshDeps(sessionExpiry) { - return { - findUsers, - getUserById, - canAccessAdmin: async (user) => { - try { - return await hasCapability( - { - id: user.id ?? user._id?.toString(), - role: user.role ?? '', - tenantId: user.tenantId, - }, - SystemCapabilities.ACCESS_ADMIN, - ); - } catch (err) { - logger.warn(`[admin/oauth/refresh] capability check failed, denying: ${err?.message}`); - return false; - } - }, - isEmailAllowed: async (user) => { - if (!user?.email) return false; - try { - const appConfig = user.tenantId - ? await resolveAppConfigForUser(getAppConfig, user) - : await getAppConfig({ baseOnly: true }); - return isEmailDomainAllowed(user.email, appConfig?.registration?.allowedDomains); - } catch (err) { - logger.warn( - `[admin/oauth/refresh] domain allowlist check failed, denying: ${err?.message}`, - ); - return false; - } - }, - mintToken: async (user) => ({ - token: await generateToken(user, sessionExpiry), - expiresAt: Date.now() + sessionExpiry, - }), - }; -} - const setBalanceConfig = createSetBalanceConfig({ getAppConfig, findBalanceByUser, @@ -111,6 +71,52 @@ function resolveRequestOrigin(req) { } } +function buildAdminRefreshClosures(sessionExpiry) { + return { + canAccessAdmin: async (user) => { + try { + return await hasCapability( + { + id: user.id ?? user._id?.toString(), + role: user.role ?? '', + tenantId: user.tenantId, + }, + SystemCapabilities.ACCESS_ADMIN, + ); + } catch (err) { + logger.warn(`[admin/oauth/refresh] capability check failed, denying: ${err?.message}`); + return false; + } + }, + mintToken: async (user) => ({ + token: await generateToken(user, sessionExpiry), + expiresAt: Date.now() + sessionExpiry, + }), + }; +} + +function buildGoogleAdminRefreshDeps(sessionExpiry) { + return { + findUsers, + getUserById, + ...buildAdminRefreshClosures(sessionExpiry), + isEmailAllowed: async (user) => { + if (!user?.email) return false; + try { + const appConfig = user.tenantId + ? await resolveAppConfigForUser(getAppConfig, user) + : await getAppConfig({ baseOnly: true }); + return isEmailDomainAllowed(user.email, appConfig?.registration?.allowedDomains); + } catch (err) { + logger.warn( + `[admin/oauth/refresh] domain allowlist check failed, denying: ${err?.message}`, + ); + return false; + } + }, + }; +} + router.post( '/login/local', middleware.logHeaders, @@ -654,27 +660,7 @@ router.post( { findUsers, getUserById, - canAccessAdmin: async (user) => { - try { - return await hasCapability( - { - id: user.id ?? user._id?.toString(), - role: user.role ?? '', - tenantId: user.tenantId, - }, - SystemCapabilities.ACCESS_ADMIN, - ); - } catch (err) { - logger.warn( - `[admin/oauth/refresh] capability check failed, denying: ${err?.message}`, - ); - return false; - } - }, - mintToken: async (user) => ({ - token: await generateToken(user, sessionExpiry), - expiresAt: Date.now() + sessionExpiry, - }), + ...buildAdminRefreshClosures(sessionExpiry), }, { userId: normalizedUserId, diff --git a/api/server/routes/admin/auth.refresh.test.js b/api/server/routes/admin/auth.refresh.test.js index b9a2c7b68c..72eaede651 100644 --- a/api/server/routes/admin/auth.refresh.test.js +++ b/api/server/routes/admin/auth.refresh.test.js @@ -296,7 +296,15 @@ describe('admin auth Google refresh route', () => { expect(response.body).toEqual({ token: 'admin-jwt', refreshToken: 'rotated-refresh', - user: expect.objectContaining({ provider: 'google' }), + user: expect.objectContaining({ + _id: 'user-id', + id: 'user-id', + email: 'admin@example.com', + name: 'Admin', + username: 'admin', + role: 'ADMIN', + provider: 'google', + }), expiresAt: 1234567890, }); expect(applyGoogleAdminRefresh).toHaveBeenCalledWith( @@ -316,6 +324,45 @@ describe('admin auth Google refresh route', () => { ); }); + it('forwards the tenant id from getTenantId() to the helper', async () => { + const { getTenantId } = require('@librechat/data-schemas'); + getTenantId.mockReturnValueOnce('tenant-x'); + + const response = await request(app) + .post('/api/admin/oauth/refresh') + .send({ refresh_token: 'incoming-google-refresh', provider: 'google' }); + + expect(response.status).toBe(200); + expect(applyGoogleAdminRefresh).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ tenantId: 'tenant-x' }), + ); + }); + + it('canAccessAdmin closure calls hasCapability with the normalized user id', async () => { + const { hasCapability } = require('~/server/middleware/roles/capabilities'); + let capturedDeps; + applyGoogleAdminRefresh.mockImplementationOnce(async (deps) => { + capturedDeps = deps; + return { + token: 'jwt', + refreshToken: 'r', + user: { id: 'u', _id: 'u', email: 'e@e.com', name: '', username: '', role: 'ADMIN' }, + expiresAt: 0, + }; + }); + + await request(app) + .post('/api/admin/oauth/refresh') + .send({ refresh_token: 'google-refresh', provider: 'google' }); + + await capturedDeps.canAccessAdmin({ id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' }); + expect(hasCapability).toHaveBeenCalledWith( + { id: 'user-1', role: 'ADMIN', tenantId: 'tenant-a' }, + 'ACCESS_ADMIN', + ); + }); + it('does not require OPENID_REUSE_TOKENS for the google provider', async () => { isEnabled.mockReturnValue(false); diff --git a/api/strategies/socialLogin.js b/api/strategies/socialLogin.js index cf86eebb81..459e69e47d 100644 --- a/api/strategies/socialLogin.js +++ b/api/strategies/socialLogin.js @@ -56,7 +56,7 @@ const socialLogin = } const passResult = (user) => - refreshToken ? cb(null, user, { refreshToken }) : cb(null, user); + refreshToken && provider === 'google' ? cb(null, user, { refreshToken }) : cb(null, user); if (existingUser?.provider === provider) { if ( @@ -74,6 +74,15 @@ const socialLogin = } if (options.existingUsersOnly && id && !existingUser[providerKey]) { await updateUser(existingUser._id, { [providerKey]: id }); + const verified = await findUser({ _id: existingUser._id, [providerKey]: id }); + if (!verified) { + logger.warn( + `[${provider}Login] Admin migrate superseded by concurrent write, denying: ${email}`, + ); + const concurrentError = new Error(ErrorTypes.AUTH_FAILED); + concurrentError.code = ErrorTypes.AUTH_FAILED; + return cb(concurrentError); + } existingUser[providerKey] = id; } await handleExistingUser(existingUser, avatarUrl, appConfig, email); diff --git a/api/strategies/socialLogin.test.js b/api/strategies/socialLogin.test.js index 14f2ae1f20..0ea950cc80 100644 --- a/api/strategies/socialLogin.test.js +++ b/api/strategies/socialLogin.test.js @@ -177,6 +177,37 @@ describe('socialLogin', () => { expect(callback).toHaveBeenCalledWith(null, existingUser); }); + it('does not migrate the provider id on the chat path (only admin path migrates)', async () => { + const { updateUser } = require('~/models'); + const provider = 'google'; + const googleId = 'google-user-chat'; + const email = 'chat@example.com'; + + const existingUser = { + _id: 'chatUser', + email: email, + provider: 'google', + }; + + findUser.mockResolvedValueOnce(null).mockResolvedValueOnce(existingUser); + + const mockProfile = { + id: googleId, + emails: [{ value: email, verified: true }], + photos: [{ value: 'https://example.com/avatar.png' }], + name: { givenName: 'Chat', familyName: 'User' }, + }; + + const loginFn = socialLogin(provider, mockGetProfileDetails); + const callback = jest.fn(); + + await loginFn(null, null, null, mockProfile, callback); + + expect(updateUser).not.toHaveBeenCalled(); + expect(handleExistingUser).toHaveBeenCalled(); + expect(callback).toHaveBeenCalledWith(null, existingUser); + }); + it('migrates the missing provider id when finding by email fallback (admin path)', async () => { const { updateUser } = require('~/models'); const provider = 'google'; @@ -427,6 +458,36 @@ describe('socialLogin', () => { ); }); + it('does not forward the refresh token as authInfo for non-google providers', async () => { + const provider = 'github'; + const githubId = 'gh-user-123'; + const email = 'user@example.com'; + + const existingUser = { + _id: 'ghUser', + email, + provider: 'github', + githubId, + }; + + findUser.mockResolvedValue(existingUser); + + const mockProfile = { + id: githubId, + emails: [{ value: email, verified: true }], + photos: [{ value: 'https://example.com/avatar.png' }], + name: { givenName: 'GitHub', familyName: 'User' }, + }; + + const loginFn = socialLogin(provider, mockGetProfileDetails); + const callback = jest.fn(); + + await loginFn(null, 'github-refresh-token', null, mockProfile, callback); + + expect(callback).toHaveBeenCalledWith(null, existingUser); + expect(callback).not.toHaveBeenCalledWith(null, existingUser, expect.anything()); + }); + it('passes the IdP refresh token through as authInfo when present', async () => { const provider = 'google'; const googleId = 'google-with-refresh'; diff --git a/packages/api/src/auth/exchange.ts b/packages/api/src/auth/exchange.ts index 53db548f68..8953f9bb54 100644 --- a/packages/api/src/auth/exchange.ts +++ b/packages/api/src/auth/exchange.ts @@ -50,6 +50,12 @@ export interface AdminExchangeData { */ export interface AdminExchangeResponse { token: string; + /** + * When Google rotates the refresh token on use, this will differ from the + * token the client originally sent. Clients MUST persist this value; failing + * to do so causes future refresh calls to fail once Google's original grant + * expires or is revoked. + */ refreshToken?: string; user: AdminExchangeUser; expiresAt?: number; diff --git a/packages/api/src/auth/googleRefresh.spec.ts b/packages/api/src/auth/googleRefresh.spec.ts index 2e457ceedd..d767a93026 100644 --- a/packages/api/src/auth/googleRefresh.spec.ts +++ b/packages/api/src/auth/googleRefresh.spec.ts @@ -10,6 +10,7 @@ jest.mock('@librechat/data-schemas', () => ({ ...jest.requireActual('@librechat/data-schemas'), logger: { debug: jest.fn(), + error: jest.fn(), info: jest.fn(), warn: jest.fn(), }, @@ -150,6 +151,21 @@ describe('applyGoogleAdminRefresh', () => { }); }); + it('throws ISSUER_MISMATCH when the id_token aud does not match the configured clientId', async () => { + fetchMock.mockResolvedValueOnce( + makeOkJson({ + access_token: 'new-access', + id_token: makeIdToken({ sub: SUB, aud: 'wrong-client' }), + }), + ); + + await expect(applyGoogleAdminRefresh(deps, baseOptions)).rejects.toMatchObject({ + code: 'ISSUER_MISMATCH', + status: 401, + }); + expect(deps.findUsers).not.toHaveBeenCalled(); + }); + it('falls back to the userinfo endpoint when id_token is absent', async () => { const user = makeUser(); fetchMock @@ -222,6 +238,18 @@ describe('applyGoogleAdminRefresh', () => { ).rejects.toMatchObject({ code: 'TENANT_MISMATCH', status: 401 }); }); + it('throws USER_ID_MISMATCH when multiple users share the same googleId', async () => { + fetchMock.mockResolvedValueOnce( + makeOkJson({ access_token: 'new-access', id_token: makeIdToken() }), + ); + deps.findUsers.mockResolvedValue([makeUser(), makeUser({ email: 'other@example.com' })]); + + await expect(applyGoogleAdminRefresh(deps, baseOptions)).rejects.toMatchObject({ + code: 'USER_ID_MISMATCH', + status: 401, + }); + }); + it('throws USER_NOT_FOUND when no admin user matches the refreshed googleId', async () => { fetchMock.mockResolvedValueOnce( makeOkJson({ access_token: 'new-access', id_token: makeIdToken() }), diff --git a/packages/api/src/auth/googleRefresh.ts b/packages/api/src/auth/googleRefresh.ts index 8dee79b732..4c5b63c7fd 100644 --- a/packages/api/src/auth/googleRefresh.ts +++ b/packages/api/src/auth/googleRefresh.ts @@ -20,6 +20,7 @@ interface GoogleTokenset { interface IdTokenClaims { sub?: string; + aud?: string | string[]; } export interface MintedGoogleAdminToken { @@ -70,7 +71,7 @@ async function resolveSubFromUserinfo(accessToken: string): Promise { +async function resolveGoogleSub(tokenset: GoogleTokenset, clientId?: string): Promise { if (typeof tokenset.access_token !== 'string') { throw new AdminRefreshError( 'IDP_INCOMPLETE', @@ -147,6 +148,17 @@ async function resolveGoogleSub(tokenset: GoogleTokenset): Promise { let sub: string | undefined; if (typeof tokenset.id_token === 'string') { const claims = decodeJwtPayload(tokenset.id_token); + if (clientId && claims?.aud !== undefined) { + const aud = claims.aud; + const audOk = Array.isArray(aud) ? aud.includes(clientId) : aud === clientId; + if (!audOk) { + throw new AdminRefreshError( + 'ISSUER_MISMATCH', + 401, + 'id_token aud does not match configured client', + ); + } + } if (typeof claims?.sub === 'string') { sub = claims.sub; } @@ -193,10 +205,18 @@ async function resolveAdminUser( const filter = ( options.tenantId ? { googleId, tenantId: options.tenantId } : { googleId } ) as FilterQuery; - const [found] = await deps.findUsers(filter, SAFE_USER_PROJECTION, { + const matches = await deps.findUsers(filter, SAFE_USER_PROJECTION, { sort: { updatedAt: -1 }, - limit: 1, + limit: 2, }); + if (matches.length > 1) { + logger.error('[admin/oauth/refresh] ambiguous googleId match', { + googleId, + tenantId: options.tenantId, + }); + throw new AdminRefreshError('USER_ID_MISMATCH', 401, 'Ambiguous identity'); + } + const [found] = matches; if (!found) { throw new AdminRefreshError('USER_NOT_FOUND', 401, 'No user found for the refreshed identity'); } @@ -233,7 +253,7 @@ export async function applyGoogleAdminRefresh( }; const tokenset = await fetchGoogleTokenset(configured); - const googleId = await resolveGoogleSub(tokenset); + const googleId = await resolveGoogleSub(tokenset, configured.clientId); const user = await resolveAdminUser(googleId, deps, options); if (deps.isEmailAllowed && !(await deps.isEmailAllowed(user))) { @@ -250,6 +270,12 @@ export async function applyGoogleAdminRefresh( const minted = await deps.mintToken(user); + if (tokenset.refresh_token && tokenset.refresh_token !== options.refreshToken) { + logger.info( + '[admin/oauth/refresh] Google rotated the refresh token; client must persist the new value', + ); + } + return { token: minted.token, refreshToken: tokenset.refresh_token ?? options.refreshToken,