diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 036ea45cc78a18a7b5180f431cff3497d099b0fd..cd2db2dad2c977fa94741b7fc43be878ae62ad09 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -18,7 +18,8 @@ class Projects::PipelinesController < Projects::ApplicationController before_action :authorize_read_build!, only: [:index, :show] before_action :authorize_read_ci_cd_analytics!, only: [:charts] before_action :authorize_create_pipeline!, only: [:new, :create] - before_action :authorize_update_pipeline!, only: [:retry, :cancel] + before_action :authorize_update_pipeline!, only: [:retry] + before_action :authorize_cancel_pipeline!, only: [:cancel] before_action :ensure_pipeline, only: [:show, :downloadable_artifacts] before_action :reject_if_build_artifacts_size_refreshing!, only: [:destroy] @@ -303,6 +304,10 @@ def authorize_update_pipeline! return access_denied! unless can?(current_user, :update_pipeline, @pipeline) end + def authorize_cancel_pipeline! + return access_denied! unless can?(current_user, :cancel_pipeline, @pipeline) + end + def limited_pipelines_count(project, scope = nil) finder = Ci::PipelinesFinder.new(project, current_user, index_params.merge(scope: scope)) diff --git a/app/graphql/mutations/ci/pipeline/cancel.rb b/app/graphql/mutations/ci/pipeline/cancel.rb index 810f458fd75c72c56ba107a58806d5d456bc3ee5..1014462d0b15f8104fd0d86b1535009e4203e872 100644 --- a/app/graphql/mutations/ci/pipeline/cancel.rb +++ b/app/graphql/mutations/ci/pipeline/cancel.rb @@ -6,7 +6,7 @@ module Pipeline class Cancel < Base graphql_name 'PipelineCancel' - authorize :update_pipeline + authorize :cancel_pipeline def resolve(id:) pipeline = authorized_find!(id: id) diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 1d60b1e79def8b2a36b7dcdadd1c38dbab87d4c6..bd6e580c33ae1bb7d08726217dbbfd03c8f2335b 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -45,6 +45,15 @@ class PipelinePolicy < BasePolicy enable :read_pipeline_variable end + # TODO: splitting out cancel from update in Issue #20207 + rule { can?(:update_pipeline) }.policy do + enable :cancel_pipeline + end + + rule { ~can?(:update_pipeline) }.policy do + prevent :cancel_pipeline + end + rule { project_allows_read_dependency }.policy do enable :read_dependency end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 20f88577d6722ce34463282700abfad751a91143..d11f7347636a98a31402096e658cb6d4c843af60 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -512,6 +512,7 @@ class ProjectPolicy < BasePolicy rule { can?(:developer_access) & user_confirmed? }.policy do enable :create_pipeline enable :update_pipeline + enable :cancel_pipeline enable :create_pipeline_schedule end @@ -652,6 +653,7 @@ class ProjectPolicy < BasePolicy # - 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 :cancel_pipeline prevent(*create_read_update_admin_destroy(:commit_status)) end diff --git a/app/serializers/ci/pipeline_entity.rb b/app/serializers/ci/pipeline_entity.rb index 5a124f099a7210a77136a96b70a81547c5842c7c..928a3b1b2be34d2e194a3963fd3a079ab1c3665a 100644 --- a/app/serializers/ci/pipeline_entity.rb +++ b/app/serializers/ci/pipeline_entity.rb @@ -100,7 +100,7 @@ def can_retry? end def can_cancel? - can?(request.current_user, :update_pipeline, pipeline) && + can?(request.current_user, :cancel_pipeline, pipeline) && pipeline.cancelable? end diff --git a/app/services/ci/cancel_pipeline_service.rb b/app/services/ci/cancel_pipeline_service.rb index b5c8c00273e2101a074c341143fc78fb939058ba..80b14274be297640a76c7724acb945cf870d2c91 100644 --- a/app/services/ci/cancel_pipeline_service.rb +++ b/app/services/ci/cancel_pipeline_service.rb @@ -24,7 +24,7 @@ def initialize( end def execute - unless can?(current_user, :update_pipeline, pipeline) + unless can?(current_user, :cancel_pipeline, pipeline) return ServiceResponse.error( message: 'Insufficient permissions to cancel the pipeline', reason: :insufficient_permissions) diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb index e74bf8f7efad97b0ce506a80874d360d8d9afb59..7475cda5cf9c9bc23fa8318ac352f1de5e44d630 100644 --- a/spec/policies/ci/pipeline_policy_spec.rb +++ b/spec/policies/ci/pipeline_policy_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::PipelinePolicy, :models do +RSpec.describe Ci::PipelinePolicy, :models, feature_category: :continuous_integration do let(:user) { create(:user) } let(:pipeline) { create(:ci_empty_pipeline, project: project) } @@ -25,6 +25,7 @@ it 'does not include ability to update pipeline' do expect(policy).to be_disallowed :update_pipeline + expect(policy).to be_disallowed :cancel_pipeline end end @@ -35,6 +36,7 @@ it 'includes ability to update pipeline' do expect(policy).to be_allowed :update_pipeline + expect(policy).to be_allowed :cancel_pipeline end end @@ -47,6 +49,7 @@ it 'does not include ability to update pipeline' do expect(policy).to be_disallowed :update_pipeline + expect(policy).to be_disallowed :cancel_pipeline end end @@ -57,6 +60,7 @@ it 'includes ability to update pipeline' do expect(policy).to be_allowed :update_pipeline + expect(policy).to be_allowed :cancel_pipeline end end end @@ -70,6 +74,7 @@ allow_any_instance_of(Project).to receive(:branch_allows_collaboration?).and_return(true) expect(policy).to be_allowed :update_pipeline + expect(policy).to be_allowed :cancel_pipeline end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 3de006d8c9b0693e959542bc2afe632366c867df..9635bd449d21bf7346ab78d2bb37d9b3f7ae5a1d 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -214,6 +214,7 @@ def set_access_level(access_level) it 'allows modify pipelines' do expect_allowed(:create_pipeline) expect_allowed(:update_pipeline) + expect_allowed(:cancel_pipeline) expect_allowed(:create_pipeline_schedule) end end @@ -224,6 +225,7 @@ def set_access_level(access_level) it 'disallows to modify pipelines' do expect_disallowed(:create_pipeline) expect_disallowed(:update_pipeline) + expect_disallowed(:cancel_pipeline) expect_disallowed(:destroy_pipeline) expect_disallowed(:create_pipeline_schedule) end @@ -304,7 +306,7 @@ def set_access_level(access_level) it 'disallows pipeline and commit_status permissions' do builds_permissions = [ - :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline, + :create_pipeline, :update_pipeline, :cancel_pipeline, :admin_pipeline, :destroy_pipeline, :create_commit_status, :update_commit_status, :admin_commit_status, :destroy_commit_status ] @@ -316,7 +318,7 @@ def set_access_level(access_level) context 'repository feature' do let(:repository_permissions) do [ - :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline, + :create_pipeline, :update_pipeline, :cancel_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,