Merge branch '11-6-security-2797-milestone-mrs' into '11-6-stable'

Show only MRs visible to user on milestone detail

See merge request gitlab/gitlabhq!2925

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?

app/models/global_milestone.rb

@@ -9,6 +9,8 @@ class GlobalMilestone
   attr_accessor :title, :milestones
   alias_attribute :name, :title
+  delegate :milestoneish_id, to: :milestone
+
   def for_display
     @first_milestone
   end
@@ -48,6 +50,10 @@ class GlobalMilestone
     @first_milestone = milestones.find {|m| m.description.present? } || milestones.first
   end
+  def milestone
+    milestones.first
+  end
+
   def milestoneish_ids
     milestones.select(:id)
   end

app/models/group_milestone.rb

@@ -22,4 +22,8 @@ class GroupMilestone < GlobalMilestone
   def legacy_group_milestone?
     true
   end
+
+  def milestone
+    @milestone ||= milestones.find { |m| m.description.present? } || milestones.first
+  end
 end

app/models/milestone.rb

@@ -73,6 +73,7 @@ class Milestone < ActiveRecord::Base
   end
   alias_attribute :name, :title
+  alias_attribute :milestoneish_id, :milestoneish_ids
   class << self
     # Searches for milestones matching the given query.

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

+---
+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