Merge pull request #30082 from UsainBloot/catalog-org-gitlab/ingest-sub-groups

[Catalog] GitLab Org SAAS - Ingest Users from Sub Groups

.changeset/major-heads-scream.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend-module-gitlab': minor
+---
+
+**BREAKING CHANGE**: User and Group discovery will default to ingesting all users in sub groups that belong to the specified root group in config. Disable by setting `restrictUsersToGroup: true` in app-config under your module settings.

plugins/catalog-backend-module-gitlab/src/__testUtils__/handlers.ts

@@ -30,6 +30,9 @@ import {
   some_endpoint,
   unhealthy_endpoint,
   userID,
+  all_saas_subgroup_1_members,
+  all_saas_subgroup_2_members,
+  group_with_subgroups_response,
 } from './mocks';
 
 const httpHandlers = [
@@ -71,6 +74,13 @@ const httpHandlers = [
     return res(ctx.set('x-next-page', ''), ctx.json(all_groups_response));
   }),
 
+  rest.get(`${apiBaseUrl}/groups/group-with-subgroup`, (_, res, ctx) => {
+    return res(
+      ctx.set('x-next-page', ''),
+      ctx.json(group_with_subgroups_response),
+    );
+  }),
+
   rest.get(`${apiBaseUrl}/groups/42`, (_, res, ctx) => {
     return res(ctx.status(500), ctx.json({ error: 'Internal Server Error' }));
   }),
@@ -89,6 +99,24 @@ const httpHandlers = [
     },
   ),
 
+  rest.get(`${apiBaseUrlSaas}/groups/456/members/all`, (_req, res, ctx) => {
+    return res(ctx.json(all_saas_users_response));
+  }),
+
+  rest.get(`${apiBaseUrlSaas}/groups/1/members/all`, (_req, res, ctx) => {
+    return res(ctx.json(all_saas_users_response));
+  }),
+
+  // Subgroup 1 members id=6
+  rest.get(`${apiBaseUrlSaas}/groups/6/members/all`, (_req, res, ctx) => {
+    return res(ctx.json(all_saas_subgroup_1_members));
+  }),
+
+  // Subgroup 2 members id=7
+  rest.get(`${apiBaseUrlSaas}/groups/7/members/all`, (_req, res, ctx) => {
+    return res(ctx.json(all_saas_subgroup_2_members));
+  }),
+
   /**
    * Users REST endpoint mocks
    */
