diff --git a/api/server/services/GraphApiService.js b/api/server/services/GraphApiService.js index 82fa245d58..08ca253964 100644 --- a/api/server/services/GraphApiService.js +++ b/api/server/services/GraphApiService.js @@ -159,7 +159,7 @@ const searchEntraIdPrincipals = async (accessToken, sub, query, type = 'all', li /** * Get current user's Entra ID group memberships from Microsoft Graph - * Uses /me/memberOf endpoint to get groups the user is a member of + * Uses /me/getMemberGroups endpoint to get transitive groups the user is a member of * @param {string} accessToken - OpenID Connect access token * @param {string} sub - Subject identifier * @returns {Promise>} Array of group ID strings (GUIDs) @@ -167,10 +167,12 @@ const searchEntraIdPrincipals = async (accessToken, sub, query, type = 'all', li const getUserEntraGroups = async (accessToken, sub) => { try { const graphClient = await createGraphClient(accessToken, sub); + const response = await graphClient + .api('/me/getMemberGroups') + .post({ securityEnabledOnly: false }); - const groupsResponse = await graphClient.api('/me/memberOf').select('id').get(); - - return (groupsResponse.value || []).map((group) => group.id); + const groupIds = Array.isArray(response?.value) ? response.value : []; + return [...new Set(groupIds.map((groupId) => String(groupId)))]; } catch (error) { logger.error('[getUserEntraGroups] Error fetching user groups:', error); return []; @@ -187,13 +189,22 @@ const getUserEntraGroups = async (accessToken, sub) => { const getUserOwnedEntraGroups = async (accessToken, sub) => { try { const graphClient = await createGraphClient(accessToken, sub); + const allGroupIds = []; + let nextLink = '/me/ownedObjects/microsoft.graph.group'; - const groupsResponse = await graphClient - .api('/me/ownedObjects/microsoft.graph.group') - .select('id') - .get(); + while (nextLink) { + const response = await graphClient.api(nextLink).select('id').top(999).get(); + const groups = response?.value || []; + allGroupIds.push(...groups.map((group) => group.id)); - return (groupsResponse.value || []).map((group) => group.id); + nextLink = response['@odata.nextLink'] + ? response['@odata.nextLink'] + .replace(/^https:\/\/graph\.microsoft\.com\/v1\.0/, '') + .trim() || null + : null; + } + + return allGroupIds; } catch (error) { logger.error('[getUserOwnedEntraGroups] Error fetching user owned groups:', error); return []; @@ -211,21 +222,27 @@ const getUserOwnedEntraGroups = async (accessToken, sub) => { const getGroupMembers = async (accessToken, sub, groupId) => { try { const graphClient = await createGraphClient(accessToken, sub); - const allMembers = []; - let nextLink = `/groups/${groupId}/members`; + const allMembers = new Set(); + let nextLink = `/groups/${groupId}/transitiveMembers`; while (nextLink) { const membersResponse = await graphClient.api(nextLink).select('id').top(999).get(); - const members = membersResponse.value || []; - allMembers.push(...members.map((member) => member.id)); + const members = membersResponse?.value || []; + members.forEach((member) => { + if (typeof member?.id === 'string' && member['@odata.type'] === '#microsoft.graph.user') { + allMembers.add(member.id); + } + }); nextLink = membersResponse['@odata.nextLink'] - ? membersResponse['@odata.nextLink'].split('/v1.0')[1] + ? membersResponse['@odata.nextLink'] + .replace(/^https:\/\/graph\.microsoft\.com\/v1\.0/, '') + .trim() || null : null; } - return allMembers; + return Array.from(allMembers); } catch (error) { logger.error('[getGroupMembers] Error fetching group members:', error); return []; diff --git a/api/server/services/GraphApiService.spec.js b/api/server/services/GraphApiService.spec.js index 5d8dd62cf5..fa11190cc3 100644 --- a/api/server/services/GraphApiService.spec.js +++ b/api/server/services/GraphApiService.spec.js @@ -73,6 +73,7 @@ describe('GraphApiService', () => { header: jest.fn().mockReturnThis(), top: jest.fn().mockReturnThis(), get: jest.fn(), + post: jest.fn(), }; Client.init.mockReturnValue(mockGraphClient); @@ -514,31 +515,33 @@ describe('GraphApiService', () => { }); describe('getUserEntraGroups', () => { - it('should fetch user groups from memberOf endpoint', async () => { + it('should fetch user groups using getMemberGroups endpoint', async () => { const mockGroupsResponse = { - value: [ - { - id: 'group-1', - }, - { - id: 'group-2', - }, - ], + value: ['group-1', 'group-2'], }; - mockGraphClient.get.mockResolvedValue(mockGroupsResponse); + mockGraphClient.post.mockResolvedValue(mockGroupsResponse); const result = await GraphApiService.getUserEntraGroups('token', 'user'); - expect(mockGraphClient.api).toHaveBeenCalledWith('/me/memberOf'); - expect(mockGraphClient.select).toHaveBeenCalledWith('id'); + expect(mockGraphClient.api).toHaveBeenCalledWith('/me/getMemberGroups'); + expect(mockGraphClient.post).toHaveBeenCalledWith({ securityEnabledOnly: false }); + + expect(result).toEqual(['group-1', 'group-2']); + }); + + it('should deduplicate returned group ids', async () => { + mockGraphClient.post.mockResolvedValue({ + value: ['group-1', 'group-2', 'group-1'], + }); + + const result = await GraphApiService.getUserEntraGroups('token', 'user'); - expect(result).toHaveLength(2); expect(result).toEqual(['group-1', 'group-2']); }); it('should return empty array on error', async () => { - mockGraphClient.get.mockRejectedValue(new Error('API error')); + mockGraphClient.post.mockRejectedValue(new Error('API error')); const result = await GraphApiService.getUserEntraGroups('token', 'user'); @@ -550,7 +553,7 @@ describe('GraphApiService', () => { value: [], }; - mockGraphClient.get.mockResolvedValue(mockGroupsResponse); + mockGraphClient.post.mockResolvedValue(mockGroupsResponse); const result = await GraphApiService.getUserEntraGroups('token', 'user'); @@ -558,7 +561,7 @@ describe('GraphApiService', () => { }); it('should handle missing value property', async () => { - mockGraphClient.get.mockResolvedValue({}); + mockGraphClient.post.mockResolvedValue({}); const result = await GraphApiService.getUserEntraGroups('token', 'user'); @@ -566,6 +569,89 @@ describe('GraphApiService', () => { }); }); + describe('getUserOwnedEntraGroups', () => { + it('should fetch owned groups with pagination support', async () => { + const firstPage = { + value: [ + { + id: 'owned-group-1', + }, + ], + '@odata.nextLink': + 'https://graph.microsoft.com/v1.0/me/ownedObjects/microsoft.graph.group?$skiptoken=xyz', + }; + + const secondPage = { + value: [ + { + id: 'owned-group-2', + }, + ], + }; + + mockGraphClient.get.mockResolvedValueOnce(firstPage).mockResolvedValueOnce(secondPage); + + const result = await GraphApiService.getUserOwnedEntraGroups('token', 'user'); + + expect(mockGraphClient.api).toHaveBeenNthCalledWith( + 1, + '/me/ownedObjects/microsoft.graph.group', + ); + expect(mockGraphClient.api).toHaveBeenNthCalledWith( + 2, + '/me/ownedObjects/microsoft.graph.group?$skiptoken=xyz', + ); + expect(mockGraphClient.top).toHaveBeenCalledWith(999); + expect(mockGraphClient.get).toHaveBeenCalledTimes(2); + + expect(result).toEqual(['owned-group-1', 'owned-group-2']); + }); + + it('should return empty array on error', async () => { + mockGraphClient.get.mockRejectedValue(new Error('API error')); + + const result = await GraphApiService.getUserOwnedEntraGroups('token', 'user'); + + expect(result).toEqual([]); + }); + }); + + describe('getGroupMembers', () => { + it('should fetch transitive members and include only users', async () => { + const firstPage = { + value: [ + { id: 'user-1', '@odata.type': '#microsoft.graph.user' }, + { id: 'child-group', '@odata.type': '#microsoft.graph.group' }, + ], + '@odata.nextLink': + 'https://graph.microsoft.com/v1.0/groups/group-id/transitiveMembers?$skiptoken=abc', + }; + const secondPage = { + value: [{ id: 'user-2', '@odata.type': '#microsoft.graph.user' }], + }; + + mockGraphClient.get.mockResolvedValueOnce(firstPage).mockResolvedValueOnce(secondPage); + + const result = await GraphApiService.getGroupMembers('token', 'user', 'group-id'); + + expect(mockGraphClient.api).toHaveBeenNthCalledWith(1, '/groups/group-id/transitiveMembers'); + expect(mockGraphClient.api).toHaveBeenNthCalledWith( + 2, + '/groups/group-id/transitiveMembers?$skiptoken=abc', + ); + expect(mockGraphClient.top).toHaveBeenCalledWith(999); + expect(result).toEqual(['user-1', 'user-2']); + }); + + it('should return empty array on error', async () => { + mockGraphClient.get.mockRejectedValue(new Error('API error')); + + const result = await GraphApiService.getGroupMembers('token', 'user', 'group-id'); + + expect(result).toEqual([]); + }); + }); + describe('testGraphApiAccess', () => { beforeEach(() => { jest.clearAllMocks();