Merge branch '118475-add-members-api-group-saml-filtering' into 'master'

Add saml identity presence filtering option to members API

Closes #118475

See merge request gitlab-org/gitlab!21931

ee/app/models/ee/group_member.rb

@@ -15,6 +15,9 @@ module EE
       scope :with_identity_provider, ->(provider) do
         joins(user: :identities).where(identities: { provider: provider })
       end
+      scope :with_saml_identity, ->(provider) do
+        joins(user: :identities).where(identities: { saml_provider_id: provider })
+      end
 
       scope :non_owners, -> { where("members.access_level < ?", ::Gitlab::Access::OWNER) }
     end

ee/changelogs/unreleased/118475-add-members-api-group-saml-filtering.yml

+---
+title: Alow to filter by saml identity linked for group members API
+merge_request: 21931
+author:
+type: added

ee/lib/ee/api/helpers/members_helpers.rb

+# frozen_string_literal: true
+
+module EE
+  module API
+    module Helpers
+      module MembersHelpers
+        extend ActiveSupport::Concern
+        extend ::Gitlab::Utils::Override
+
+        prepended do
+          params :optional_filter_params_ee do
+            optional :with_saml_identity, type: Grape::API::Boolean, desc: "List only members with linked SAML identity"
+          end
+        end
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        override :retrieve_members
+        def retrieve_members(source, params:, deep: false)
+          members = super
+
+          if can_view_group_identity?(source)
+            members = members.includes(user: :group_saml_identities)
+            if params[:with_saml_identity] && source.saml_provider
+              members = members.with_saml_identity(source.saml_provider)
+            end
+          end
+
+          members
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
+        def can_view_group_identity?(members_source)
+          can?(current_user, :read_group_saml_identity, members_source)
+        end
+      end
+    end
+  end
+end

ee/lib/ee/api/members.rb

-# frozen_string_literal: true
-
-module EE
-  module API
-    module Members
-      extend ActiveSupport::Concern
-
-      prepended do
-        helpers do
-          # rubocop: disable CodeReuse/ActiveRecord
-          def retrieve_members(source, *args)
-            super.tap do |members|
-              members.includes(user: :group_saml_identities) if can_view_group_identity?(source)
-            end
-          end
-          # rubocop: enable CodeReuse/ActiveRecord
-
-          def can_view_group_identity?(members_source)
-            can?(current_user, :read_group_saml_identity, members_source)
-          end
-        end
-      end
-    end
-  end
-end

ee/spec/models/group_member_spec.rb

@@ -60,6 +60,28 @@ describe GroupMember do
     end
   end
 
+  describe '.with_saml_identity' do
+    let(:saml_provider) { create :saml_provider }
+    let(:group) { saml_provider.group }
+    let!(:member) do
+      create(:group_member, group: group).tap do |m|
+        create(:group_saml_identity, saml_provider: saml_provider, user: m.user)
+      end
+    end
+    let!(:member_without_identity) do
+      create(:group_member, group: group)
+    end
+    let!(:member_with_different_identity) do
+      create(:group_member, group: group).tap do |m|
+        create(:group_saml_identity, user: m.user)
+      end
+    end
+
+    it 'returns members with identity linked to given saml provider' do
+      expect(described_class.with_saml_identity(saml_provider)).to eq([member])
+    end
+  end
+
   describe '#group_saml_identity' do
     subject(:group_saml_identity) { member.group_saml_identity }
 

ee/spec/requests/api/members_spec.rb

@@ -29,9 +29,13 @@ describe API::Members do
 
   describe 'GET /groups/:id/members' do
     context 'when a group has SAML provider configured' do
+      let(:maintainer) { create(:user) }
+
       before do
         saml_provider = create :saml_provider, group: group
         create :group_saml_identity, user: owner, saml_provider: saml_provider
+
+        group.add_maintainer(maintainer)
       end
 
       context 'and current_user is group owner' do
@@ -39,23 +43,34 @@ describe API::Members do
           get api("/groups/#{group.to_param}/members", owner)
 
           expect(response).to have_gitlab_http_status(200)
+          expect(json_response.size).to eq(2)
           expect(json_response.first['group_saml_identity']).to match(kind_of(Hash))
         end
-      end
 
-      context 'and current_user is not an owner' do
-        let(:maintainer) do
-          create(:user).tap do |user|
-            group.add_maintainer(user)
-          end
+        it 'allows to filter by linked identity presence' do
+          get api("/groups/#{group.to_param}/members?with_saml_identity=true", owner)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response.size).to eq(1)
+          expect(json_response.any? { |member| member['id'] == maintainer.id }).to be_falsey
         end
+      end
 
-        it 'returns a list of users with group SAML identities info' do
+      context 'and current_user is not an owner' do
+        it 'returns a list of users without group SAML identities info' do
           get api("/groups/#{group.to_param}/members", maintainer)
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response.map(&:keys).flatten).not_to include('group_saml_identity')
         end
+
+        it 'ignores filter by linked identity presence' do
+          get api("/groups/#{group.to_param}/members?with_saml_identity=true", maintainer)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response.size).to eq(2)
+          expect(json_response.any? { |member| member['id'] == maintainer.id }).to be_truthy
+        end
       end
     end
   end

lib/api/helpers/members_helpers.rb

@@ -5,6 +5,11 @@
 module API
   module Helpers
     module MembersHelpers
+      extend Grape::API::Helpers
+
+      params :optional_filter_params_ee do
+      end
+
       def find_source(source_type, id)
         public_send("find_#{source_type}!", id) # rubocop:disable GitlabSecurity/PublicSend
       end
@@ -42,3 +47,5 @@ module API
     end
   end
 end
+
+API::Helpers::MembersHelpers.prepend_if_ee('EE::API::Helpers::MembersHelpers')

lib/api/members.rb

@@ -19,6 +19,7 @@ module API
         params do
           optional :query, type: String, desc: 'A query string to search for members'
           optional :user_ids, type: Array[Integer], desc: 'Array of user ids to look up for membership'
+          use :optional_filter_params_ee
           use :pagination
         end
 
@@ -157,5 +158,3 @@ module API
     end
   end
 end
-
-API::Members.prepend_if_ee('EE::API::Members')