Merge branch 'security-ag-hide-private-members-in-project-member-autocomplete' into 'master'

Hide private members in project member autocomplete

See merge request gitlab/gitlabhq!3212

2 files changed, 123 insertions, 16 deletions

diff --git a/spec/controllers/projects/autocomplete_sources_controller_spec.rb b/spec/controllers/projects/autocomplete_sources_controller_spec.rb
index 34765ae3951..fc8fe1ac4f6 100644
--- a/spec/controllers/projects/autocomplete_sources_controller_spec.rb
+++ b/spec/controllers/projects/autocomplete_sources_controller_spec.rb
@@ -8,6 +8,10 @@ describe Projects::AutocompleteSourcesController do
   let_it_be(:issue) { create(:issue, project: project) }
   let_it_be(:user) { create(:user) }
 
+  def members_by_username(username)
+    json_response.find { |member| member['username'] == username }
+  end
+
   describe 'GET members' do
     before do
       group.add_owner(user)
@@ -17,22 +21,21 @@ describe Projects::AutocompleteSourcesController do
     it 'returns an array of member object' do
       get :members, format: :json, params: { namespace_id: group.path, project_id: project.path, type: issue.class.name, type_id: issue.id }
 
-      all = json_response.find {|member| member["username"] == 'all'}
-      the_group = json_response.find {|member| member["username"] == group.full_path}
-      the_user = json_response.find {|member| member["username"] == user.username}
-
-      expect(all.symbolize_keys).to include(username: 'all',
-                                            name: 'All Project and Group Members',
-                                            count: 1)
-
-      expect(the_group.symbolize_keys).to include(type: group.class.name,
-                                                  name: group.full_name,
-                                                  avatar_url: group.avatar_url,
-                                                  count: 1)
-
-      expect(the_user.symbolize_keys).to include(type: user.class.name,
-                                                 name: user.name,
-                                                 avatar_url: user.avatar_url)
+      expect(members_by_username('all').symbolize_keys).to include(
+        username: 'all',
+        name: 'All Project and Group Members',
+        count: 1)
+
+      expect(members_by_username(group.full_path).symbolize_keys).to include(
+        type: group.class.name,
+        name: group.full_name,
+        avatar_url: group.avatar_url,
+        count: 1)
+
+      expect(members_by_username(user.username).symbolize_keys).to include(
+        type: user.class.name,
+        name: user.name,
+        avatar_url: user.avatar_url)
     end
   end
 
diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb
index 4def83513a4..239d28557ee 100644
--- a/spec/services/projects/participants_service_spec.rb
+++ b/spec/services/projects/participants_service_spec.rb
@@ -57,4 +57,108 @@ describe Projects::ParticipantsService do
       end
     end
   end
+
+  describe '#project_members' do
+    subject(:usernames) { service.project_members.map { |member| member[:username] } }
+
+    context 'when there is a project in group namespace' do
+      set(:public_group) { create(:group, :public) }
+      set(:public_project) { create(:project, :public, namespace: public_group)}
+
+      set(:public_group_owner) { create(:user) }
+
+      let(:service) { described_class.new(public_project, create(:user)) }
+
+      before do
+        public_group.add_owner(public_group_owner)
+      end
+
+      it 'returns members of a group' do
+        expect(usernames).to include(public_group_owner.username)
+      end
+    end
+
+    context 'when there is a private group and a public project' do
+      set(:public_group) { create(:group, :public) }
+      set(:private_group) { create(:group, :private, :nested) }
+      set(:public_project) { create(:project, :public, namespace: public_group)}
+
+      set(:project_issue) { create(:issue, project: public_project)}
+
+      set(:public_group_owner) { create(:user) }
+      set(:private_group_member) { create(:user) }
+      set(:public_project_maintainer) { create(:user) }
+      set(:private_group_owner) { create(:user) }
+
+      set(:group_ancestor_owner) { create(:user) }
+
+      before(:context) do
+        public_group.add_owner public_group_owner
+        private_group.add_developer private_group_member
+        public_project.add_maintainer public_project_maintainer
+
+        private_group.add_owner private_group_owner
+        private_group.parent.add_owner group_ancestor_owner
+      end
+
+      context 'when the private group is invited to the public project' do
+        before(:context) do
+          create(:project_group_link, group: private_group, project: public_project)
+        end
+
+        context 'when a user who is outside the public project and the private group is signed in' do
+          let(:service) { described_class.new(public_project, create(:user)) }
+
+          it 'does not return the private group' do
+            expect(usernames).not_to include(private_group.name)
+          end
+
+          it 'does not return private group members' do
+            expect(usernames).not_to include(private_group_member.username)
+          end
+
+          it 'returns the project maintainer' do
+            expect(usernames).to include(public_project_maintainer.username)
+          end
+
+          it 'returns project members from an invited public group' do
+            invited_public_group = create(:group, :public)
+            invited_public_group.add_owner create(:user)
+
+            create(:project_group_link, group: invited_public_group, project: public_project)
+
+            expect(usernames).to include(invited_public_group.users.first.username)
+          end
+
+          it 'does not return ancestors of the private group' do
+            expect(usernames).not_to include(group_ancestor_owner.username)
+          end
+        end
+
+        context 'when private group owner is signed in' do
+          let(:service) { described_class.new(public_project, private_group_owner) }
+
+          it 'returns private group members' do
+            expect(usernames).to include(private_group_member.username)
+          end
+
+          it 'returns ancestors of the the private group' do
+            expect(usernames).to include(group_ancestor_owner.username)
+          end
+        end
+
+        context 'when the namespace owner of the public project is signed in' do
+          let(:service) { described_class.new(public_project, public_group_owner) }
+
+          it 'returns private group members' do
+            expect(usernames).to include(private_group_member.username)
+          end
+
+          it 'does not return members of the ancestral groups of the private group' do
+            expect(usernames).to include(group_ancestor_owner.username)
+          end
+        end
+      end
+    end
+  end
 end