Merge branch 'test-permissions' into 'master'

[master] Pipelines section is available to unauthorized users

See merge request gitlab/gitlabhq!2480

35 files changed, 324 insertions, 95 deletions

diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb
index 368ee89ff5c..54ff7ded8e5 100644
--- a/app/controllers/projects/merge_requests/application_controller.rb
+++ b/app/controllers/projects/merge_requests/application_controller.rb
@@ -39,8 +39,11 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
   end
 
   def set_pipeline_variables
-    @pipelines = @merge_request.all_pipelines
-    @pipeline = @merge_request.head_pipeline
-    @statuses_count = @pipeline.present? ? @pipeline.statuses.relevant.count : 0
+    @pipelines =
+      if can?(current_user, :read_pipeline, @project)
+        @merge_request.all_pipelines
+      else
+        Ci::Pipeline.none
+      end
   end
 end
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 67827b1d3bb..df43d9994a1 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -4,6 +4,7 @@ class Projects::PipelinesController < Projects::ApplicationController
   before_action :whitelist_query_limiting, only: [:create, :retry]
   before_action :pipeline, except: [:index, :new, :create, :charts]
   before_action :authorize_read_pipeline!
+  before_action :authorize_read_build!, only: [:index]
   before_action :authorize_create_pipeline!, only: [:new, :create]
   before_action :authorize_update_pipeline!, only: [:retry, :cancel]
 
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index b0d3d509f5d..85248a16f50 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -305,7 +305,8 @@ module ProjectsHelper
       nav_tabs << :container_registry
     end
 
-    if project.builds_enabled? && can?(current_user, :read_pipeline, project)
+    # Pipelines feature is tied to presence of builds
+    if can?(current_user, :read_build, project)
       nav_tabs << :pipelines
     end
 
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 01f4c58daa1..982e13e2845 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -11,6 +11,7 @@ class Commit
   include Mentionable
   include Referable
   include StaticModel
+  include Presentable
   include ::Gitlab::Utils::StrongMemoize
 
   attr_mentionable :safe_message, pipeline: :single_line
@@ -304,7 +305,9 @@ class Commit
   end
 
   def last_pipeline
-    @last_pipeline ||= pipelines.last
+    strong_memoize(:last_pipeline) do
+      pipelines.last
+    end
   end
 
   def status(ref = nil)
diff --git a/app/models/project.rb b/app/models/project.rb
index 1023b40a608..cecfee82334 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -578,6 +578,14 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def all_pipelines
+    if builds_enabled?
+      super
+    else
+      super.external
+    end
+  end
+
   # returns all ancestor-groups upto but excluding the given namespace
   # when no namespace is given, all ancestors upto the top are returned
   def ancestors_upto(top = nil, hierarchy_order: nil)
diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb
index e42d78f47c5..2c90b8a73cd 100644
--- a/app/policies/ci/pipeline_policy.rb
+++ b/app/policies/ci/pipeline_policy.rb
@@ -10,6 +10,15 @@ module Ci
       @subject.project.branch_allows_collaboration?(@user, @subject.ref)
     end
 
+    condition(:external_pipeline, scope: :subject, score: 0) do
+      @subject.external?
+    end
+
+    # Disallow users without permissions from accessing internal pipelines
+    rule { ~can?(:read_build) & ~external_pipeline }.policy do
+      prevent :read_pipeline
+    end
+
     rule { protected_ref }.prevent :update_pipeline
 
     rule { can?(:public_access) & branch_allows_collaboration }.policy do
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 95ae85ed60c..cadbc5ae009 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -108,6 +108,10 @@ class ProjectPolicy < BasePolicy
   condition(:has_clusters, scope: :subject) { clusterable_has_clusters? }
   condition(:can_have_multiple_clusters) { multiple_clusters_available? }
 
+  condition(:internal_builds_disabled) do
+    !@subject.builds_enabled?
+  end
+
   features = %w[
     merge_requests
     issues
@@ -196,7 +200,6 @@ class ProjectPolicy < BasePolicy
     enable :read_build
     enable :read_container_image
     enable :read_pipeline
-    enable :read_pipeline_schedule
     enable :read_environment
     enable :read_deployment
     enable :read_merge_request
@@ -235,6 +238,7 @@ class ProjectPolicy < BasePolicy
     enable :update_build
     enable :create_pipeline
     enable :update_pipeline
+    enable :read_pipeline_schedule
     enable :create_pipeline_schedule
     enable :create_merge_request_from
     enable :create_wiki
@@ -320,7 +324,6 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { builds_disabled | repository_disabled }.policy do
-    prevent(*create_update_admin_destroy(:pipeline))
     prevent(*create_read_update_admin_destroy(:build))
     prevent(*create_read_update_admin_destroy(:pipeline_schedule))
     prevent(*create_read_update_admin_destroy(:environment))
@@ -328,11 +331,22 @@ class ProjectPolicy < BasePolicy
     prevent(*create_read_update_admin_destroy(:deployment))
   end
 
+  # There's two separate cases when builds_disabled is true:
+  # 1. When internal CI is disabled - builds_disabled && internal_builds_disabled
+  #   - We do not prevent the user from accessing Pipelines to allow him to access external CI
+  # 2. When the user is not allowed to access CI - builds_disabled && ~internal_builds_disabled
+  #   - We prevent the user from accessing Pipelines
+  rule { (builds_disabled & ~internal_builds_disabled) | repository_disabled }.policy do
+    prevent(*create_read_update_admin_destroy(:pipeline))
+    prevent(*create_read_update_admin_destroy(:commit_status))
+  end
+
   rule { repository_disabled }.policy do
     prevent :push_code
     prevent :download_code
     prevent :fork_project
     prevent :read_commit_status
+    prevent :read_pipeline
     prevent(*create_read_update_admin_destroy(:release))
   end
 
@@ -359,7 +373,6 @@ class ProjectPolicy < BasePolicy
     enable :read_merge_request
     enable :read_note
     enable :read_pipeline
-    enable :read_pipeline_schedule
     enable :read_commit_status
     enable :read_container_image
     enable :download_code
@@ -378,7 +391,6 @@ class ProjectPolicy < BasePolicy
 
   rule { public_builds & can?(:guest_access) }.policy do
     enable :read_pipeline
-    enable :read_pipeline_schedule
   end
 
   # These rules are included to allow maintainers of projects to push to certain
diff --git a/app/presenters/commit_presenter.rb b/app/presenters/commit_presenter.rb
new file mode 100644
index 00000000000..05adbe1d4f5
--- /dev/null
+++ b/app/presenters/commit_presenter.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class CommitPresenter < Gitlab::View::Presenter::Simple
+  presents :commit
+
+  def status_for(ref)
+    can?(current_user, :read_commit_status, commit.project) && commit.status(ref)
+  end
+
+  def any_pipelines?
+    can?(current_user, :read_pipeline, commit.project) && commit.pipelines.any?
+  end
+end
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 44b6ca299ae..c59e73f824c 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -170,6 +170,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
     source_branch_exists? && merge_request.can_remove_source_branch?(current_user)
   end
 
+  def can_read_pipeline?
+    pipeline && can?(current_user, :read_pipeline, pipeline)
+  end
+
   def mergeable_discussions_state
     # This avoids calling MergeRequest#mergeable_discussions_state without
     # considering the state of the MR first. If a MR isn't mergeable, we can
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index 9361c9f987b..f42abf06e1e 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -57,7 +57,7 @@ class MergeRequestWidgetEntity < IssuableEntity
   end
 
   expose :merge_commit_message
-  expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline
+  expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline, if: -> (mr, _) { presenter(mr).can_read_pipeline? }
   expose :merge_pipeline, with: PipelineDetailsEntity, if: ->(mr, _) { mr.merged? && can?(request.current_user, :read_pipeline, mr.target_project)}
 
   # Booleans
diff --git a/app/views/projects/commit/_ci_menu.html.haml b/app/views/projects/commit/_ci_menu.html.haml
index f6666921a25..8b6e3e42ea1 100644
--- a/app/views/projects/commit/_ci_menu.html.haml
+++ b/app/views/projects/commit/_ci_menu.html.haml
@@ -1,9 +1,11 @@
+- any_pipelines = @commit.present(current_user: current_user).any_pipelines?
+
 %ul.nav-links.no-top.no-bottom.commit-ci-menu.nav.nav-tabs
   = nav_link(path: 'commit#show') do
     = link_to project_commit_path(@project, @commit.id) do
       Changes
       %span.badge.badge-pill= @diffs.size
-  - if can?(current_user, :read_pipeline, @project)
+  - if any_pipelines
     = nav_link(path: 'commit#pipelines') do
       = link_to pipelines_project_commit_path(@project, @commit.id) do
         Pipelines
diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml
index 2a919a767c0..3971ca473e0 100644
--- a/app/views/projects/commit/_commit_box.html.haml
+++ b/app/views/projects/commit/_commit_box.html.haml
@@ -74,8 +74,8 @@
     %span.commit-info.merge-requests{ 'data-project-commit-path' => merge_requests_project_commit_path(@project, @commit.id, format: :json) }
       = icon('spinner spin')
 
-  - if @commit.last_pipeline
-    - last_pipeline = @commit.last_pipeline
+  - last_pipeline = @commit.last_pipeline
+  - if can?(current_user, :read_pipeline, last_pipeline)
     .well-segment.pipeline-info
       .status-icon-container
         = link_to project_pipeline_path(@project, last_pipeline.id), class: "ci-status-icon-#{last_pipeline.status}" do
diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml
index 79e32949db9..06f0cd9675e 100644
--- a/app/views/projects/commit/show.html.haml
+++ b/app/views/projects/commit/show.html.haml
@@ -9,10 +9,7 @@
 
 .container-fluid{ class: [limited_container_width, container_class] }
   = render "commit_box"
-  - if @commit.status
-    = render "ci_menu"
-  - else
-    .block-connector
+  = render "ci_menu"
   = render "projects/diffs/diffs", diffs: @diffs, environment: @environment, is_commit: true
 
   .limited-width-notes
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index 1a74b120c26..0d3c6e7027c 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -6,6 +6,7 @@
 - merge_request = local_assigns.fetch(:merge_request, nil)
 - project       = local_assigns.fetch(:project) { merge_request&.project }
 - ref           = local_assigns.fetch(:ref) { merge_request&.source_branch }
+- commit_status = commit.present(current_user: current_user).status_for(ref)
 
 - link = commit_path(project, commit, merge_request: merge_request)
 %li.commit.flex-row.js-toggle-container{ id: "commit-#{commit.short_id}" }
@@ -22,7 +23,7 @@
       %span.commit-row-message.d-block.d-sm-none
         &middot;
         = commit.short_id
-      - if commit.status(ref)
+      - if commit_status
         .d-block.d-sm-none
           = render_commit_status(commit, ref: ref)
       - if commit.description?
@@ -45,7 +46,7 @@
       - else
         = render partial: 'projects/commit/ajax_signature', locals: { commit: commit }
 
-      - if commit.status(ref)
+      - if commit_status
         = render_commit_status(commit, ref: ref)
 
       .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } }
diff --git a/app/views/projects/issues/_merge_requests.html.haml b/app/views/projects/issues/_merge_requests.html.haml
index eb46bf5c6a3..6f652391be3 100644
--- a/app/views/projects/issues/_merge_requests.html.haml
+++ b/app/views/projects/issues/_merge_requests.html.haml
@@ -12,6 +12,7 @@
     %ul.content-list.related-items-list
       - has_any_head_pipeline = @merge_requests.any?(&:head_pipeline_id)
       - @merge_requests.each do |merge_request|
+        - merge_request = merge_request.present(current_user: current_user)
         %li.list-item.py-0.px-0
           .item-body.issuable-info-container.py-lg-3.px-lg-3.pl-md-3
             .item-contents
@@ -25,7 +26,7 @@
                     = merge_request.target_project.full_path
                   = merge_request.to_reference
                 %span.mr-ci-status.flex-md-grow-1.justify-content-end.d-flex.ml-md-2
-                  - if merge_request.head_pipeline
+                  - if merge_request.can_read_pipeline?
                     = render_pipeline_status(merge_request.head_pipeline, tooltip_placement: 'bottom')
                   - elsif has_any_head_pipeline
                     = icon('blank fw')
diff --git a/app/views/projects/issues/_related_branches.html.haml b/app/views/projects/issues/_related_branches.html.haml
index 1df38db9fd4..ffdd96870ef 100644
--- a/app/views/projects/issues/_related_branches.html.haml
+++ b/app/views/projects/issues/_related_branches.html.haml
@@ -6,7 +6,7 @@
       %li
         - target = @project.repository.find_branch(branch).dereferenced_target
         - pipeline = @project.pipeline_for(branch, target.sha) if target
-        - if pipeline
+        - if can?(current_user, :read_pipeline, pipeline)
           %span.related-branch-ci-status
             = render_pipeline_status(pipeline)
         %span.related-branch-info
diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml
index faa070d0389..d7994909366 100644
--- a/app/views/projects/merge_requests/_merge_request.html.haml
+++ b/app/views/projects/merge_requests/_merge_request.html.haml
@@ -46,7 +46,7 @@
           %li.issuable-status.d-none.d-sm-inline-block
             = icon('ban')
             CLOSED
-        - if merge_request.head_pipeline
+        - if can?(current_user, :read_pipeline, merge_request.head_pipeline)
           %li.issuable-pipeline-status.d-none.d-sm-inline-block
             = render_pipeline_status(merge_request.head_pipeline)
         - if merge_request.open? && merge_request.broken?
diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml
index 0f0114d513c..69a47faabed 100644
--- a/app/views/projects/pipelines/_info.html.haml
+++ b/app/views/projects/pipelines/_info.html.haml
@@ -6,23 +6,22 @@
       = preserve(markdown(commit.description, pipeline: :single_line))
 
 .info-well
-  - if commit.status
-    .well-segment.pipeline-info
-      .icon-container
-        = icon('clock-o')
-      = pluralize @pipeline.total_size, "job"
-      - if @pipeline.ref
-        from
-        - if @pipeline.ref_exists?
-          = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name"
-        - else
-          %span.ref-name
-            = @pipeline.ref
-      - if @pipeline.duration
-        in
-        = time_interval_in_words(@pipeline.duration)
-      - if @pipeline.queued_duration
-        = "(queued for #{time_interval_in_words(@pipeline.queued_duration)})"
+  .well-segment.pipeline-info
+    .icon-container
+      = icon('clock-o')
+    = pluralize @pipeline.total_size, "job"
+    - if @pipeline.ref
+      from
+      - if @pipeline.ref_exists?
+        = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name"
+      - else
+        %span.ref-name
+          = @pipeline.ref
+    - if @pipeline.duration
+      in
+      = time_interval_in_words(@pipeline.duration)
+    - if @pipeline.queued_duration
+      = "(queued for #{time_interval_in_words(@pipeline.queued_duration)})"
 
   .well-segment
     .icon-container
diff --git a/changelogs/unreleased/test-permissions.yml b/changelogs/unreleased/test-permissions.yml
new file mode 100644
index 00000000000..cfb69fdcb1e
--- /dev/null
+++ b/changelogs/unreleased/test-permissions.yml
@@ -0,0 +1,5 @@
+---
+title: Disallows unauthorized users from accessing the pipelines section.
+merge_request:
+author:
+type: security
diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb
index 7a7b23d2bbb..0317d69edde 100644
--- a/lib/api/pipelines.rb
+++ b/lib/api/pipelines.rb
@@ -76,7 +76,7 @@ module API
         requires :pipeline_id, type: Integer, desc: 'The pipeline ID'
       end
       get ':id/pipelines/:pipeline_id' do
-        authorize! :read_pipeline, user_project
+        authorize! :read_pipeline, pipeline
 
         present pipeline, with: Entities::Pipeline
       end
@@ -104,7 +104,7 @@ module API
         requires :pipeline_id, type: Integer,  desc: 'The pipeline ID'
       end
       post ':id/pipelines/:pipeline_id/retry' do
-        authorize! :update_pipeline, user_project
+        authorize! :update_pipeline, pipeline
 
         pipeline.retry_failed(current_user)
 
@@ -119,7 +119,7 @@ module API
         requires :pipeline_id, type: Integer,  desc: 'The pipeline ID'
       end
       post ':id/pipelines/:pipeline_id/cancel' do
-        authorize! :update_pipeline, user_project
+        authorize! :update_pipeline, pipeline
 
         pipeline.cancel_running
 
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 c2dd666f9df..10f61731206 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
 
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 9b0da882390..6084dc96410 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 585dfe46189..8a45f9856bc 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -405,6 +405,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 6c854bab5a5..49226a01846 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -175,21 +175,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