Show only MRs visible to user on milestone detail

4 files changed, 61 insertions, 4 deletions

diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb
index eccbe35577b..c0c0160a827 100644
--- a/app/controllers/concerns/milestone_actions.rb
+++ b/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
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 055ffe04646..39372c4f68b 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/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?
diff --git a/changelogs/unreleased/security-2797-milestone-mrs.yml b/changelogs/unreleased/security-2797-milestone-mrs.yml
new file mode 100644
index 00000000000..5bb104ec403
--- /dev/null
+++ b/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
diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb
index 87bf731340f..4647eecbdef 100644
--- a/spec/models/concerns/milestoneish_spec.rb
+++ b/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