Merge branch 'fix-issue-17496' into 'master'

Fix groups API to list only user's accessible projects

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/17496

See merge request !1966

CHANGELOG

@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.9.0 (unreleased)
   - Redesign navigation for project pages
+  - Fix groups API to list only user's accessible projects
   - Use gitlab-shell v3.0.0
   - Add rake task 'gitlab:db:configure' for conditionally seeding or migrating the database
   - Changed the Slack build message to use the singular duration if necessary (Aran Koning)

app/finders/group_projects_finder.rb

@@ -18,7 +18,7 @@ class GroupProjectsFinder < UnionFinder
     projects = []
 
     if current_user
-      if @group.users.include?(current_user)
+      if @group.users.include?(current_user) || current_user.admin?
         projects << @group.projects unless only_shared
         projects << @group.shared_projects unless only_owned
       else

lib/api/groups.rb

@@ -95,8 +95,7 @@ module API
       #   GET /groups/:id/projects
       get ":id/projects" do
         group = find_group(params[:id])
-        projects = group.projects
-        projects = filter_projects(projects)
+        projects = GroupProjectsFinder.new(group).execute(current_user)
         projects = paginate projects
         present projects, with: Entities::Project
       end

spec/requests/api/groups_spec.rb

@@ -12,6 +12,7 @@ describe API::API, api: true  do
   let!(:group2) { create(:group, :private) }
   let!(:project1) { create(:project, namespace: group1) }
   let!(:project2) { create(:project, namespace: group2) }
+  let!(:project3) { create(:project, namespace: group1, path: 'test', visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
 
   before do
     group1.add_owner(user1)
@@ -147,9 +148,11 @@ describe API::API, api: true  do
     context "when authenticated as user" do
       it "should return the group's projects" do
         get api("/groups/#{group1.id}/projects", user1)
+
         expect(response.status).to eq(200)
-        expect(json_response.length).to eq(1)
-        expect(json_response.first['name']).to eq(project1.name)
+        expect(json_response.length).to eq(2)
+        project_names = json_response.map { |proj| proj['name' ] }
+        expect(project_names).to match_array([project1.name, project3.name])
       end
 
       it "should not return a non existing group" do
@@ -162,6 +165,16 @@ describe API::API, api: true  do
 
         expect(response.status).to eq(404)
       end
+
+      it "should only return projects to which user has access" do
+        project3.team << [user3, :developer]
+
+        get api("/groups/#{group1.id}/projects", user3)
+
+        expect(response.status).to eq(200)
+        expect(json_response.length).to eq(1)
+        expect(json_response.first['name']).to eq(project3.name)
+      end
     end
 
     context "when authenticated as admin" do
@@ -181,8 +194,10 @@ describe API::API, api: true  do
     context 'when using group path in URL' do
       it 'should return any existing group' do
         get api("/groups/#{group1.path}/projects", admin)
+
         expect(response.status).to eq(200)
-        expect(json_response.first['name']).to eq(project1.name)
+        project_names = json_response.map { |proj| proj['name' ] }
+        expect(project_names).to match_array([project1.name, project3.name])
       end
 
       it 'should not return a non existing group' do