Merge branch 'rs-security-group-api' into 'security-10-2'

[10.2] Ensure we expose group projects using GroupProjectsFinder

See merge request gitlab/gitlabhq!2234

(cherry picked from commit 072f8f2fd6ec794645375a16ca4ddc1cbeb76d7a)

a2240338 Ensure we expose group projects using GroupProjectsFinder

3 files changed, 82 insertions, 2 deletions

diff --git a/changelogs/unreleased/rs-security-group-api.yml b/changelogs/unreleased/rs-security-group-api.yml
new file mode 100644
index 00000000000..34a39ddd6dc
--- /dev/null
+++ b/changelogs/unreleased/rs-security-group-api.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent an information disclosure in the Groups API
+merge_request:
+author:
+type: security
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index d96e7f2770f..928706dfda7 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -248,8 +248,21 @@ module API
     end
 
     class GroupDetail < Group
-      expose :projects, using: Entities::Project
-      expose :shared_projects, using: Entities::Project
+      expose :projects, using: Entities::Project do |group, options|
+        GroupProjectsFinder.new(
+          group: group,
+          current_user: options[:current_user],
+          options: { only_owned: true }
+        ).execute
+      end
+
+      expose :shared_projects, using: Entities::Project do |group, options|
+        GroupProjectsFinder.new(
+          group: group,
+          current_user: options[:current_user],
+          options: { only_shared: true }
+        ).execute
+      end
     end
 
     class Commit < Grape::Entity
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index 554723d6b1e..6330c140246 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -173,6 +173,28 @@ describe API::Groups do
   end
 
   describe "GET /groups/:id" do
+    # Given a group, create one project for each visibility level
+    #
+    # group      - Group to add projects to
+    # share_with - If provided, each project will be shared with this Group
+    #
+    # Returns a Hash of visibility_level => Project pairs
+    def add_projects_to_group(group, share_with: nil)
+      projects = {
+        public:   create(:project, :public,   namespace: group),
+        internal: create(:project, :internal, namespace: group),
+        private:  create(:project, :private,  namespace: group)
+      }
+
+      if share_with
+        create(:project_group_link, project: projects[:public],   group: share_with)
+        create(:project_group_link, project: projects[:internal], group: share_with)
+        create(:project_group_link, project: projects[:private],  group: share_with)
+      end
+
+      projects
+    end
+
     context 'when unauthenticated' do
       it 'returns 404 for a private group' do
         get api("/groups/#{group2.id}")
@@ -183,6 +205,26 @@ describe API::Groups do
         get api("/groups/#{group1.id}")
         expect(response).to have_gitlab_http_status(200)
       end
+
+      it 'returns only public projects in the group' do
+        public_group = create(:group, :public)
+        projects = add_projects_to_group(public_group)
+
+        get api("/groups/#{public_group.id}")
+
+        expect(json_response['projects'].map { |p| p['id'].to_i })
+          .to contain_exactly(projects[:public].id)
+      end
+
+      it 'returns only public projects shared with the group' do
+        public_group = create(:group, :public)
+        projects = add_projects_to_group(public_group, share_with: group1)
+
+        get api("/groups/#{group1.id}")
+
+        expect(json_response['shared_projects'].map { |p| p['id'].to_i })
+          .to contain_exactly(projects[:public].id)
+      end
     end
 
     context "when authenticated as user" do
@@ -222,6 +264,26 @@ describe API::Groups do
 
         expect(response).to have_gitlab_http_status(404)
       end
+
+      it 'returns only public and internal projects in the group' do
+        public_group = create(:group, :public)
+        projects = add_projects_to_group(public_group)
+
+        get api("/groups/#{public_group.id}", user2)
+
+        expect(json_response['projects'].map { |p| p['id'].to_i })
+          .to contain_exactly(projects[:public].id, projects[:internal].id)
+      end
+
+      it 'returns only public and internal projects shared with the group' do
+        public_group = create(:group, :public)
+        projects = add_projects_to_group(public_group, share_with: group1)
+
+        get api("/groups/#{group1.id}", user2)
+
+        expect(json_response['shared_projects'].map { |p| p['id'].to_i })
+          .to contain_exactly(projects[:public].id, projects[:internal].id)
+      end
     end
 
     context "when authenticated as admin" do