👫 fix: Update Entra ID group retrieval to use getMemberGroups and add pagination support (#10199)

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<string>>} 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();
+    const groupIds = Array.isArray(response?.value) ? response.value : [];
-
+    return [...new Set(groupIds.map((groupId) => String(groupId)))];
-    return (groupsResponse.value || []).map((group) => group.id);
   } 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
+    while (nextLink) {
-      .api('/me/ownedObjects/microsoft.graph.group')
+      const response = await graphClient.api(nextLink).select('id').top(999).get();
-      .select('id')
+      const groups = response?.value || [];
-      .get();
+      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 = [];
+    const allMembers = new Set();
-    let nextLink = `/groups/${groupId}/members`;
+    let nextLink = `/groups/${groupId}/transitiveMembers`;
 
     while (nextLink) {
       const membersResponse = await graphClient.api(nextLink).select('id').top(999).get();
 
-      const members = membersResponse.value || [];
+      const members = membersResponse?.value || [];
-      allMembers.push(...members.map((member) => member.id));
+      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 [];

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: [
+        value: ['group-1', 'group-2'],
-          {
-            id: 'group-1',
-          },
-          {
-            id: '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.api).toHaveBeenCalledWith('/me/getMemberGroups');
-      expect(mockGraphClient.select).toHaveBeenCalledWith('id');
+      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();