diff options
| -rw-r--r-- | app/controllers/concerns/milestone_actions.rb | 2 | ||||
| -rw-r--r-- | app/models/concerns/milestoneish.rb | 11 | ||||
| -rw-r--r-- | changelogs/unreleased/security-2797-milestone-mrs.yml | 5 | ||||
| -rw-r--r-- | spec/models/concerns/milestoneish_spec.rb | 47 |
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 |