@@ -608,7 +636,47 @@ const graphqlHandlers = [
 
   graphql
     .link(saasGraphQlBaseUrl)
-    .query('listDescendantGroups', async (_, res, ctx) => {
+    .query('listDescendantGroups', async (req, res, ctx) => {
+      const { group } = req.variables;
+
+      if (group === 'group1') {
+        return res(
+          ctx.data({
+            group: {
+              descendantGroups: {
+                nodes: req.variables.endCursor
+                  ? [
+                      {
+                        id: 'gid://gitlab/Group/6',
+                        name: 'subgroup1',
+                        description: 'description1',
+                        fullPath: 'path/subgroup1',
+                        parent: {
+                          id: '1',
+                        },
+                      },
+                    ]
+                  : [
+                      {
+                        id: 'gid://gitlab/Group/7',
+                        name: 'subgroup2',
+                        description: 'description2',
+                        fullPath: 'path/subgroup2',
+                        parent: {
+                          id: '1',
+                        },
+                      },
+                    ],
+                pageInfo: {
+                  endCursor: req.variables.endCursor ? 'end' : 'next',
+                  hasNextPage: !req.variables.endCursor,
+                },
+              },
+            },
+          }),
+        );
+      }
+
       return res(
         ctx.data({
           group: {

plugins/catalog-backend-module-gitlab/src/__testUtils__/mocks.ts

@@ -796,6 +796,30 @@ export const config_org_group_restrictUsers_true_selfHosted = {
     },
   },
 };
+
+export const config_org_group_with_subgroups_sass = {
+  integrations: {
+    gitlab: [
+      {
+        host: 'gitlab.com',
+        apiBaseUrl: 'https://gitlab.com/api/v4',
+        token: '1234',
+      },
+    ],
+  },
+  catalog: {
+    providers: {
+      gitlab: {
+        'test-id': {
+          host: 'gitlab.com',
+          group: 'group1',
+          orgEnabled: true,
+          skipForkedRepos: true,
+        },
+      },
+    },
+  },
+};
 /**
  * GitLab API responses
  */
@@ -1020,7 +1044,72 @@ export const all_saas_users_response: MockObject[] = [
     access_level: 30,
     created_at: '2023-07-19T08:58:34.984Z',
     expires_at: null,
-    id: 34,
+    id: 36,
+    username: 'testusernoseat1',
+    name: 'Test User No Seat 1',
+    state: 'active',
+    avatar_url: 'https://secure.gravatar.com/',
+    web_url: 'https://gitlab.com/testusernoseat1',
+    email: 'testusernoseat1@example.com',
+    group_saml_identity: {
+      provider: 'group_saml',
+      extern_uid: '60',
+      saml_provider_id: 1,
+    },
+    is_using_seat: false,
+    membership_state: 'active',
+  },
+];
+
+// Only test user 1
+export const all_saas_group_with_subgroups_members: MockObject[] = [
+  {
+    access_level: 30,
+    created_at: '2023-07-17T08:58:34.984Z',
+    expires_at: null,
+    id: 12,
+    username: 'testuser1',
+    name: 'Test User 1',
+    state: 'active',
+    avatar_url: 'https://secure.gravatar.com/',
+    web_url: 'https://gitlab.com/testuser1',
+    email: 'testuser1@example.com',
+    group_saml_identity: {
+      provider: 'group_saml',
+      extern_uid: '50',
+      saml_provider_id: 1,
+    },
+    is_using_seat: true,
+    membership_state: 'active',
+  },
+];
+
+// Only test user 1
+export const all_saas_subgroup_1_members: MockObject[] = [
+  {
+    access_level: 30,
+    created_at: '2023-07-17T08:58:34.984Z',
+    expires_at: null,
+    id: 12,
+    username: 'testuser1',
+    name: 'Test User 1',
+    state: 'active',
+    avatar_url: 'https://secure.gravatar.com/',
+    web_url: 'https://gitlab.com/testuser1',
+    email: 'testuser1@example.com',
+    group_saml_identity: {
+      provider: 'group_saml',
+      extern_uid: '51',
+      saml_provider_id: 1,
+    },
+    is_using_seat: true,
+    membership_state: 'active',
+  },
+  {
+    access_level: 30,
+    created_at: '2023-07-17T08:58:34.984Z',
+    expires_at: null,
+    id: 33,
     username: 'testuser3',
     name: 'Test User 3',
     state: 'active',
@@ -1032,7 +1121,49 @@ export const all_saas_users_response: MockObject[] = [
       extern_uid: '53',
       saml_provider_id: 1,
     },
-    is_using_seat: false,
+    is_using_seat: true,
+    membership_state: 'active',
+  },
+];
+
+// Only test user 2
+export const all_saas_subgroup_2_members: MockObject[] = [
+  {
+    access_level: 30,
+    created_at: '2023-07-19T08:58:34.984Z',
+    expires_at: null,
+    id: 34,
+    username: 'testuser2',
+    name: 'Test User 2',
+    state: 'active',
+    avatar_url: 'https://secure.gravatar.com/',
+    web_url: 'https://gitlab.com/testuser2',
+    email: 'testuser2@example.com',
+    group_saml_identity: {
+      provider: 'group_saml',
+      extern_uid: '52',
+      saml_provider_id: 1,
+    },
+    is_using_seat: true,
+    membership_state: 'active',
+  },
+  {
+    access_level: 30,
+    created_at: '2023-07-17T08:58:34.984Z',
+    expires_at: null,
+    id: 44,
+    username: 'testuser4',
+    name: 'Test User 4',
+    state: 'active',
+    avatar_url: 'https://secure.gravatar.com/',
+    web_url: 'https://gitlab.com/testuser4',
+    email: 'testuser4@example.com',
+    group_saml_identity: {
+      provider: 'group_saml',
+      extern_uid: '54',
+      saml_provider_id: 1,
+    },
+    is_using_seat: true,
     membership_state: 'active',
   },
 ];
@@ -1074,6 +1205,21 @@ export const all_groups_response: GitLabGroup[] = [
     description: '',
     full_path: 'group1/subgroup1',
   },
+  {
+    id: 7,
+    name: 'subgroup2',
+    description: '',
+    full_path: 'group1/subgroup2',
+  },
+];
+
+export const group_with_subgroups_response: GitLabGroup[] = [
+  {
+    id: 1,
+    name: 'group1',
+    description: 'description1',
+    full_path: 'group1',
+  },
 ];
 
 export const group_with_parent: MockObject[] = [
@@ -2183,6 +2329,58 @@ export const expected_full_org_scan_entities_saas: MockObject[] = [
     },
     locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
   },
+  {
+    entity: {
+      apiVersion: 'backstage.io/v1alpha1',
+      kind: 'User',
+      metadata: {
+        annotations: {
+          'backstage.io/managed-by-location':
+            'url:https://gitlab.com/testuser4',
+          'backstage.io/managed-by-origin-location':
+            'url:https://gitlab.com/testuser4',
+          'gitlab.com/user-login': 'https://gitlab.com/testuser4',
+          'gitlab.com/saml-external-uid': '54',
+        },
+        name: 'testuser4',
+      },
+      spec: {
+        memberOf: [],
+        profile: {
+          displayName: 'Test User 4',
+          email: 'testuser4@example.com',
+          picture: 'https://secure.gravatar.com/',
+        },
+      },
+    },
+    locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
+  },
+  {
+    entity: {
+      apiVersion: 'backstage.io/v1alpha1',
+      kind: 'User',
+      metadata: {
+        annotations: {
+          'backstage.io/managed-by-location':
+            'url:https://gitlab.com/testuser3',
+          'backstage.io/managed-by-origin-location':
+            'url:https://gitlab.com/testuser3',
+          'gitlab.com/user-login': 'https://gitlab.com/testuser3',
+          'gitlab.com/saml-external-uid': '53',
+        },
+        name: 'testuser3',
+      },
+      spec: {
+        memberOf: [],
+        profile: {
+          displayName: 'Test User 3',
+          email: 'testuser3@example.com',
+          picture: 'https://secure.gravatar.com/',
+        },
+      },
+    },
+    locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
+  },
 ];
 
 export const expected_full_org_scan_entities_includeUsersWithoutSeat_saas: MockObject[] =
@@ -2239,6 +2437,58 @@ export const expected_full_org_scan_entities_includeUsersWithoutSeat_saas: MockO
       },
       locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
     },
+    {
+      entity: {
+        apiVersion: 'backstage.io/v1alpha1',
+        kind: 'User',
+        metadata: {
+          annotations: {
+            'backstage.io/managed-by-location':
+              'url:https://gitlab.com/testusernoseat1',
+            'backstage.io/managed-by-origin-location':
+              'url:https://gitlab.com/testusernoseat1',
+            'gitlab.com/user-login': 'https://gitlab.com/testusernoseat1',
+            'gitlab.com/saml-external-uid': '60',
+          },
+          name: 'testusernoseat1',
+        },
+        spec: {
+          memberOf: [],
+          profile: {
+            displayName: 'Test User No Seat 1',
+            email: 'testusernoseat1@example.com',
+            picture: 'https://secure.gravatar.com/',
+          },
+        },
+      },
+      locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
+    },
+    {
+      entity: {
+        apiVersion: 'backstage.io/v1alpha1',
+        kind: 'User',
+        metadata: {
+          annotations: {
+            'backstage.io/managed-by-location':
+              'url:https://gitlab.com/testuser4',
+            'backstage.io/managed-by-origin-location':
+              'url:https://gitlab.com/testuser4',
+            'gitlab.com/user-login': 'https://gitlab.com/testuser4',
+            'gitlab.com/saml-external-uid': '54',
+          },
+          name: 'testuser4',
+        },
+        spec: {
+          memberOf: [],
+          profile: {
+            displayName: 'Test User 4',
+            email: 'testuser4@example.com',
+            picture: 'https://secure.gravatar.com/',
+          },
+        },
+      },
+      locationKey: 'GitlabOrgDiscoveryEntityProvider:test-id',
+    },
     {
       entity: {
         apiVersion: 'backstage.io/v1alpha1',

plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts

@@ -303,7 +303,7 @@ describe('GitlabOrgDiscoveryEntityProvider - refresh', () => {
     });
   });
 
-  // This should return all members of the SaaS Root group (group1) -> expected_full_org_scan_entities_saas
+  // This should return all members of the SaaS Root group (group1) and any subgroups -> expected_full_org_scan_entities_saas
   it('SaaS: should get all saas root group users when restrictUsersToGroup is not set', async () => {
     const config = new ConfigReader(mock.config_org_group_saas);
     const schedule = new PersistingTaskRunner();
@@ -334,7 +334,7 @@ describe('GitlabOrgDiscoveryEntityProvider - refresh', () => {
     });
   });
 
-  // This should return all members of the SaaS Root group (group1) -> expected_full_org_scan_entities_saas
+  // This should return all members of the SaaS Root group (group1) and any subgroups -> expected_full_org_scan_entities_saas
   it('SaaS: should get all saas root group users when restrictUsersToGroup is false', async () => {
     const config = new ConfigReader(
       mock.config_org_group_restrictUsers_false_saas,

plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts

@@ -359,7 +359,7 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider {
     }
 
     let groups;
-    let users;
+    const allUsers = [];
 
     // Self-hosted: Fetch the users either from the defined group (restrictUsersToGroup) or fetch all users from the GitLab instance
     // SaaS: Fetch the users from the defined group (restrictUsersToGroup) or fetch all users from the root group.
@@ -367,13 +367,15 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider {
       groups = (await this.gitLabClient.listDescendantGroups(this.config.group))
         .items;
       groups.push(await this.gitLabClient.getGroupByPath(this.config.group)); // adds the parent group for #26554
-      users = paginated<GitLabUser>(
-        options =>
-          this.gitLabClient.listGroupMembers(this.config.group, options), // calls /groups/<groupId>/members
-        {
-          page: 1,
-          per_page: 100,
-        },
+      allUsers.push(
+        paginated<GitLabUser>(
+          options =>
+            this.gitLabClient.listGroupMembers(this.config.group, options), // calls /groups/<groupId>/members
+          {
+            page: 1,
+            per_page: 100,
+          },
+        ),
       );
     } else if (
       this.gitLabClient.isSelfManaged() &&
@@ -387,35 +389,46 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider {
           all_available: true,
         },
       );
-      users = paginated<GitLabUser>(
-        options => this.gitLabClient.listUsers(options), // calls /users?
-        { page: 1, per_page: 100, active: true },
+      allUsers.push(
+        paginated<GitLabUser>(
+          options => this.gitLabClient.listUsers(options), // calls /users?
+          { page: 1, per_page: 100, active: true },
+        ),
       );
     }
     // SaaS: Fetch the users from the defined group (restrictUsersToGroup) or fetch all users from the root group.
     else {
-      groups = (await this.gitLabClient.listDescendantGroups(this.config.group))
-        .items;
+      const descendantGroups = (
+        await this.gitLabClient.listDescendantGroups(this.config.group)
+      ).items;
+      groups = descendantGroups;
 
       groups.push(await this.gitLabClient.getGroupByPath(this.config.group)); // adds the parent group for #26554
 
       const rootGroupSplit = this.config.group.split('/');
-      const groupPath = this.config.restrictUsersToGroup
-        ? this.config.group
-        : rootGroupSplit[0];
 
-      users = paginated<GitLabUser>(
-        options =>
-          this.gitLabClient.listSaaSUsers(
-            groupPath,
-            options,
-            this.config.includeUsersWithoutSeat,
+      const groupPaths = this.config.restrictUsersToGroup
+        ? [this.config.group]
+        : [rootGroupSplit[0], ...descendantGroups.map(g => `${g.id}`)];
+
+      // Fetch users group and descendant groups
+      for (const group of groupPaths) {
+        logger.debug(`Fetching users for group: ${group}`);
+        allUsers.push(
+          paginated<GitLabUser>(
+            options =>
+              this.gitLabClient.listSaaSUsers(
+                group,
+                options,
+                this.config.includeUsersWithoutSeat,
+              ),
+            {
+              page: 1,
+              per_page: 100,
+            },
           ),
-        {
-          page: 1,
-          per_page: 100,
-        },
-      );
+        );
+      }
     }
 
     const idMappedUser: { [userId: number]: GitLabUser } = {};
@@ -430,16 +443,23 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider {
       matches: [],
     };
 
-    for await (const user of users) {
-      userRes.scanned++;
+    for (const users of allUsers) {
+      // Iterate through paginated users
+      for await (const user of users) {
+        // Skip if user already processed and do not count it as scanned
+        if (user.id in idMappedUser) {
+          continue; // Skip if user already processed
+        }
+        userRes.scanned++;
 
-      if (!this.shouldProcessUser(user)) {
-        logger.debug(`Skipped user: ${user.username}`);
-        continue;
+        if (!this.shouldProcessUser(user)) {
+          logger.debug(`Skipped user: ${user.username}`);
+          continue;
+        }
+
+        idMappedUser[user.id] = user;
+        userRes.matches.push(user);
       }
-
-      idMappedUser[user.id] = user;
-      userRes.matches.push(user);
     }
 
     for await (const group of groups) {