summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <kamil@gitlab.com>2019-01-28 12:12:30 +0000
committerYorick Peterse <yorickpeterse@gmail.com>2019-01-28 12:12:30 +0000
commita0383ab43ec2c885aae6602ffa47ffde79c76786 (patch)
tree79872a5735874f95154c39e4bb4c66d4bb51f0d0
parent35d4344edf5eec007d18acddbf40354646aa148e (diff)
downloadgitlab-ce-a0383ab43ec2c885aae6602ffa47ffde79c76786.tar.gz
[master] Pipelines section is available to unauthorized users
-rw-r--r--app/controllers/projects/merge_requests/application_controller.rb9
-rw-r--r--app/controllers/projects/pipelines_controller.rb1
-rw-r--r--app/helpers/projects_helper.rb3
-rw-r--r--app/models/commit.rb5
-rw-r--r--app/models/project.rb8
-rw-r--r--app/policies/ci/pipeline_policy.rb9
-rw-r--r--app/policies/project_policy.rb20
-rw-r--r--app/presenters/commit_presenter.rb13
-rw-r--r--app/presenters/merge_request_presenter.rb4
-rw-r--r--app/serializers/merge_request_widget_entity.rb2
-rw-r--r--app/views/projects/commit/_ci_menu.html.haml4
-rw-r--r--app/views/projects/commit/_commit_box.html.haml4
-rw-r--r--app/views/projects/commit/show.html.haml5
-rw-r--r--app/views/projects/commits/_commit.html.haml5
-rw-r--r--app/views/projects/issues/_merge_requests.html.haml3
-rw-r--r--app/views/projects/issues/_related_branches.html.haml2
-rw-r--r--app/views/projects/merge_requests/_merge_request.html.haml2
-rw-r--r--app/views/projects/pipelines/_info.html.haml33
-rw-r--r--changelogs/unreleased/test-permissions.yml5
-rw-r--r--lib/api/pipelines.rb6
-rw-r--r--spec/controllers/projects/pipeline_schedules_controller_spec.rb11
-rw-r--r--spec/controllers/projects/pipelines_controller_spec.rb47
-rw-r--r--spec/features/security/project/internal_access_spec.rb6
-rw-r--r--spec/features/security/project/private_access_spec.rb2
-rw-r--r--spec/features/security/project/public_access_spec.rb10
-rw-r--r--spec/helpers/projects_helper_spec.rb16
-rw-r--r--spec/lib/gitlab/import_export/project_tree_restorer_spec.rb4
-rw-r--r--spec/models/commit_spec.rb1
-rw-r--r--spec/models/project_spec.rb24
-rw-r--r--spec/policies/ci/pipeline_policy_spec.rb8
-rw-r--r--spec/policies/project_policy_spec.rb44
-rw-r--r--spec/presenters/commit_presenter_spec.rb54
-rw-r--r--spec/serializers/merge_request_widget_entity_spec.rb39
-rw-r--r--spec/views/projects/commit/_commit_box.html.haml_spec.rb6
-rw-r--r--spec/views/projects/issues/_related_branches.html.haml_spec.rb4
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