Merge branch 'security-check-mr-permissions-for-pipeline-widget' into 'master'

Check merge requests read permissions before showing them in the pipeline widget

Closes #60

See merge request gitlab-org/security/gitlab!207

app/presenters/ci/pipeline_presenter.rb

@@ -134,7 +134,11 @@ module Ci
 
     def all_related_merge_requests
       strong_memoize(:all_related_merge_requests) do
-        pipeline.ref ? pipeline.all_merge_requests_by_recency.to_a : []
+        if pipeline.ref && can?(current_user, :read_merge_request, pipeline.project)
+          pipeline.all_merge_requests_by_recency.to_a
+        else
+          []
+        end
       end
     end
   end

changelogs/unreleased/security-check-mr-permissions-for-pipeline-widget.yml

+---
+title: Check merge requests read permissions before showing them in the pipeline widget
+merge_request:
+author:
+type: security

spec/presenters/ci/pipeline_presenter_spec.rb

@@ -6,6 +6,7 @@ describe Ci::PipelinePresenter do
   include Gitlab::Routing
 
   let(:user) { create(:user) }
+  let(:current_user) { user }
   let(:project) { create(:project) }
   let(:pipeline) { create(:ci_pipeline, project: project) }
 
@@ -15,7 +16,7 @@ describe Ci::PipelinePresenter do
 
   before do
     project.add_developer(user)
-    allow(presenter).to receive(:current_user) { user }
+    allow(presenter).to receive(:current_user) { current_user }
   end
 
   it 'inherits from Gitlab::View::Presenter::Delegated' do
@@ -224,10 +225,90 @@ describe Ci::PipelinePresenter do
   describe '#all_related_merge_requests' do
     it 'memoizes the returned relation' do
       query_count = ActiveRecord::QueryRecorder.new do
-        2.times { presenter.send(:all_related_merge_requests).count }
+        3.times { presenter.send(:all_related_merge_requests).count }
       end.count
 
-      expect(query_count).to eq(1)
+      expect(query_count).to eq(2)
+    end
+
+    context 'permissions' do
+      let!(:merge_request) do
+        create(:merge_request, project: project, source_project: project)
+      end
+
+      subject(:all_related_merge_requests) do
+        presenter.send(:all_related_merge_requests)
+      end
+
+      shared_examples 'private merge requests' do
+        context 'when not logged in' do
+          let(:current_user) {}
+
+          it { is_expected.to be_empty }
+        end
+
+        context 'when logged in as a non_member' do
+          let(:current_user) { create(:user) }
+
+          it { is_expected.to be_empty }
+        end
+
+        context 'when logged in as a guest' do
+          let(:current_user) { create(:user) }
+
+          before do
+            project.add_guest(current_user)
+          end
+
+          it { is_expected.to be_empty }
+        end
+
+        context 'when logged in as a developer' do
+          it { is_expected.to contain_exactly(merge_request) }
+        end
+
+        context 'when logged in as a maintainer' do
+          let(:current_user) { create(:user) }
+
+          before do
+            project.add_maintainer(current_user)
+          end
+
+          it { is_expected.to contain_exactly(merge_request) }
+        end
+      end
+
+      context 'with a private project' do
+        it_behaves_like 'private merge requests'
+      end
+
+      context 'with a public project with private merge requests' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+
+          project
+            .project_feature
+            .update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+        end
+
+        it_behaves_like 'private merge requests'
+      end
+
+      context 'with a public project with public merge requests' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+
+          project
+            .project_feature
+            .update!(merge_requests_access_level: ProjectFeature::ENABLED)
+        end
+
+        context 'when not logged in' do
+          let(:current_user) {}
+
+          it { is_expected.to contain_exactly(merge_request) }
+        end
+      end
     end
   end