From 28401da2decd9488107757259a31acf1e71d32d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Eduardo=20Sanz=20Garc=C3=ADa?= <esanz-garcia@gitlab.com>
Date: Mon, 12 Feb 2024 18:48:01 +0100
Subject: [PATCH 1/2] Use group/project ID for invite user modal

We are replacing the query to invite users when the SAML is enabled. We
have decided to use a group query or project query depending of the
context. See
https://gitlab.com/gitlab-org/gitlab/-/issues/434464#note_1764747685.

These API queries are not yet in use because the feature flag,
`group_users_saml` is currently off in all environments. So it is safe
to deploy.
---
 app/assets/javascripts/api/user_api.js        | 11 +++--
 .../components/invite_members_modal.vue       |  3 +-
 .../components/members_token_select.vue       |  8 +++-
 spec/frontend/api/user_api_spec.js            | 15 +++++-
 .../components/members_token_select_spec.js   | 48 +++++++++++++++----
 5 files changed, 67 insertions(+), 18 deletions(-)

diff --git a/app/assets/javascripts/api/user_api.js b/app/assets/javascripts/api/user_api.js
index 302de9760800c1..0a333ed198f18e 100644
--- a/app/assets/javascripts/api/user_api.js
+++ b/app/assets/javascripts/api/user_api.js
@@ -4,7 +4,8 @@ import { buildApiUrl } from './api_utils';
 
 const USER_COUNTS_PATH = '/api/:version/user_counts';
 const USERS_PATH = '/api/:version/users.json';
-const USERS_SAML_PATH = '/api/:version/groups/:id/users.json';
+const USERS_SAML_GROUP_PATH = '/api/:version/groups/:id/users.json';
+const USERS_SAML_PROJECT_PATH = '/api/:version/projects/:id/users.json';
 const USER_PATH = '/api/:version/users/:id';
 const USER_STATUS_PATH = '/api/:version/users/:id/status';
 const USER_PROJECTS_PATH = '/api/:version/users/:id/projects';
