From fae0735ee03778f9995e717f2c31d0ca4183b23b Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Mon, 25 Sep 2023 11:59:01 +0200 Subject: [PATCH 1/5] fix: use publicEmail in GitLab client client was using commitEmail that is visible only for self-managed GitLab admins. publicEmail is visible, but only if the user explicitly enable the corresponding setting on GitLab. Signed-off-by: Matteo Silvestri --- plugins/catalog-backend-module-gitlab/src/lib/client.ts | 4 ++-- plugins/catalog-backend-module-gitlab/src/lib/types.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index 79ae328ce2..0b69ded359 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -205,7 +205,7 @@ export class GitLabClient { user { id username - commitEmail + publicEmail name state webUrl @@ -240,7 +240,7 @@ export class GitLabClient { const formattedUserResponse = { id: Number(userItem.user.id.replace(/^gid:\/\/gitlab\/User\//, '')), username: userItem.user.username, - email: userItem.user.commitEmail, + email: userItem.user.publicEmail, name: userItem.user.name, state: userItem.user.state, web_url: userItem.user.webUrl, diff --git a/plugins/catalog-backend-module-gitlab/src/lib/types.ts b/plugins/catalog-backend-module-gitlab/src/lib/types.ts index a339ac57e4..5a4082eec3 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/types.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/types.ts @@ -64,7 +64,7 @@ export type GitLabGroupMembersResponse = { user: { id: string; username: string; - commitEmail: string; + publicEmail: string; name: string; state: string; webUrl: string; From 4f70fdfc93aa4e081b97bcb4eb6cda88f8493ce0 Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Mon, 25 Sep 2023 16:17:19 +0200 Subject: [PATCH 2/5] fix gitlab client tests and add changeset Signed-off-by: Matteo Silvestri --- .changeset/flat-rules-rush.md | 7 +++++++ .../catalog-backend-module-gitlab/src/lib/client.test.ts | 8 ++++---- .../providers/GitlabOrgDiscoveryEntityProvider.test.ts | 4 ++-- 3 files changed, 13 insertions(+), 6 deletions(-) create mode 100644 .changeset/flat-rules-rush.md diff --git a/.changeset/flat-rules-rush.md b/.changeset/flat-rules-rush.md new file mode 100644 index 0000000000..f9f5354595 --- /dev/null +++ b/.changeset/flat-rules-rush.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend-module-gitlab': patch +--- + +fix: use publicEmail in GitLab client + +client was using commitEmail that is visible only for self-managed GitLab admins. publicEmail is visible, but only if the user explicitly enable the corresponding setting on GitLab. diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.test.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.test.ts index dda30fd86a..2bb1f713b5 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.test.ts @@ -413,7 +413,7 @@ describe('GitLabClient', () => { user: { id: 'gid://gitlab/User/1', username: 'user1', - commitEmail: 'user1@example.com', + publicEmail: 'user1@example.com', name: 'user1', state: 'active', webUrl: 'user1.com', @@ -517,7 +517,7 @@ describe('GitLabClient', () => { user: { id: 'gid://gitlab/User/1', username: 'user1', - commitEmail: 'user1@example.com', + publicEmail: 'user1@example.com', name: 'user1', state: 'active', webUrl: 'user1.com', @@ -530,7 +530,7 @@ describe('GitLabClient', () => { user: { id: 'gid://gitlab/User/2', username: 'user2', - commitEmail: 'user2@example.com', + publicEmail: 'user2@example.com', name: 'user2', state: 'active', webUrl: 'user2.com', @@ -764,7 +764,7 @@ describe('GitLabClient', () => { user: { id: 'gid://gitlab/User/1', username: 'user1', - commitEmail: 'user1@example.com', + publicEmail: 'user1@example.com', name: 'user1', state: 'active', webUrl: 'user1.com', diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts index ce00510140..1b1357c1bf 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts @@ -560,7 +560,7 @@ describe('GitlabOrgDiscoveryEntityProvider', () => { user: { id: 'gid://gitlab/User/12', username: 'testuser1', - commitEmail: 'testuser1@example.com', + publicEmail: 'testuser1@example.com', state: 'active', name: 'Test User 1', webUrl: 'https://gitlab.com/testuser1', @@ -571,7 +571,7 @@ describe('GitlabOrgDiscoveryEntityProvider', () => { user: { id: 'gid://gitlab/User/34', username: 'testuser2', - commitEmail: 'testuser2@example.com', + publicEmail: 'testuser2@example.com', state: 'active', name: 'Test User 2', webUrl: 'https://gitlab.com/testuser2', From 753b35afb56268a2080875159f65ca1f449e51a2 Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Tue, 3 Oct 2023 10:38:47 +0200 Subject: [PATCH 3/5] use GitLab group members REST API Signed-off-by: Matteo Silvestri --- .changeset/flat-rules-rush.md | 11 +++++++++-- .../src/lib/client.ts | 16 ++++++++++++++++ .../GitlabOrgDiscoveryEntityProvider.ts | 14 ++++++++------ 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/.changeset/flat-rules-rush.md b/.changeset/flat-rules-rush.md index f9f5354595..f57e800d91 100644 --- a/.changeset/flat-rules-rush.md +++ b/.changeset/flat-rules-rush.md @@ -2,6 +2,13 @@ '@backstage/plugin-catalog-backend-module-gitlab': patch --- -fix: use publicEmail in GitLab client +fix: use REST API to get root group memberships for GitLab SaaS users listing -client was using commitEmail that is visible only for self-managed GitLab admins. publicEmail is visible, but only if the user explicitly enable the corresponding setting on GitLab. +This API is the only one that shows `email` field for enterprise users and +allows to filter out bot users not using a license using the `is_using_seat` +field. + +ref: + +https://docs.gitlab.com/ee/user/enterprise_user/#get-users-email-addresses-through-the-api +https://docs.gitlab.com/ee/api/members.html#limitations diff --git a/plugins/catalog-backend-module-gitlab/src/lib/client.ts b/plugins/catalog-backend-module-gitlab/src/lib/client.ts index 0b69ded359..5fb7e59dda 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/client.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/client.ts @@ -91,6 +91,22 @@ export class GitLabClient { }); } + async listSaaSUsers( + groupPath: string, + options?: CommonListOptions, + ): Promise> { + return this.pagedRequest( + `/groups/${encodeURIComponent(groupPath)}/members`, + { + ...options, + show_seat_info: true, + }, + ).then(resp => { + resp.items = resp.items.filter(user => user.is_using_seat); + return resp; + }); + } + async listGroups( options?: CommonListOptions, ): Promise> { diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts index 0ef0e2b23c..b98b8ab7c3 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -190,12 +190,14 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { }); } else { groups = (await client.listDescendantGroups(this.config.group)).items; - users = ( - await client.getGroupMembers(this.config.group.split('/')[0], [ - 'DIRECT', - 'DESCENDANTS', - ]) - ).items; + const rootGroup = this.config.group.split('/')[0]; + users = paginated( + options => client.listSaaSUsers(rootGroup, options), + { + page: 1, + per_page: 100, + }, + ); } const idMappedUser: { [userId: number]: GitLabUser } = {}; From 627a84204d952e848c4890407944c530ce37be12 Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Tue, 3 Oct 2023 16:00:39 +0200 Subject: [PATCH 4/5] add tests and documentation Signed-off-by: Matteo Silvestri --- .changeset/flat-rules-rush.md | 5 + .../src/lib/types.ts | 7 +- .../GitlabOrgDiscoveryEntityProvider.test.ts | 107 +++++++++++------- .../GitlabOrgDiscoveryEntityProvider.ts | 4 + 4 files changed, 80 insertions(+), 43 deletions(-) diff --git a/.changeset/flat-rules-rush.md b/.changeset/flat-rules-rush.md index f57e800d91..c68a4e5756 100644 --- a/.changeset/flat-rules-rush.md +++ b/.changeset/flat-rules-rush.md @@ -8,6 +8,11 @@ This API is the only one that shows `email` field for enterprise users and allows to filter out bot users not using a license using the `is_using_seat` field. +We also added the annotation `gitlab.com/saml-external-uid` taking the value +of `group_saml_identity.extern_uid` of the forementioned endpoint response. +This is useful in case you want to backreference the user with the id of your +identity provider (e.g. OneLogin). + ref: https://docs.gitlab.com/ee/user/enterprise_user/#get-users-email-addresses-through-the-api diff --git a/plugins/catalog-backend-module-gitlab/src/lib/types.ts b/plugins/catalog-backend-module-gitlab/src/lib/types.ts index 5a4082eec3..946317a11a 100644 --- a/plugins/catalog-backend-module-gitlab/src/lib/types.ts +++ b/plugins/catalog-backend-module-gitlab/src/lib/types.ts @@ -39,12 +39,17 @@ export type GitLabProject = { export type GitLabUser = { id: number; username: string; - email: string; + email?: string; name: string; state: string; web_url: string; avatar_url: string; groups?: GitLabGroup[]; + group_saml_identity?: GitLabGroupSamlIdentity; +}; + +export type GitLabGroupSamlIdentity = { + extern_uid: string; }; export type GitLabGroup = { diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts index 1b1357c1bf..ad930ea23d 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.test.ts @@ -548,46 +548,6 @@ describe('GitlabOrgDiscoveryEntityProvider', () => { }), ), ), - graphql - .link('https://gitlab.com/api/graphql') - .query('getGroupMembers', async (_, res, ctx) => - res( - ctx.data({ - group: { - groupMembers: { - nodes: [ - { - user: { - id: 'gid://gitlab/User/12', - username: 'testuser1', - publicEmail: 'testuser1@example.com', - state: 'active', - name: 'Test User 1', - webUrl: 'https://gitlab.com/testuser1', - avatarUrl: 'https://secure.gravatar.com/', - }, - }, - { - user: { - id: 'gid://gitlab/User/34', - username: 'testuser2', - publicEmail: 'testuser2@example.com', - state: 'active', - name: 'Test User 2', - webUrl: 'https://gitlab.com/testuser2', - avatarUrl: 'https://secure.gravatar.com/', - }, - }, - ], - pageInfo: { - endCursor: 'end', - hasNextPage: false, - }, - }, - }, - }), - ), - ), graphql .link('https://gitlab.com/api/graphql') .query('getGroupMembers', async (req, res, ctx) => @@ -608,6 +568,67 @@ describe('GitlabOrgDiscoveryEntityProvider', () => { }), ), ), + rest.get( + `https://gitlab.com/api/v4/groups/group1/members`, + (_req, res, ctx) => { + const response = [ + { + 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-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: 50, + created_at: '2023-07-15T08:58:34.984Z', + expires_at: '2023-10-26', + id: 54, + username: 'group_100_bot_23dc8057bef66e05181f39be4652577c', + name: 'Token Bot', + state: 'active', + avatar_url: 'https://secure.gravatar.com/', + web_url: + 'https://gitlab.com/group_100_bot_23dc8057bef66e05181f39be4652577c', + group_saml_identity: null, + is_using_seat: false, + membership_state: 'active', + }, + ]; + return res(ctx.json(response)); + }, + ), ); await provider.connect(entityProviderConnection); @@ -630,11 +651,12 @@ describe('GitlabOrgDiscoveryEntityProvider', () => { 'backstage.io/managed-by-origin-location': 'url:https://gitlab.com/testuser1', 'gitlab.com/user-login': 'https://gitlab.com/testuser1', + 'gitlab.com/saml-external-uid': '51', }, name: 'testuser1', }, spec: { - memberOf: ['group2', 'group3'], + memberOf: ['group2'], profile: { displayName: 'Test User 1', email: 'testuser1@example.com', @@ -655,11 +677,12 @@ describe('GitlabOrgDiscoveryEntityProvider', () => { 'backstage.io/managed-by-origin-location': 'url:https://gitlab.com/testuser2', 'gitlab.com/user-login': 'https://gitlab.com/testuser2', + 'gitlab.com/saml-external-uid': '52', }, name: 'testuser2', }, spec: { - memberOf: ['group2', 'group3'], + memberOf: ['group3'], profile: { displayName: 'Test User 2', email: 'testuser2@example.com', diff --git a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts index b98b8ab7c3..b73874c401 100644 --- a/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts +++ b/plugins/catalog-backend-module-gitlab/src/providers/GitlabOrgDiscoveryEntityProvider.ts @@ -331,6 +331,10 @@ export class GitlabOrgDiscoveryEntityProvider implements EntityProvider { const annotations: { [annotationName: string]: string } = {}; annotations[`${host}/user-login`] = user.web_url; + if (user?.group_saml_identity?.extern_uid) { + annotations[`${host}/saml-external-uid`] = + user.group_saml_identity.extern_uid; + } const entity: UserEntity = { apiVersion: 'backstage.io/v1alpha1', From 07b05a7dc903e8cb9f97e4d719949818762e8c70 Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Wed, 4 Oct 2023 09:56:48 +0200 Subject: [PATCH 5/5] quality of changeset Signed-off-by: Matteo Silvestri --- .changeset/flat-rules-rush.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/flat-rules-rush.md b/.changeset/flat-rules-rush.md index c68a4e5756..a95772bb6f 100644 --- a/.changeset/flat-rules-rush.md +++ b/.changeset/flat-rules-rush.md @@ -9,9 +9,9 @@ allows to filter out bot users not using a license using the `is_using_seat` field. We also added the annotation `gitlab.com/saml-external-uid` taking the value -of `group_saml_identity.extern_uid` of the forementioned endpoint response. -This is useful in case you want to backreference the user with the id of your -identity provider (e.g. OneLogin). +of `group_saml_identity.extern_uid` of the `groups/:group-id/members` endpoint +response. This is useful in case you want to create a `SignInResolver` that +references the user with the id of your identity provider (e.g. OneLogin). ref: