Merge branch 'security-11-7-test-permissions' into 'security-11-7'

[11.7] Pipelines section is available to unauthorized users

See merge request gitlab/gitlabhq!2804

(cherry picked from commit 2bf899ed3a5306bb934507dc0584fd3d26f490bc)

627c00da Backport security fix
4c369519 Add CHANGELOG entry

15 files changed, 223 insertions, 53 deletions

diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index 80506249ea9..fa732437fc1 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -3,9 +3,14 @@ require 'spec_helper'
 describe Projects::PipelineSchedulesController do
   include AccessMatchersForController
 
+  set(:user) { create(:user) }
   set(:project) { create(:project, :public, :repository) }
   set(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
 
+  before do
+    project.add_developer(user)
+  end
+
   describe 'GET #index' do
     render_views
 
@@ -14,6 +19,10 @@ describe Projects::PipelineSchedulesController do
       create(:ci_pipeline_schedule, :inactive, project: project)
     end
 
+    before do
+      sign_in(user)
+    end
+
     it 'renders the index view' do
       visit_pipelines_schedules
 
@@ -21,7 +30,7 @@ describe Projects::PipelineSchedulesController do
       expect(response).to render_template(:index)
     end
 
-    it 'avoids N + 1 queries' do
+    it 'avoids N + 1 queries', :request_store do
       control_count = ActiveRecord::QueryRecorder.new { visit_pipelines_schedules }.count
 
       create_list(:ci_pipeline_schedule, 2, project: project)
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index 0bb3ef76a3b..740b28f0f46 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::PipelinesController do
 
   set(:user) { create(:user) }
   let(:project) { create(:project, :public, :repository) }
-  let(:feature) { ProjectFeature::DISABLED }
+  let(:feature) { ProjectFeature::ENABLED }
 
   before do
     stub_not_protect_default_branch
@@ -186,6 +186,27 @@ describe Projects::PipelinesController do
       end
     end
 
+    context 'when builds are disabled' do
+      let(:feature) { ProjectFeature::DISABLED }
+
+      it 'users can not see internal pipelines' do
+        get_pipeline_json
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+
+      context 'when pipeline is external' do
+        let(:pipeline) { create(:ci_pipeline, source: :external, project: project) }
+
+        it 'users can see the external pipeline' do
+          get_pipeline_json
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['id']).to be(pipeline.id)
+        end
+      end
+    end
+
     def get_pipeline_json
       get :show, params: { namespace_id: project.namespace, project_id: project, id: pipeline }, format: :json
     end
@@ -326,16 +347,14 @@ describe Projects::PipelinesController do
                    format: :json
     end
 
-    context 'when builds are enabled' do
-      let(:feature) { ProjectFeature::ENABLED }
-
-      it 'retries a pipeline without returning any content' do
-        expect(response).to have_gitlab_http_status(:no_content)
-        expect(build.reload).to be_retried
-      end
+    it 'retries a pipeline without returning any content' do
+      expect(response).to have_gitlab_http_status(:no_content)
+      expect(build.reload).to be_retried
     end
 
     context 'when builds are disabled' do
+      let(:feature) { ProjectFeature::DISABLED }
+
       it 'fails to retry pipeline' do
         expect(response).to have_gitlab_http_status(:not_found)
       end
@@ -355,16 +374,14 @@ describe Projects::PipelinesController do
                     format: :json
     end
 
-    context 'when builds are enabled' do
-      let(:feature) { ProjectFeature::ENABLED }
-
-      it 'cancels a pipeline without returning any content' do
-        expect(response).to have_gitlab_http_status(:no_content)
-        expect(pipeline.reload).to be_canceled
-      end
+    it 'cancels a pipeline without returning any content' do
+      expect(response).to have_gitlab_http_status(:no_content)
+      expect(pipeline.reload).to be_canceled
     end
 
     context 'when builds are disabled' do
+      let(:feature) { ProjectFeature::DISABLED }
+
       it 'fails to retry pipeline' do
         expect(response).to have_gitlab_http_status(:not_found)
       end
diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb
index 001e6c10eb2..9ee87563ecf 100644
--- a/spec/features/security/project/internal_access_spec.rb
+++ b/spec/features/security/project/internal_access_spec.rb
@@ -452,9 +452,9 @@ describe "Internal Project Access"  do
     it { is_expected.to be_allowed_for(:owner).of(project) }
     it { is_expected.to be_allowed_for(:maintainer).of(project) }
     it { is_expected.to be_allowed_for(:developer).of(project) }
-    it { is_expected.to be_allowed_for(:reporter).of(project) }
-    it { is_expected.to be_allowed_for(:guest).of(project) }
-    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_denied_for(:reporter).of(project) }
+    it { is_expected.to be_denied_for(:guest).of(project) }
+    it { is_expected.to be_denied_for(:user) }
     it { is_expected.to be_denied_for(:external) }
     it { is_expected.to be_denied_for(:visitor) }
   end
diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb
index c6618355eea..12613c39307 100644
--- a/spec/features/security/project/private_access_spec.rb
+++ b/spec/features/security/project/private_access_spec.rb
@@ -485,7 +485,7 @@ describe "Private Project Access"  do
     it { is_expected.to be_allowed_for(:owner).of(project) }
     it { is_expected.to be_allowed_for(:maintainer).of(project) }
     it { is_expected.to be_allowed_for(:developer).of(project) }
-    it { is_expected.to be_allowed_for(:reporter).of(project) }
+    it { is_expected.to be_denied_for(:reporter).of(project) }
     it { is_expected.to be_denied_for(:guest).of(project) }
     it { is_expected.to be_denied_for(:user) }
     it { is_expected.to be_denied_for(:external) }
diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb
index 3717dc13f1e..57cc0db1f38 100644
--- a/spec/features/security/project/public_access_spec.rb
+++ b/spec/features/security/project/public_access_spec.rb
@@ -272,11 +272,11 @@ describe "Public Project Access"  do
     it { is_expected.to be_allowed_for(:owner).of(project) }
     it { is_expected.to be_allowed_for(:maintainer).of(project) }
     it { is_expected.to be_allowed_for(:developer).of(project) }
-    it { is_expected.to be_allowed_for(:reporter).of(project) }
-    it { is_expected.to be_allowed_for(:guest).of(project) }
-    it { is_expected.to be_allowed_for(:user) }
-    it { is_expected.to be_allowed_for(:external) }
-    it { is_expected.to be_allowed_for(:visitor) }
+    it { is_expected.to be_denied_for(:reporter).of(project) }
+    it { is_expected.to be_denied_for(:guest).of(project) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe "GET /:project_path/environments" do
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 88b5d87f087..646d10d0d10 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -354,8 +354,20 @@ describe ProjectsHelper do
         allow(project).to receive(:builds_enabled?).and_return(false)
       end
 
-      it "do not include pipelines tab" do
-        is_expected.not_to include(:pipelines)
+      context 'when user has access to builds' do
+        it "does include pipelines tab" do
+          is_expected.to include(:pipelines)
+        end
+      end
+
+      context 'when user does not have access to builds' do
+        before do
+          allow(helper).to receive(:can?) { false }
+        end
+
+        it "does not include pipelines tab" do
+          is_expected.not_to include(:pipelines)
+        end
       end
     end
   end
diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
index 242c16c4bdc..6e68db2884f 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -12,7 +12,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
       ]
 
       RSpec::Mocks.with_temporary_scope do
-        @project = create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project')
+        @project = create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project')
         @shared = @project.import_export_shared
         allow(@shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/')
 
@@ -40,7 +40,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         project = Project.find_by_path('project')
 
         expect(project.project_feature.issues_access_level).to eq(ProjectFeature::DISABLED)
-        expect(project.project_feature.builds_access_level).to eq(ProjectFeature::DISABLED)
+        expect(project.project_feature.builds_access_level).to eq(ProjectFeature::ENABLED)
         expect(project.project_feature.snippets_access_level).to eq(ProjectFeature::ENABLED)
         expect(project.project_feature.wiki_access_level).to eq(ProjectFeature::ENABLED)
         expect(project.project_feature.merge_requests_access_level).to eq(ProjectFeature::ENABLED)
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index a2d2d77746d..baad8352185 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -11,6 +11,7 @@ describe Commit do
     it { is_expected.to include_module(Participable) }
     it { is_expected.to include_module(Referable) }
     it { is_expected.to include_module(StaticModel) }
+    it { is_expected.to include_module(Presentable) }
   end
 
   describe '.lazy' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index ae1269bde83..fcae39b7bdd 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -400,6 +400,30 @@ describe Project do
     end
   end
 
+  describe '#all_pipelines' do
+    let(:project) { create(:project) }
+
+    before do
+      create(:ci_pipeline, project: project, ref: 'master', source: :web)
+      create(:ci_pipeline, project: project, ref: 'master', source: :external)
+    end
+
+    it 'has all pipelines' do
+      expect(project.all_pipelines.size).to eq(2)
+    end
+
+    context 'when builds are disabled' do
+      before do
+        project.project_feature.update_attribute(:builds_access_level, ProjectFeature::DISABLED)
+      end
+
+      it 'should return .external pipelines' do
+        expect(project.all_pipelines).to all(have_attributes(source: 'external'))
+        expect(project.all_pipelines.size).to eq(1)
+      end
+    end
+  end
+
   describe 'project token' do
     it 'sets an random token if none provided' do
       project = FactoryBot.create(:project, runners_token: '')
diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb
index 8022f61e67d..844d96017de 100644
--- a/spec/policies/ci/pipeline_policy_spec.rb
+++ b/spec/policies/ci/pipeline_policy_spec.rb
@@ -75,6 +75,14 @@ describe Ci::PipelinePolicy, :models do
       end
     end
 
+    context 'when user does not have access to internal CI' do
+      let(:project) { create(:project, :builds_disabled, :public) }
+
+      it 'disallows the user from reading the pipeline' do
+        expect(policy).to be_disallowed :read_pipeline
+      end
+    end
+
     describe 'destroy_pipeline' do
       let(:project) { create(:project, :public) }
 
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 2a4030de998..ef1e56845d8 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -153,21 +153,41 @@ describe ProjectPolicy do
   end
 
   context 'builds feature' do
-    subject { described_class.new(owner, project) }
+    context 'when builds are disabled' do
+      subject { described_class.new(owner, project) }
 
-    it 'disallows all permissions when the feature is disabled' do
-      project.project_feature.update(builds_access_level: ProjectFeature::DISABLED)
+      before do
+        project.project_feature.update(builds_access_level: ProjectFeature::DISABLED)
+      end
 
-      builds_permissions = [
-        :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline,
-        :create_build, :read_build, :update_build, :admin_build, :destroy_build,
-        :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
-        :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
-        :create_cluster, :read_cluster, :update_cluster, :admin_cluster,
-        :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment
-      ]
+      it 'disallows all permissions except pipeline when the feature is disabled' do
+        builds_permissions = [
+          :create_build, :read_build, :update_build, :admin_build, :destroy_build,
+          :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
+          :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
+          :create_cluster, :read_cluster, :update_cluster, :admin_cluster, :destroy_cluster,
+          :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment
+        ]
+
+        expect_disallowed(*builds_permissions)
+      end
+    end
+
+    context 'when builds are disabled only for some users' do
+      subject { described_class.new(guest, project) }
 
-      expect_disallowed(*builds_permissions)
+      before do
+        project.project_feature.update(builds_access_level: ProjectFeature::PRIVATE)
+      end
+
+      it 'disallows pipeline and commit_status permissions' do
+        builds_permissions = [
+          :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline,
+          :create_commit_status, :update_commit_status, :admin_commit_status, :destroy_commit_status
+        ]
+
+        expect_disallowed(*builds_permissions)
+      end
     end
   end
 
diff --git a/spec/presenters/commit_presenter_spec.rb b/spec/presenters/commit_presenter_spec.rb
new file mode 100644
index 00000000000..4a0d3a28c32
--- /dev/null
+++ b/spec/presenters/commit_presenter_spec.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe CommitPresenter do
+  let(:project) { create(:project, :repository) }
+  let(:commit) { project.commit }
+  let(:user) { create(:user) }
+  let(:presenter) { described_class.new(commit, current_user: user) }
+
+  describe '#status_for' do
+    subject { presenter.status_for('ref') }
+
+    context 'when user can read_commit_status' do
+      before do
+        allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(true)
+      end
+
+      it 'returns commit status for ref' do
+        expect(commit).to receive(:status).with('ref').and_return('test')
+
+        expect(subject).to eq('test')
+      end
+    end
+
+    context 'when user can not read_commit_status' do
+      it 'is false' do
+        is_expected.to eq(false)
+      end
+    end
+  end
+
+  describe '#any_pipelines?' do
+    subject { presenter.any_pipelines? }
+
+    context 'when user can read pipeline' do
+      before do
+        allow(presenter).to receive(:can?).with(user, :read_pipeline, project).and_return(true)
+      end
+
+      it 'returns if there are any pipelines for commit' do
+        expect(commit).to receive_message_chain(:pipelines, :any?).and_return(true)
+
+        expect(subject).to eq(true)
+      end
+    end
+
+    context 'when user can not read pipeline' do
+      it 'is false' do
+        is_expected.to eq(false)
+      end
+    end
+  end
+end
diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index 561421d5ac8..376698a16df 100644
--- a/spec/serializers/merge_request_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -31,23 +31,40 @@ describe MergeRequestWidgetEntity do
   describe 'pipeline' do
     let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
 
-    context 'when is up to date' do
-      let(:req) { double('request', current_user: user, project: project) }
+    before do
+      allow_any_instance_of(MergeRequestPresenter).to receive(:can?).and_call_original
+      allow_any_instance_of(MergeRequestPresenter).to receive(:can?).with(user, :read_pipeline, anything).and_return(result)
+    end
 
-      it 'returns pipeline' do
-        pipeline_payload = PipelineDetailsEntity
-          .represent(pipeline, request: req)
-          .as_json
+    context 'when user has access to pipelines' do
+      let(:result) { true }
+
+      context 'when is up to date' do
+        let(:req) { double('request', current_user: user, project: project) }
+
+        it 'returns pipeline' do
+          pipeline_payload = PipelineDetailsEntity
+            .represent(pipeline, request: req)
+            .as_json
+
+          expect(subject[:pipeline]).to eq(pipeline_payload)
+        end
+      end
+
+      context 'when is not up to date' do
+        it 'returns nil' do
+          pipeline.update(sha: "not up to date")
 
-        expect(subject[:pipeline]).to eq(pipeline_payload)
+          expect(subject[:pipeline]).to eq(nil)
+        end
       end
     end
 
-    context 'when is not up to date' do
-      it 'returns nil' do
-        pipeline.update(sha: "not up to date")
+    context 'when user does not have access to pipelines' do
+      let(:result) { false }
 
-        expect(subject[:pipeline]).to be_nil
+      it 'does not have pipeline' do
+        expect(subject[:pipeline]).to eq(nil)
       end
     end
   end
diff --git a/spec/views/projects/commit/_commit_box.html.haml_spec.rb b/spec/views/projects/commit/_commit_box.html.haml_spec.rb
index 2fdd28a3be4..1086546c10d 100644
--- a/spec/views/projects/commit/_commit_box.html.haml_spec.rb
+++ b/spec/views/projects/commit/_commit_box.html.haml_spec.rb
@@ -9,6 +9,7 @@ describe 'projects/commit/_commit_box.html.haml' do
     assign(:commit, project.commit)
     allow(view).to receive(:current_user).and_return(user)
     allow(view).to receive(:can_collaborate_with_project?).and_return(false)
+    project.add_developer(user)
   end
 
   it 'shows the commit SHA' do
@@ -48,7 +49,6 @@ describe 'projects/commit/_commit_box.html.haml' do
   context 'viewing a commit' do
     context 'as a developer' do
       before do
-        project.add_developer(user)
         allow(view).to receive(:can_collaborate_with_project?).and_return(true)
       end
 
@@ -60,6 +60,10 @@ describe 'projects/commit/_commit_box.html.haml' do
     end
 
     context 'as a non-developer' do
+      before do
+        project.add_guest(user)
+      end
+
       it 'does not have a link to create a new tag' do
         render
 
diff --git a/spec/views/projects/issues/_related_branches.html.haml_spec.rb b/spec/views/projects/issues/_related_branches.html.haml_spec.rb
index 8c845251765..5cff7694029 100644
--- a/spec/views/projects/issues/_related_branches.html.haml_spec.rb
+++ b/spec/views/projects/issues/_related_branches.html.haml_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
 describe 'projects/issues/_related_branches' do
   include Devise::Test::ControllerHelpers
 
+  let(:user) { create(:user) }
   let(:project) { create(:project, :repository) }
   let(:branch) { project.repository.find_branch('feature') }
   let!(:pipeline) { create(:ci_pipeline, project: project, sha: branch.dereferenced_target.id, ref: 'feature') }
@@ -11,6 +12,9 @@ describe 'projects/issues/_related_branches' do
     assign(:project, project)
     assign(:related_branches, ['feature'])
 
+    project.add_developer(user)
+    allow(view).to receive(:current_user).and_return(user)
+
     render
   end