diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb index eccbe35577b444c4c18d275e8dcb81d067aedb86..c0c0160a827a4964bd8419d49970051d4c25dcde 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 e44a069b7302cccef6dc666816d07c6f701fba96..65e28d15b06033998d6340ce66a65933d9f86557 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/app/models/global_milestone.rb b/app/models/global_milestone.rb index 085ffd16c6ab2f0b0ead3ba4ed1f920cb0961e91..03653aac3b81ebf3136ba84cc17b21ad7ad2c53a 100644 --- a/app/models/global_milestone.rb +++ b/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 diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb index 9dfaebacc83b18a5866af0cec41d1af9de77b984..5874a745d708b5d4676a6a15cda7087044bb13d3 100644 --- a/app/models/group_milestone.rb +++ b/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 diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 3cc8e2c44bb6267240bd78eb6f6db98a5a0eeca9..bc39a4837538eab26441df95bee640b061ca943f 100644 --- a/app/models/milestone.rb +++ b/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. diff --git a/changelogs/unreleased/security-2797-milestone-mrs.yml b/changelogs/unreleased/security-2797-milestone-mrs.yml new file mode 100644 index 0000000000000000000000000000000000000000..5bb104ec403ae0741ce927000bc67cf60f54530e --- /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 87bf731340f58e652eaedc3824ccc99ddb138a36..4647eecbdef2c42ecfa4ce6656fbefc85674aa52 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