Merge branch '209022-include-email-address-in-group-members-api' into 'master'

Expose user email to group managed account owners

Closes #209022

See merge request gitlab-org/gitlab!30584

doc/api/members.md

@@ -59,6 +59,7 @@ Example response:
     "web_url": "http://192.168.1.8:3000/root",
     "expires_at": "2012-10-22T14:13:35Z",
     "access_level": 30,
+    "email": "john@example.com",
     "group_saml_identity": {
       "extern_uid":"ABC-1234567890",
       "provider": "group_saml",
@@ -116,6 +117,7 @@ Example response:
     "web_url": "http://192.168.1.8:3000/root",
     "expires_at": "2012-10-22T14:13:35Z",
     "access_level": 30
+    "email": "john@example.com",
     "group_saml_identity": {
       "extern_uid":"ABC-1234567890",
       "provider": "group_saml",

ee/app/models/ee/user.rb

@@ -300,6 +300,10 @@ module EE
       managing_group.present?
     end
 
+    def managed_by?(user)
+      self.group_managed_account? && self.managing_group.owned_by?(user)
+    end
+
     override :ldap_sync_time
     def ldap_sync_time
       ::Gitlab.config.ldap['sync_time']

ee/changelogs/unreleased/209022-include-email-address-in-group-members-api.yml

+---
+title: REST API membership responses for group owner enqueries include group managed
+  account emails
+merge_request: 30584
+author:
+type: added

ee/lib/ee/api/entities/user_basic.rb

@@ -8,6 +8,7 @@ module EE
 
         prepended do
           expose :gitlab_employee?, as: :is_gitlab_employee, if: proc { ::Gitlab.com? && ::Feature.enabled?(:gitlab_employee_badge) }
+          expose :email, if: -> (user, options) { user.managed_by?(options[:current_user]) }
         end
       end
     end

ee/spec/lib/ee/api/entities/member_spec.rb

@@ -31,4 +31,24 @@ describe API::Entities::Member do
       expect(entity_representation.keys).not_to include(:group_saml_identity)
     end
   end
+
+  context 'when current user is allowed to manage user' do
+    before do
+      allow(member.user).to receive(:managed_by?).and_return(true)
+    end
+
+    it 'exposes email' do
+      expect(entity_representation.keys).to include(:email)
+    end
+  end
+
+  context 'when current user is not allowed to manage user' do
+    before do
+      allow(member.user).to receive(:managed_by?).and_return(false)
+    end
+
+    it 'does not expose email' do
+      expect(entity_representation.keys).not_to include(:email)
+    end
+  end
 end

ee/spec/models/user_spec.rb

@@ -567,6 +567,42 @@ describe User do
     end
   end
 
+  describe '#managed_by?' do
+    let(:group) { create :group }
+    let(:owner) { create :user }
+    let(:member1) { create :user }
+    let(:member2) { create :user }
+
+    before do
+      group.add_owner(owner)
+      group.add_developer(member1)
+      group.add_developer(member2)
+    end
+
+    context 'when a normal user account' do
+      it 'returns false' do
+        expect(member1.managed_by?(owner)).to be_falsey
+        expect(member1.managed_by?(member2)).to be_falsey
+      end
+    end
+
+    context 'when a group managed account' do
+      let(:group) { create :group_with_managed_accounts }
+
+      before do
+        member1.update(managing_group: group)
+      end
+
+      it 'returns true with group managed account owner' do
+        expect(member1.managed_by?(owner)).to be_truthy
+      end
+
+      it 'returns false with a regular user account' do
+        expect(member1.managed_by?(member2)).to be_falsey
+      end
+    end
+  end
+
   describe '#password_required?' do
     context 'when user has managing group linked' do
       before do

ee/spec/requests/api/members_spec.rb

@@ -3,6 +3,221 @@
 require 'spec_helper'
 
 describe API::Members do
+  context 'group members endpoint for group managed accounts' do
+    let(:group) { create(:group) }
+    let(:owner) { create(:user) }
+
+    before do
+      group.add_owner(owner)
+    end
+
+    include_context "group managed account with group members"
+
+    subject do
+      get api(url, owner)
+      json_response
+    end
+
+    describe "GET /groups/:id/members" do
+      let(:url) { "/groups/#{group.id}/members" }
+
+      it_behaves_like 'members response with exposed emails' do
+        let(:emails) { gma_member.email }
+      end
+
+      it_behaves_like 'members response with hidden emails' do
+        let(:emails) { member.email }
+      end
+    end
+
+    describe "GET /groups/:id/members/:user_id" do
+      let(:url) { "/groups/#{group.id}/members/#{user_id}" }
+
+      context 'with group managed account member' do
+        let(:user_id) { gma_member.id }
+
+        it_behaves_like 'member response with exposed email' do
+          let(:email) { gma_member.email }
+        end
+      end
+
+      context 'with a regular member' do
+        let(:user_id) { member.id }
+
+        it_behaves_like 'member response with hidden email'
+      end
+    end
+
+    describe "GET /groups/:id/members/all" do
+      include_context "child group with group managed account members"
+
+      context 'parent group' do
+        let(:url) { "/groups/#{group.id}/members/all" }
+
+        it_behaves_like 'members response with exposed emails' do
+          let(:emails) { gma_member.email }
+        end
+
+        it_behaves_like 'members response with hidden emails' do
+          let(:emails) { member.email }
+        end
+      end
+
+      context 'child group' do
+        let(:url) { "/groups/#{child_group.id}/members/all" }
+
+        it_behaves_like 'members response with exposed emails' do
+          let(:emails) { [gma_member.email, child_gma_member.email] }
+        end
+
+        it_behaves_like 'members response with hidden emails' do
+          let(:emails) { [member.email, child_member.email] }
+        end
+      end
+    end
+
+    describe "GET /groups/:id/members/all/:user_id" do
+      include_context "child group with group managed account members"
+
+      let(:url) { "/groups/#{child_group.id}/members/all/#{user_id}" }
+
+      context 'with group managed account member' do
+        let(:user_id) { gma_member.id }
+
+        it_behaves_like 'member response with exposed email' do
+          let(:email) { gma_member.email }
+        end
+      end
+
+      context 'with regular member' do
+        let(:user_id) { member.id }
+
+        it_behaves_like 'member response with hidden email'
+      end
+
+      context 'with group managed account child group member' do
+        let(:user_id) { child_gma_member.id }
+
+        it_behaves_like 'member response with exposed email' do
+          let(:email) { child_gma_member.email }
+        end
+      end
+
+      context 'with child group regular member' do
+        let(:user_id) { child_member.id }
+
+        it_behaves_like 'member response with hidden email'
+      end
+    end
+  end
+
+  context 'project members endpoint for group managed accounts' do
+    let(:group) { create(:group) }
+    let(:owner) { create(:user) }
+    let(:project) { create(:project, group: group) }
+
+    before do
+      group.add_owner(owner)
+    end
+
+    include_context "group managed account with project members"
+
+    subject do
+      get api(url, owner)
+      json_response
+    end
+
+    describe "GET /projects/:id/members" do
+      let(:url) { "/projects/#{project.id}/members" }
+
+      it_behaves_like 'members response with exposed emails' do
+        let(:emails) { gma_member.email }
+      end
+
+      it_behaves_like 'members response with hidden emails' do
+        let(:emails) { member.email }
+      end
+    end
+
+    describe "GET /projects/:id/members/:user_id" do
+      let(:url) { "/projects/#{project.id}/members/#{user_id}" }
+
+      context 'with group managed account member' do
+        let(:user_id) { gma_member.id }
+
+        it_behaves_like 'member response with exposed email' do
+          let(:email) { gma_member.email }
+        end
+      end
+
+      context 'with a regular member' do
+        let(:user_id) { member.id }
+
+        it_behaves_like 'member response with hidden email'
+      end
+    end
+
+    describe "GET /project/:id/members/all" do
+      include_context "child project with group managed account members"
+
+      context 'parent group project' do
+        let(:url) { "/projects/#{project.id}/members/all" }
+
+        it_behaves_like 'members response with exposed emails' do
+          let(:emails) { gma_member.email }
+        end
+
+        it_behaves_like 'members response with hidden emails' do
+          let(:emails) { member.email }
+        end
+      end
+
+      context 'child group project' do
+        let(:url) { "/projects/#{child_project.id}/members/all" }
+
+        it_behaves_like 'members response with exposed emails' do
+          let(:emails) { [child_gma_member.email] }
+        end
+
+        it_behaves_like 'members response with hidden emails' do
+          let(:emails) { [member.email, child_member.email] }
+        end
+      end
+    end
+
+    describe "GET /projects/:id/members/all/:user_id" do
+      include_context "child project with group managed account members"
+
+      let(:url) { "/projects/#{child_project.id}/members/all/#{user_id}" }
+
+      context 'with group managed account member' do
+        let(:user_id) { gma_member.id }
+
+        it_behaves_like 'member response with hidden email'
+      end
+
+      context 'with regular member' do
+        let(:user_id) { member.id }
+
+        it_behaves_like 'member response with hidden email'
+      end
+
+      context 'with group managed account child group member' do
+        let(:user_id) { child_gma_member.id }
+
+        it_behaves_like 'member response with exposed email' do
+          let(:email) { child_gma_member.email }
+        end
+      end
+
+      context 'with child group regular member' do
+        let(:user_id) { child_member.id }
+
+        it_behaves_like 'member response with hidden email'
+      end
+    end
+  end
+
   context 'without LDAP' do
     let(:group) { create(:group) }
     let(:owner) { create(:user) }

ee/spec/support/shared_contexts/requests/api/members_shared_contexts.rb

+# frozen_string_literal: true
+
+RSpec.shared_context 'group managed account with group members' do
+  let(:group) { create :group_with_managed_accounts }
+  let(:member) { create :user, :group_managed }
+  let(:gma_member) { create :user, :group_managed, managing_group: group }
+
+  before do
+    stub_licensed_features(group_saml: true)
+
+    group.add_maintainer(member)
+    group.add_maintainer(gma_member)
+  end
+end
+
+RSpec.shared_context 'group managed account with project members' do
+  let(:group) { create :group_with_managed_accounts }
+  let(:member) { create :user, :group_managed }
+  let(:gma_member) { create :user, managing_group: group }
+
+  before do
+    stub_licensed_features(group_saml: true)
+
+    project.add_maintainer(member)
+    project.add_maintainer(gma_member)
+  end
+end
+
+RSpec.shared_context 'child group with group managed account members' do
+  let(:child_group) { create :group, parent: group }
+  let(:child_member) { create :user, :group_managed }
+  let(:child_gma_member) { create :user, :group_managed, managing_group: group }
+
+  before do
+    child_group.add_owner(owner)
+
+    child_group.add_developer(child_member)
+    child_group.add_developer(child_gma_member)
+  end
+end
+
+RSpec.shared_context 'child project with group managed account members' do
+  let(:child_group) { create :group, parent: group }
+  let(:child_project) { create :project, group: child_group }
+  let(:child_member) { create :user, :group_managed }
+  let(:child_gma_member) { create :user, :group_managed, managing_group: group }
+
+  before do
+    child_group.add_owner(owner)
+
+    child_project.add_developer(child_member)
+    child_project.add_developer(child_gma_member)
+  end
+end

ee/spec/support/shared_examples/requests/api/members_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'members response with exposed emails' do
+  it do
+    Array(emails).flatten.each do |email|
+      is_expected.to include(a_hash_including('email' => email))
+    end
+  end
+end
+
+RSpec.shared_examples 'members response with hidden emails' do
+  it do
+    Array(emails).flatten.each do |email|
+      is_expected.not_to include(a_hash_including('email' => email))
+    end
+  end
+end
+
+RSpec.shared_examples 'member response with exposed email' do
+  it { is_expected.to include('email' => email) }
+end
+
+RSpec.shared_examples 'member response with hidden email' do
+  it { is_expected.not_to have_key('email') }
+end