From aee1ced81713d06246646709d89dffe56d5f9d5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Airam=20Hern=C3=A1ndez=20Hern=C3=A1ndez?= <100208966+Airamhh@users.noreply.github.com> Date: Sun, 15 Mar 2026 23:09:53 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=99=20fix:=20Resolve=20Azure=20AD=20Gr?= =?UTF-8?q?oup=20Overage=20via=20OBO=20Token=20Exchange=20for=20OpenID=20(?= =?UTF-8?q?#12187)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Azure AD users belong to 200+ groups, group claims are moved out of the ID token (overage). The existing resolveGroupsFromOverage() called Microsoft Graph directly with the app-audience access token, which Graph rejected (401/403). Changes: - Add exchangeTokenForOverage() dedicated OBO exchange with User.Read scope - Update resolveGroupsFromOverage() to exchange token before Graph call - Add overage handling to OPENID_ADMIN_ROLE block (was silently failing) - Share resolved overage groups between required role and admin role checks - Always resolve via Graph when overage detected (even with partial groups) - Remove debug-only bypass that forced Graph resolution - Add tests for OBO exchange, caching, and admin role overage scenarios Co-authored-by: Airam Hernández Hernández --- api/strategies/openidStrategy.js | 104 ++++++++- api/strategies/openidStrategy.spec.js | 313 +++++++++++++++++++++++++- 2 files changed, 406 insertions(+), 11 deletions(-) diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index 0ebdcb04e1..7c43358297 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -315,24 +315,85 @@ function convertToUsername(input, defaultValue = '') { return defaultValue; } +/** + * Exchange the access token for a Graph-scoped token using the On-Behalf-Of (OBO) flow. + * + * The original access token has the app's own audience (api://), which Microsoft Graph + * rejects. This exchange produces a token with audience https://graph.microsoft.com and the + * minimum delegated scope (User.Read) required by /me/getMemberObjects. + * + * Uses a dedicated cache key (`${sub}:overage`) to avoid collisions with other OBO exchanges + * in the codebase (userinfo, Graph principal search). + * + * @param {string} accessToken - The original access token from the OpenID tokenset + * @param {string} sub - The subject identifier for cache keying + * @returns {Promise} A Graph-scoped access token + * @see https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-on-behalf-of-flow + */ +async function exchangeTokenForOverage(accessToken, sub) { + if (!openidConfig) { + throw new Error('[openidStrategy] OpenID config not initialized; cannot exchange OBO token'); + } + + const tokensCache = getLogStores(CacheKeys.OPENID_EXCHANGED_TOKENS); + const cacheKey = `${sub}:overage`; + + const cached = await tokensCache.get(cacheKey); + if (cached?.access_token) { + logger.debug('[openidStrategy] Using cached Graph token for overage resolution'); + return cached.access_token; + } + + const grantResponse = await client.genericGrantRequest( + openidConfig, + 'urn:ietf:params:oauth:grant-type:jwt-bearer', + { + scope: 'https://graph.microsoft.com/User.Read', + assertion: accessToken, + requested_token_use: 'on_behalf_of', + }, + ); + + if (!grantResponse.access_token) { + throw new Error( + '[openidStrategy] OBO exchange succeeded but returned no access_token; cannot call Graph API', + ); + } + + const ttlMs = + Number.isFinite(grantResponse.expires_in) && grantResponse.expires_in > 0 + ? grantResponse.expires_in * 1000 + : 3600 * 1000; + + await tokensCache.set(cacheKey, { access_token: grantResponse.access_token }, ttlMs); + + return grantResponse.access_token; +} + /** * 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 + * Before calling Graph, the access token is exchanged via the OBO flow to obtain a token with the + * correct audience (https://graph.microsoft.com) and User.Read scope. + * + * @param {string} accessToken - Access token from the OpenID tokenset (app audience) + * @param {string} sub - The subject identifier of the user (for OBO exchange and cache keying) * @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) { +async function resolveGroupsFromOverage(accessToken, sub) { try { if (!accessToken) { logger.error('[openidStrategy] Access token missing; cannot resolve group overage'); return null; } + const graphToken = await exchangeTokenForOverage(accessToken, sub); + // 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'; @@ -344,7 +405,7 @@ async function resolveGroupsFromOverage(accessToken) { const fetchOptions = { method: 'POST', headers: { - Authorization: `Bearer ${accessToken}`, + Authorization: `Bearer ${graphToken}`, 'Content-Type': 'application/json', }, body: JSON.stringify({ securityEnabledOnly: false }), @@ -364,6 +425,7 @@ async function resolveGroupsFromOverage(accessToken) { } const data = await response.json(); + const values = Array.isArray(data?.value) ? data.value : null; if (!values) { logger.error( @@ -432,6 +494,8 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { const fullName = getFullName(userinfo); const requiredRole = process.env.OPENID_REQUIRED_ROLE; + let resolvedOverageGroups = null; + if (requiredRole) { const requiredRoles = requiredRole .split(',') @@ -451,19 +515,21 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { // 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. + const hasOverage = + decodedToken?.hasgroups || + (decodedToken?._claim_names?.groups && + decodedToken?._claim_sources?.[decodedToken._claim_names.groups]); + 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])) + hasOverage ) { - const overageGroups = await resolveGroupsFromOverage(tokenset.access_token); + const overageGroups = await resolveGroupsFromOverage(tokenset.access_token, claims.sub); if (overageGroups) { roles = overageGroups; + resolvedOverageGroups = overageGroups; } } @@ -550,7 +616,25 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { throw new Error('Invalid admin role token kind'); } - const adminRoles = get(adminRoleObject, adminRoleParameterPath); + let adminRoles = get(adminRoleObject, adminRoleParameterPath); + + // Handle Azure AD group overage for admin role when using ID token groups + if (adminRoleTokenKind === 'id' && adminRoleParameterPath === 'groups' && adminRoleObject) { + const hasAdminOverage = + adminRoleObject.hasgroups || + (adminRoleObject._claim_names?.groups && + adminRoleObject._claim_sources?.[adminRoleObject._claim_names.groups]); + + if (hasAdminOverage) { + const overageGroups = + resolvedOverageGroups || + (await resolveGroupsFromOverage(tokenset.access_token, claims.sub)); + if (overageGroups) { + adminRoles = overageGroups; + } + } + } + let adminRoleValues = []; if (Array.isArray(adminRoles)) { adminRoleValues = adminRoles; diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index 485b77829e..16fa548a59 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -64,6 +64,10 @@ jest.mock('openid-client', () => { // Only return additional properties, but don't override any claims return Promise.resolve({}); }), + genericGrantRequest: jest.fn().mockResolvedValue({ + access_token: 'exchanged_graph_token', + expires_in: 3600, + }), customFetch: Symbol('customFetch'), }; }); @@ -730,7 +734,7 @@ describe('setupOpenId', () => { expect.objectContaining({ method: 'POST', headers: expect.objectContaining({ - Authorization: `Bearer ${tokenset.access_token}`, + Authorization: 'Bearer exchanged_graph_token', }), }), ); @@ -745,6 +749,313 @@ describe('setupOpenId', () => { ); }); + describe('OBO token exchange for overage', () => { + it('exchanges access token via OBO before calling Graph API', async () => { + const openidClient = require('openid-client'); + 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({ hasgroups: true }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: ['group-required'] }), + }); + + await validate(tokenset); + + expect(openidClient.genericGrantRequest).toHaveBeenCalledWith( + expect.anything(), + 'urn:ietf:params:oauth:grant-type:jwt-bearer', + expect.objectContaining({ + scope: 'https://graph.microsoft.com/User.Read', + assertion: tokenset.access_token, + requested_token_use: 'on_behalf_of', + }), + ); + + expect(undici.fetch).toHaveBeenCalledWith( + 'https://graph.microsoft.com/v1.0/me/getMemberObjects', + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: 'Bearer exchanged_graph_token', + }), + }), + ); + }); + + it('caches the exchanged token and reuses it on subsequent calls', async () => { + const openidClient = require('openid-client'); + const getLogStores = require('~/cache/getLogStores'); + const mockSet = jest.fn(); + const mockGet = jest + .fn() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce({ access_token: 'exchanged_graph_token' }); + getLogStores.mockReturnValue({ get: mockGet, set: mockSet }); + + 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({ hasgroups: true }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: ['group-required'] }), + }); + + // First call: cache miss → OBO exchange → cache set + await validate(tokenset); + expect(mockSet).toHaveBeenCalledWith( + '1234:overage', + { access_token: 'exchanged_graph_token' }, + 3600000, + ); + expect(openidClient.genericGrantRequest).toHaveBeenCalledTimes(1); + + // Second call: cache hit → no new OBO exchange + openidClient.genericGrantRequest.mockClear(); + await validate(tokenset); + expect(openidClient.genericGrantRequest).not.toHaveBeenCalled(); + }); + }); + + describe('admin role group overage', () => { + it('resolves admin groups via Graph when overage is detected for admin role', async () => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + process.env.OPENID_ADMIN_ROLE = 'admin-group-id'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'id'; + + jwtDecode.mockReturnValue({ hasgroups: true }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: ['group-required', 'admin-group-id'] }), + }); + + const { user } = await validate(tokenset); + + expect(user.role).toBe('ADMIN'); + }); + + it('does not grant admin when overage groups do not contain admin role', async () => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + process.env.OPENID_ADMIN_ROLE = 'admin-group-id'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'id'; + + jwtDecode.mockReturnValue({ hasgroups: true }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: ['group-required', 'other-group'] }), + }); + + const { user } = await validate(tokenset); + + expect(user).toBeTruthy(); + expect(user.role).toBeUndefined(); + }); + + it('reuses already-resolved overage groups for admin role check (no duplicate Graph call)', async () => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + process.env.OPENID_ADMIN_ROLE = 'admin-group-id'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'id'; + + jwtDecode.mockReturnValue({ hasgroups: true }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: ['group-required', 'admin-group-id'] }), + }); + + await validate(tokenset); + + // Graph API should be called only once (for required role), admin role reuses the result + expect(undici.fetch).toHaveBeenCalledTimes(1); + }); + + it('demotes existing admin when overage groups no longer contain admin role', async () => { + process.env.OPENID_REQUIRED_ROLE = 'group-required'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + process.env.OPENID_ADMIN_ROLE = 'admin-group-id'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'id'; + + const existingAdminUser = { + _id: 'existingAdminId', + provider: 'openid', + email: tokenset.claims().email, + openidId: tokenset.claims().sub, + username: 'adminuser', + name: 'Admin User', + role: 'ADMIN', + }; + + findUser.mockImplementation(async (query) => { + if (query.openidId === tokenset.claims().sub || query.email === tokenset.claims().email) { + return existingAdminUser; + } + return null; + }); + + jwtDecode.mockReturnValue({ hasgroups: true }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: ['group-required'] }), + }); + + const { user } = await validate(tokenset); + + expect(user.role).toBe('USER'); + }); + + it('does not attempt overage for admin role when token kind is not id', async () => { + process.env.OPENID_REQUIRED_ROLE = 'requiredRole'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'roles'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + process.env.OPENID_ADMIN_ROLE = 'admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'access'; + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + hasgroups: true, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + const { user } = await validate(tokenset); + + // No Graph call since admin uses access token (not id) + expect(undici.fetch).not.toHaveBeenCalled(); + expect(user.role).toBeUndefined(); + }); + + it('resolves admin via Graph independently when OPENID_REQUIRED_ROLE is not configured', async () => { + delete process.env.OPENID_REQUIRED_ROLE; + process.env.OPENID_ADMIN_ROLE = 'admin-group-id'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'id'; + + jwtDecode.mockReturnValue({ hasgroups: true }); + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: ['admin-group-id'] }), + }); + + const { user } = await validate(tokenset); + expect(user.role).toBe('ADMIN'); + expect(undici.fetch).toHaveBeenCalledTimes(1); + }); + + it('denies admin when OPENID_REQUIRED_ROLE is absent and Graph does not contain admin group', async () => { + delete process.env.OPENID_REQUIRED_ROLE; + process.env.OPENID_ADMIN_ROLE = 'admin-group-id'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'groups'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'id'; + + jwtDecode.mockReturnValue({ hasgroups: true }); + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + undici.fetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ value: ['other-group'] }), + }); + + const { user } = await validate(tokenset); + expect(user).toBeTruthy(); + expect(user.role).toBeUndefined(); + }); + + it('denies login and logs error when OBO exchange throws', async () => { + const openidClient = require('openid-client'); + 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({ hasgroups: true }); + openidClient.genericGrantRequest.mockRejectedValueOnce(new Error('OBO exchange rejected')); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + const { user, details } = await validate(tokenset); + expect(user).toBe(false); + expect(details.message).toBe('You must have "group-required" role to log in.'); + expect(undici.fetch).not.toHaveBeenCalled(); + }); + + it('denies login when OBO exchange returns no access_token', async () => { + const openidClient = require('openid-client'); + 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({ hasgroups: true }); + openidClient.genericGrantRequest.mockResolvedValueOnce({ expires_in: 3600 }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + const { user, details } = await validate(tokenset); + expect(user).toBe(false); + expect(details.message).toBe('You must have "group-required" role to log in.'); + expect(undici.fetch).not.toHaveBeenCalled(); + }); + }); + it('should attempt to download and save the avatar if picture is provided', async () => { // Act const { user } = await validate(tokenset);