From 417405a97402e54c0c87a8384071a3b46f1f5715 Mon Sep 17 00:00:00 2001 From: WhammyLeaf Date: Thu, 12 Feb 2026 04:11:05 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=8F=A2=20fix:=20Handle=20Group=20Overage?= =?UTF-8?q?=20for=20Azure=20Entra=20Authentication=20(#11557)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit small fix add tests reorder Update api/strategies/openidStrategy.spec.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Update api/strategies/openidStrategy.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> some fixes and fix fix more fixes fix --- api/strategies/openidStrategy.js | 94 +++++++- api/strategies/openidStrategy.spec.js | 310 ++++++++++++++++++++++++++ 2 files changed, 403 insertions(+), 1 deletion(-) diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index 84458ce992..c937b3dc9e 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -287,6 +287,77 @@ function convertToUsername(input, defaultValue = '') { return defaultValue; } +/** + * Resolve Azure AD groups when group overage is in effect (groups moved to _claim_names/_claim_sources). + * + * NOTE: Microsoft recommends treating _claim_names/_claim_sources as a signal only and using Microsoft Graph + * to resolve group membership instead of calling the endpoint in _claim_sources directly. + * + * @param {string} accessToken - Access token with Microsoft Graph permissions + * @returns {Promise} Resolved group IDs or null on failure + * @see https://learn.microsoft.com/en-us/entra/identity-platform/access-token-claims-reference#groups-overage-claim + * @see https://learn.microsoft.com/en-us/graph/api/directoryobject-getmemberobjects + */ +async function resolveGroupsFromOverage(accessToken) { + try { + if (!accessToken) { + logger.error('[openidStrategy] Access token missing; cannot resolve group overage'); + return null; + } + + // Use /me/getMemberObjects so least-privileged delegated permission User.Read is sufficient + // when resolving the signed-in user's group membership. + const url = 'https://graph.microsoft.com/v1.0/me/getMemberObjects'; + + logger.debug( + `[openidStrategy] Detected group overage, resolving groups via Microsoft Graph getMemberObjects: ${url}`, + ); + + const fetchOptions = { + method: 'POST', + headers: { + Authorization: `Bearer ${accessToken}`, + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ securityEnabledOnly: false }), + }; + + if (process.env.PROXY) { + const { ProxyAgent } = undici; + fetchOptions.dispatcher = new ProxyAgent(process.env.PROXY); + } + + const response = await undici.fetch(url, fetchOptions); + if (!response.ok) { + logger.error( + `[openidStrategy] Failed to resolve groups via Microsoft Graph getMemberObjects: HTTP ${response.status} ${response.statusText}`, + ); + return null; + } + + const data = await response.json(); + const values = Array.isArray(data?.value) ? data.value : null; + if (!values) { + logger.error( + '[openidStrategy] Unexpected response format when resolving groups via Microsoft Graph getMemberObjects', + ); + return null; + } + const groupIds = values.filter((id) => typeof id === 'string'); + + logger.debug( + `[openidStrategy] Successfully resolved ${groupIds.length} groups via Microsoft Graph getMemberObjects`, + ); + return groupIds; + } catch (err) { + logger.error( + '[openidStrategy] Error resolving groups via Microsoft Graph getMemberObjects:', + err, + ); + return null; + } +} + /** * Process OpenID authentication tokenset and userinfo * This is the core logic extracted from the passport strategy callback @@ -350,6 +421,25 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { } let roles = get(decodedToken, requiredRoleParameterPath); + + // Handle Azure AD group overage for ID token groups: when hasgroups or _claim_* indicate overage, + // resolve groups via Microsoft Graph instead of relying on token group values. + if ( + !Array.isArray(roles) && + typeof roles !== 'string' && + requiredRoleTokenKind === 'id' && + requiredRoleParameterPath === 'groups' && + decodedToken && + (decodedToken.hasgroups || + (decodedToken._claim_names?.groups && + decodedToken._claim_sources?.[decodedToken._claim_names.groups])) + ) { + const overageGroups = await resolveGroupsFromOverage(tokenset.access_token); + if (overageGroups) { + roles = overageGroups; + } + } + if (!roles || (!Array.isArray(roles) && typeof roles !== 'string')) { logger.error( `[openidStrategy] Key '${requiredRoleParameterPath}' not found in ${requiredRoleTokenKind} token!`, @@ -361,7 +451,9 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { throw new Error(`You must have ${rolesList} role to log in.`); } - if (!requiredRoles.some((role) => roles.includes(role))) { + const roleValues = Array.isArray(roles) ? roles : [roles]; + + if (!requiredRoles.some((role) => roleValues.includes(role))) { const rolesList = requiredRoles.length === 1 ? `"${requiredRoles[0]}"` diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index ada27cca17..99b9483522 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -1,5 +1,6 @@ const fetch = require('node-fetch'); const jwtDecode = require('jsonwebtoken/decode'); +const undici = require('undici'); const { ErrorTypes } = require('librechat-data-provider'); const { findUser, createUser, updateUser } = require('~/models'); const { setupOpenId } = require('./openidStrategy'); @@ -7,6 +8,10 @@ const { setupOpenId } = require('./openidStrategy'); // --- Mocks --- jest.mock('node-fetch'); jest.mock('jsonwebtoken/decode'); +jest.mock('undici', () => ({ + fetch: jest.fn(), + ProxyAgent: jest.fn(), +})); jest.mock('~/server/services/Files/strategies', () => ({ getStrategyFunctions: jest.fn(() => ({ saveBuffer: jest.fn().mockResolvedValue('/fake/path/to/avatar.png'), @@ -360,6 +365,25 @@ describe('setupOpenId', () => { expect(details.message).toBe('You must have "requiredRole" role to log in.'); }); + it('should not treat substring matches in string roles as satisfying required role', async () => { + // Arrange – override required role to "read" then re-setup + process.env.OPENID_REQUIRED_ROLE = 'read'; + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + // Token contains "bread" which *contains* "read" as a substring + jwtDecode.mockReturnValue({ + roles: 'bread', + }); + + // Act + const { user, details } = await validate(tokenset); + + // Assert – verify that substring match does not grant access + expect(user).toBe(false); + expect(details.message).toBe('You must have "read" role to log in.'); + }); + it('should allow login when single required role is present (backward compatibility)', async () => { // Arrange – ensure single role configuration (as set in beforeEach) // OPENID_REQUIRED_ROLE = 'requiredRole' @@ -378,6 +402,292 @@ describe('setupOpenId', () => { expect(createUser).toHaveBeenCalled(); }); + describe('group overage and groups handling', () => { + it.each([ + ['groups array contains required group', ['group-required', 'other-group'], true, undefined], + [ + 'groups array missing required group', + ['other-group'], + false, + 'You must have "group-required" role to log in.', + ], + ['groups string equals required group', 'group-required', true, undefined], + [ + 'groups string is other group', + 'other-group', + false, + 'You must have "group-required" role to log in.', + ], + ])( + 'uses groups claim directly when %s (no overage)', + async (_label, groupsClaim, expectedAllowed, expectedMessage) => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + + jwtDecode.mockReturnValue({ + groups: groupsClaim, + permissions: ['admin'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + const { user, details } = await validate(tokenset); + + expect(undici.fetch).not.toHaveBeenCalled(); + expect(Boolean(user)).toBe(expectedAllowed); + expect(details?.message).toBe(expectedMessage); + }, + ); + + it.each([ + ['token kind is not id', { kind: 'access', path: 'groups', decoded: { hasgroups: true } }], + ['parameter path is not groups', { kind: 'id', path: 'roles', decoded: { hasgroups: true } }], + ['decoded token is falsy', { kind: 'id', path: 'groups', decoded: null }], + [ + 'no overage indicators in decoded token', + { + kind: 'id', + path: 'groups', + decoded: { + permissions: ['admin'], + }, + }, + ], + [ + 'only _claim_names present (no _claim_sources)', + { + kind: 'id', + path: 'groups', + decoded: { + _claim_names: { groups: 'src1' }, + permissions: ['admin'], + }, + }, + ], + [ + 'only _claim_sources present (no _claim_names)', + { + kind: 'id', + path: 'groups', + decoded: { + _claim_sources: { src1: { endpoint: 'https://graph.windows.net/ignored' } }, + permissions: ['admin'], + }, + }, + ], + ])('does not attempt overage resolution when %s', async (_label, cfg) => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = cfg.path; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = cfg.kind; + + jwtDecode.mockReturnValue(cfg.decoded); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + const { user, details } = await validate(tokenset); + + expect(undici.fetch).not.toHaveBeenCalled(); + expect(user).toBe(false); + expect(details.message).toBe('You must have "group-required" role to log in.'); + const { logger } = require('@librechat/data-schemas'); + const expectedTokenKind = cfg.kind === 'access' ? 'access token' : 'id token'; + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining(`Key '${cfg.path}' not found in ${expectedTokenKind}!`), + ); + }); + }); + + describe('resolving groups via Microsoft Graph', () => { + it('denies login and does not call Graph when access token is missing', async () => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + + const { logger } = require('@librechat/data-schemas'); + + jwtDecode.mockReturnValue({ + hasgroups: true, + permissions: ['admin'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + const tokensetWithoutAccess = { + ...tokenset, + access_token: undefined, + }; + + const { user, details } = await validate(tokensetWithoutAccess); + + expect(user).toBe(false); + expect(details.message).toBe('You must have "group-required" role to log in.'); + + expect(undici.fetch).not.toHaveBeenCalled(); + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('Access token missing; cannot resolve group overage'), + ); + }); + + it.each([ + [ + 'Graph returns HTTP error', + async () => ({ + ok: false, + status: 403, + statusText: 'Forbidden', + json: async () => ({}), + }), + [ + '[openidStrategy] Failed to resolve groups via Microsoft Graph getMemberObjects: HTTP 403 Forbidden', + ], + ], + [ + 'Graph network error', + async () => { + throw new Error('network error'); + }, + [ + '[openidStrategy] Error resolving groups via Microsoft Graph getMemberObjects:', + expect.any(Error), + ], + ], + [ + 'Graph returns unexpected shape (no value)', + async () => ({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({}), + }), + [ + '[openidStrategy] Unexpected response format when resolving groups via Microsoft Graph getMemberObjects', + ], + ], + [ + 'Graph returns invalid value type', + async () => ({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: 'not-an-array' }), + }), + [ + '[openidStrategy] Unexpected response format when resolving groups via Microsoft Graph getMemberObjects', + ], + ], + ])( + 'denies login when overage resolution fails because %s', + async (_label, setupFetch, expectedErrorArgs) => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + + const { logger } = require('@librechat/data-schemas'); + + jwtDecode.mockReturnValue({ + hasgroups: true, + permissions: ['admin'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockImplementation(setupFetch); + + const { user, details } = await validate(tokenset); + + expect(undici.fetch).toHaveBeenCalled(); + expect(user).toBe(false); + expect(details.message).toBe('You must have "group-required" role to log in.'); + + expect(logger.error).toHaveBeenCalledWith(...expectedErrorArgs); + }, + ); + + it.each([ + [ + 'hasgroups overage and Graph contains required group', + { + hasgroups: true, + }, + ['group-required', 'some-other-group'], + true, + ], + [ + '_claim_* overage and Graph contains required group', + { + _claim_names: { groups: 'src1' }, + _claim_sources: { src1: { endpoint: 'https://graph.windows.net/ignored' } }, + }, + ['group-required', 'some-other-group'], + true, + ], + [ + 'hasgroups overage and Graph does NOT contain required group', + { + hasgroups: true, + }, + ['some-other-group'], + false, + ], + [ + '_claim_* overage and Graph does NOT contain required group', + { + _claim_names: { groups: 'src1' }, + _claim_sources: { src1: { endpoint: 'https://graph.windows.net/ignored' } }, + }, + ['some-other-group'], + false, + ], + ])( + 'resolves groups via Microsoft Graph when %s', + async (_label, decodedTokenValue, graphGroups, expectedAllowed) => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + + const { logger } = require('@librechat/data-schemas'); + + jwtDecode.mockReturnValue(decodedTokenValue); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ + value: graphGroups, + }), + }); + + const { user } = await validate(tokenset); + + expect(undici.fetch).toHaveBeenCalledWith( + 'https://graph.microsoft.com/v1.0/me/getMemberObjects', + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ + Authorization: `Bearer ${tokenset.access_token}`, + }), + }), + ); + expect(Boolean(user)).toBe(expectedAllowed); + + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining( + `Successfully resolved ${graphGroups.length} groups via Microsoft Graph getMemberObjects`, + ), + ); + }, + ); + }); + it('should attempt to download and save the avatar if picture is provided', async () => { // Act const { user } = await validate(tokenset);