@@ -31,11 +32,13 @@ export function getUsers(query, options) {
  * If the query string is less than 3 characters it returns an empty list.
  *
  * @param {string} query - query string to search
- * @param {string} groupId -- top-level group id
+ * @param {boolean} isProject
+ * @param {string} id -- group or project id
  * @param {object} options
  */
-export function getGroupUsers(query, groupId, options) {
-  const url = buildApiUrl(USERS_SAML_PATH).replace(':id', groupId);
+export function getGroupUsers(query, isProject, id, options) {
+  const path = isProject ? USERS_SAML_PROJECT_PATH : USERS_SAML_GROUP_PATH;
+  const url = buildApiUrl(path).replace(':id', id);
   return axios.get(url, {
     params: {
       search: query,
diff --git a/app/assets/javascripts/invite_members/components/invite_members_modal.vue b/app/assets/javascripts/invite_members/components/invite_members_modal.vue
index 5125f6f70ac46c..3a92c16b777f53 100644
--- a/app/assets/javascripts/invite_members/components/invite_members_modal.vue
+++ b/app/assets/javascripts/invite_members/components/invite_members_modal.vue
@@ -449,6 +449,7 @@ export default {
 
     <template #select="{ exceptionState, inputId }">
       <members-token-select
+        :id="id"
         v-model="newUsersToInvite"
         class="gl-mb-2"
         aria-labelledby="empty-invites-alert"
@@ -456,7 +457,7 @@ export default {
         :exception-state="exceptionState"
         :users-filter="usersFilter"
         :filter-id="filterId"
-        :group-id="id"
+        :is-project="isProject"
         :invalid-members="invalidMembers"
         @clear="clearValidation"
         @token-remove="removeToken"
diff --git a/app/assets/javascripts/invite_members/components/members_token_select.vue b/app/assets/javascripts/invite_members/components/members_token_select.vue
index 0f5b437bb569f8..337bd0427cb3e5 100644
--- a/app/assets/javascripts/invite_members/components/members_token_select.vue
+++ b/app/assets/javascripts/invite_members/components/members_token_select.vue
@@ -57,10 +57,14 @@ export default {
       required: false,
       default: '',
     },
-    groupId: {
+    id: {
       type: String,
       required: true,
     },
+    isProject: {
+      type: Boolean,
+      required: true,
+    },
   },
   data() {
     return {
@@ -140,7 +144,7 @@ export default {
     },
     retrieveUsersRequest() {
       if (this.usersFilter === USERS_FILTER_SAML_PROVIDER_ID && this.glFeatures.groupUserSaml) {
-        return getGroupUsers(this.query, this.groupId, this.queryOptions);
+        return getGroupUsers(this.query, this.isProject, this.id, this.queryOptions);
       }
 
       return getUsers(this.query, this.queryOptions);
diff --git a/spec/frontend/api/user_api_spec.js b/spec/frontend/api/user_api_spec.js
index aeddf6b9ae135f..3966390b06b022 100644
--- a/spec/frontend/api/user_api_spec.js
+++ b/spec/frontend/api/user_api_spec.js
@@ -52,11 +52,22 @@ describe('~/api/user_api', () => {
   });
 
   describe('getSAMLUsers', () => {
-    it('calls correct URL with expected query parameters', async () => {
+    it('calls the group URL with expected query parameters', async () => {
       const expectedUrl = '/api/v4/groups/34/users.json';
       axiosMock.onGet(expectedUrl).replyOnce(HTTP_STATUS_OK);
 
-      await getGroupUsers('den', '34', { include_service_accounts: true });
+      await getGroupUsers('den', false, '34', { include_service_accounts: true });
+
+      const { url, params } = axiosMock.history.get[0];
+      expect(url).toBe(expectedUrl);
+      expect(params).toMatchObject({ search: 'den', include_service_accounts: true });
+    });
+
+    it('calls the project URL with expected query parameters', async () => {
+      const expectedUrl = '/api/v4/projects/34/users.json';
+      axiosMock.onGet(expectedUrl).replyOnce(HTTP_STATUS_OK);
+
+      await getGroupUsers('den', true, '34', { include_service_accounts: true });
 
       const { url, params } = axiosMock.history.get[0];
       expect(url).toBe(expectedUrl);
diff --git a/spec/frontend/invite_members/components/members_token_select_spec.js b/spec/frontend/invite_members/components/members_token_select_spec.js
index 44f62d5904d2b9..e3492d041643e4 100644
--- a/spec/frontend/invite_members/components/members_token_select_spec.js
+++ b/spec/frontend/invite_members/components/members_token_select_spec.js
@@ -10,7 +10,7 @@ import * as Sentry from '~/sentry/sentry_browser_wrapper';
 
 const label = 'testgroup';
 const placeholder = 'Search for a member';
-const groupId = '31';
+const id = '31';
 const user1 = { id: 1, name: 'John Smith', username: 'one_1', avatar_url: '' };
 const user2 = { id: 2, name: 'Jane Doe', username: 'two_2', avatar_url: '' };
 const allUsers = [user1, user2];
@@ -22,7 +22,8 @@ const createComponent = (props = {}, glFeatures = {}) => {
       ariaLabelledby: label,
       invalidMembers: {},
       placeholder,
-      groupId,
+      isProject: false,
+      id,
       ...props,
     },
     provide: { glFeatures },
@@ -222,20 +223,49 @@ describe('MembersTokenSelect', () => {
 
   describe('when component is mounted for a group using a SAML provider', () => {
     const searchParam = 'name';
+    let isProject;
 
     beforeEach(() => {
       jest.spyOn(UserApi, 'getGroupUsers').mockResolvedValue({ data: allUsers });
+    });
+
+    describe('when used on projects', () => {
+      beforeEach(() => {
+        isProject = true;
+        wrapper = createComponent(
+          { usersFilter: 'saml_provider_id', isProject },
+          { groupUserSaml: true },
+        );
 
-      wrapper = createComponent({ usersFilter: 'saml_provider_id' }, { groupUserSaml: true });
+        findTokenSelector().vm.$emit('text-input', searchParam);
+      });
 
-      findTokenSelector().vm.$emit('text-input', searchParam);
+      it('calls the group API with correct parameters', () => {
+        expect(UserApi.getGroupUsers).toHaveBeenCalledWith(searchParam, isProject, id, {
+          active: true,
+          include_saml_users: true,
+          include_service_accounts: true,
+        });
+      });
     });
 
-    it('calls the group API with correct parameters', () => {
-      expect(UserApi.getGroupUsers).toHaveBeenCalledWith(searchParam, groupId, {
-        active: true,
-        include_saml_users: true,
-        include_service_accounts: true,
+    describe('when used on groups', () => {
+      beforeEach(() => {
+        isProject = false;
+        wrapper = createComponent(
+          { usersFilter: 'saml_provider_id', isProject },
+          { groupUserSaml: true },
+        );
+
+        findTokenSelector().vm.$emit('text-input', searchParam);
+      });
+
+      it('calls the group API with correct parameters', () => {
+        expect(UserApi.getGroupUsers).toHaveBeenCalledWith(searchParam, isProject, id, {
+          active: true,
+          include_saml_users: true,
+          include_service_accounts: true,
+        });
       });
     });
   });
-- 
GitLab


From dd0e686e197de74e5a9f5e144da3ae8256deb519 Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@gitlab.com>
Date: Wed, 14 Feb 2024 12:34:21 -0600
Subject: [PATCH 2/2] Add project's group users endpoint

---
 ee/app/policies/ee/project_policy.rb | 11 +++++++++
 ee/lib/ee/api/projects.rb            | 34 ++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb
index 98dc2ac6fa601c..f633fab3b5d9dd 100644
--- a/ee/app/policies/ee/project_policy.rb
+++ b/ee/app/policies/ee/project_policy.rb
@@ -683,6 +683,13 @@ module ProjectPolicy
         ::Gitlab::Auth::GroupSaml::SsoEnforcer.access_restricted?(user: @user, resource: subject)
       end
 
+      condition(:sso_enforced, scope: :subject) do
+        saml_provider = @subject.root_namespace.saml_provider
+        next false unless saml_provider
+
+        saml_provider.enabled? && saml_provider.enforced_sso?
+      end
+
       with_scope :subject
       condition(:ip_enforcement_prevents_access) do
         !::Gitlab::IpRestriction::Enforcer.new(subject.group).allows_current_ip? if subject.group
@@ -922,6 +929,10 @@ module ProjectPolicy
       end
 
       rule { membership_for_chat & chat_allowed_for_parent_group & chat_available_for_user }.enable :access_duo_chat
+
+      rule { can?(:admin_project_member) & sso_enforced }.policy do
+        enable :read_saml_user
+      end
     end
 
     override :lookup_access_level!
diff --git a/ee/lib/ee/api/projects.rb b/ee/lib/ee/api/projects.rb
index 244611a8fc2dc4..74782cbfa2b28c 100644
--- a/ee/lib/ee/api/projects.rb
+++ b/ee/lib/ee/api/projects.rb
@@ -21,6 +21,40 @@ module Projects
               render_api_error!(result[:message], 400)
             end
           end
+
+          desc "Get a list of users for the project's group" do
+            success code: 200, model: ::API::Entities::UserPublic
+            failure [
+              { code: 400, message: 'Bad request' },
+              { code: 403, message: 'Forbidden' },
+              { code: 404, message: '404 Not Found' }
+            ]
+          end
+          params do
+            optional :search, type: String, desc: 'Search users by name, email or username'
+            optional :active, type: ::Grape::API::Boolean, default: false, desc: 'Filters only active users'
+
+            optional :include_saml_users,
+              type: Grape::API::Boolean,
+              desc: 'Return users with a SAML identity in this group'
+            optional :include_service_accounts,
+              type: Grape::API::Boolean,
+              desc: 'Return service accounts owned by this group'
+            at_least_one_of :include_saml_users, :include_service_accounts
+
+            use :pagination
+          end
+          get ':id/users' do
+            authorize! :read_saml_user, user_project
+            params = declared_params(include_missing: false).except!(:id)
+            bad_request!(users_params_error) unless any_allowed_filters_present?(params)
+
+            finder = ::Groups::UsersFinder.new(current_user, user_project.group, params)
+            users = finder.execute.preload(:identities, :scim_identities) # rubocop: disable CodeReuse/ActiveRecord -- Simple preload of associations
+
+            present paginate(users), with: ::API::Entities::UserPublic
+          end
+
           segment ':id/audit_events', feature_category: :audit_events do
             before do
               authorize! :read_project_audit_events, user_project
-- 
GitLab