Merge branch 'security-2797-milestone-mrs' into 'master'

Show only MRs visible to user on milestone detail

See merge request gitlab/gitlabhq!2895

app/controllers/concerns/milestone_actions.rb

@@ -8,7 +8,7 @@ module MilestoneActions
       format.html { redirect_to milestone_redirect_path }
       format.json do
         render json: tabs_json("shared/milestones/_merge_requests_tab", {
-          merge_requests: @milestone.sorted_merge_requests, # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          merge_requests: @milestone.sorted_merge_requests(current_user), # rubocop:disable Gitlab/ModuleWithInstanceVariables
           show_project_name: true
         })
       end

app/models/concerns/milestoneish.rb

@@ -46,12 +46,19 @@ module Milestoneish
     end
   end
 
+  def merge_requests_visible_to_user(user)
+    memoize_per_user(user, :merge_requests_visible_to_user) do
+      MergeRequestsFinder.new(user, {})
+        .execute.where(milestone_id: milestoneish_id)
+    end
+  end
+
   def sorted_issues(user)
     issues_visible_to_user(user).preload_associations.sort_by_attribute('label_priority')
   end
 
-  def sorted_merge_requests
-    merge_requests.sort_by_attribute('label_priority')
+  def sorted_merge_requests(user)
+    merge_requests_visible_to_user(user).sort_by_attribute('label_priority')
   end
 
   def upcoming?

changelogs/unreleased/security-2797-milestone-mrs.yml

@@ -0,0 +1,5 @@
+---
+title: Show only merge requests visible to user on milestone detail page
+merge_request:
+author:
+type: security

spec/models/concerns/milestoneish_spec.rb

@@ -48,7 +48,7 @@ describe Milestone, 'Milestoneish' do
       merge_request_2 = create(:labeled_merge_request, labels: [label_1], source_project: project, source_branch: 'branch_2', milestone: milestone)
       merge_request_3 = create(:labeled_merge_request, labels: [label_3], source_project: project, source_branch: 'branch_3', milestone: milestone)
 
-      merge_requests = milestone.sorted_merge_requests
+      merge_requests = milestone.sorted_merge_requests(member)
 
       expect(merge_requests.first).to eq(merge_request_2)
       expect(merge_requests.second).to eq(merge_request_1)
@@ -56,6 +56,51 @@ describe Milestone, 'Milestoneish' do
     end
   end
 
+  describe '#merge_requests_visible_to_user' do
+    let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
+
+    context 'when project is private' do
+      before do
+        project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+      end
+
+      it 'does not return any merge request for a non member' do
+        merge_requests = milestone.merge_requests_visible_to_user(non_member)
+        expect(merge_requests).to be_empty
+      end
+
+      it 'returns milestone merge requests for a member' do
+        merge_requests = milestone.merge_requests_visible_to_user(member)
+        expect(merge_requests).to contain_exactly(merge_request)
+      end
+    end
+
+    context 'when project is public' do
+      context 'when merge requests are available to anyone' do
+        it 'returns milestone merge requests for a non member' do
+          merge_requests = milestone.merge_requests_visible_to_user(non_member)
+          expect(merge_requests).to contain_exactly(merge_request)
+        end
+      end
+
+      context 'when merge requests are available to project members' do
+        before do
+          project.project_feature.update(merge_requests_access_level: ProjectFeature::PRIVATE)
+        end
+
+        it 'does not return any merge request for a non member' do
+          merge_requests = milestone.merge_requests_visible_to_user(non_member)
+          expect(merge_requests).to be_empty
+        end
+
+        it 'returns milestone merge requests for a member' do
+          merge_requests = milestone.merge_requests_visible_to_user(member)
+          expect(merge_requests).to contain_exactly(merge_request)
+        end
+      end
+    end
+  end
+
   describe '#closed_items_count' do
     it 'does not count confidential issues for non project members' do
       expect(milestone.closed_items_count(non_member)).to eq 2