diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 5a419aab8e1b92ee4df9a2516b24f8dc45988d54..d5a7f25d4ce8a95f1feeb59acba568610c1fab62 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -15,6 +15,7 @@ class Projects::JobsController < Projects::ApplicationController before_action :authorize_read_build_report_results!, only: [:test_report_summary] before_action :authorize_update_build!, except: [:index, :show, :raw, :trace, :erase, :cancel, :unschedule, :test_report_summary] + before_action :authorize_cancel_build!, only: [:cancel] before_action :authorize_erase_build!, only: [:erase] before_action :authorize_use_build_terminal!, only: [:terminal, :terminal_websocket_authorize] before_action :verify_api_request!, only: :terminal_websocket_authorize @@ -193,6 +194,10 @@ def authorize_update_build! return access_denied! unless can?(current_user, :update_build, @build) end + def authorize_cancel_build! + return access_denied! unless can?(current_user, :cancel_build, @build) + end + def authorize_erase_build! return access_denied! unless can?(current_user, :erase_build, @build) end diff --git a/app/graphql/types/permission_types/ci/pipeline.rb b/app/graphql/types/permission_types/ci/pipeline.rb index cfd68380005b6427ffa0ed780fe96a85165c1bef..94adbf7c59b67f4fc80266c64746ae17371ca67d 100644 --- a/app/graphql/types/permission_types/ci/pipeline.rb +++ b/app/graphql/types/permission_types/ci/pipeline.rb @@ -8,6 +8,7 @@ class Pipeline < BasePermissionType abilities :admin_pipeline, :destroy_pipeline ability_field :update_pipeline, calls_gitaly: true + ability_field :cancel_pipeline, calls_gitaly: true end end end diff --git a/app/serializers/ci/job_entity.rb b/app/serializers/ci/job_entity.rb index 813938c2a188c8a57adc77d58f80b14141e99739..828a9eb33a526ec208c033aae22b2a243da20155 100644 --- a/app/serializers/ci/job_entity.rb +++ b/app/serializers/ci/job_entity.rb @@ -53,7 +53,7 @@ class JobEntity < Grape::Entity alias_method :job, :object def cancelable? - job.cancelable? && can?(request.current_user, :update_build, job) + job.cancelable? && can?(request.current_user, :cancel_build, job) end def retryable? diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 6ec9b4a233de340aeff1fede9bcf735ade07364f..76d6b0a042dddd3a5d01c5b049cdd258a0adb7a9 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -104,10 +104,10 @@ .btn-group - if can?(current_user, :read_job_artifacts, job) && job.artifacts? = link_button_to nil, download_project_job_artifacts_path(job.project, job), rel: 'nofollow', download: '', title: _('Download artifacts'), icon: 'download' + - if can?(current_user, :cancel_build, job) && job.active? + = link_button_to nil, cancel_project_job_path(job.project, job, continue: { to: request.fullpath }), method: :post, title: _('Cancel'), icon: 'cancel' - if can?(current_user, :update_build, job) - - if job.active? && can?(current_user, :cancel_build, job) - = link_button_to nil, cancel_project_job_path(job.project, job, continue: { to: request.fullpath }), method: :post, title: _('Cancel'), icon: 'cancel' - - elsif job.scheduled? + - if job.scheduled? = render Pajamas::ButtonComponent.new(disabled: true, icon: 'planning') do %time.js-remaining-time{ datetime: job.scheduled_at.utc.iso8601 } = duration_in_numbers(job.execute_in) @@ -124,7 +124,7 @@ class: 'has-tooltip', icon: 'time-out' - elsif allow_retry - - if job.playable? && !admin && can?(current_user, :update_build, job) + - if job.playable? && !admin = link_button_to nil, play_project_job_path(job.project, job, return_to: request.original_url), method: :post, title: _('Play'), icon: 'play' - elsif job.retryable? = link_button_to nil, retry_project_job_path(job.project, job, return_to: request.original_url), method: :post, title: _('Retry'), icon: 'retry' diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 9917c897645ffb3a91a69cea976622abd6fccdf3..cd89078eedffe0dd85a9005d4d200024b552ce18 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -22809,6 +22809,7 @@ Represents pipeline counts for the project. | Name | Type | Description | | ---- | ---- | ----------- | | `adminPipeline` | [`Boolean!`](#boolean) | If `true`, the user can perform `admin_pipeline` on this resource. | +| `cancelPipeline` | [`Boolean!`](#boolean) | If `true`, the user can perform `cancel_pipeline` on this resource. | | `destroyPipeline` | [`Boolean!`](#boolean) | If `true`, the user can perform `destroy_pipeline` on this resource. | | `updatePipeline` | [`Boolean!`](#boolean) | If `true`, the user can perform `update_pipeline` on this resource. | diff --git a/ee/app/models/ci/project_cancellation_restriction.rb b/ee/app/models/ci/project_cancellation_restriction.rb index 7eaca4f6a619df543a52c840eb8f0e17d58f4042..69a46703809948339ba2460d03a1b0b01e14b0ae 100644 --- a/ee/app/models/ci/project_cancellation_restriction.rb +++ b/ee/app/models/ci/project_cancellation_restriction.rb @@ -2,7 +2,7 @@ module Ci class ProjectCancellationRestriction - # Checks if cancellation restrictions are applied for piplines and processables + # Checks if cancellation restrictions are applied for pipelines and processables # based on the given project include Gitlab::Utils::StrongMemoize diff --git a/lib/api/ci/pipelines.rb b/lib/api/ci/pipelines.rb index 3361f4564b26327a763a0dd4d4340a5a3b03f190..b5123ab49dc550215c45895a4ae3f1addb866be5 100644 --- a/lib/api/ci/pipelines.rb +++ b/lib/api/ci/pipelines.rb @@ -352,7 +352,7 @@ class Pipelines < ::API::Base requires :pipeline_id, type: Integer, desc: 'The pipeline ID', documentation: { example: 18 } end post ':id/pipelines/:pipeline_id/cancel', urgency: :low, feature_category: :continuous_integration do - authorize! :update_pipeline, pipeline + authorize! :cancel_pipeline, pipeline # TODO: inconsistent behavior: when pipeline is not cancelable we should return an error ::Ci::CancelPipelineService.new(pipeline: pipeline, current_user: current_user).execute diff --git a/spec/graphql/types/permission_types/ci/pipeline_spec.rb b/spec/graphql/types/permission_types/ci/pipeline_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6830b659b12866ccb4363723174f101ad9e90f6c --- /dev/null +++ b/spec/graphql/types/permission_types/ci/pipeline_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Types::PermissionTypes::Ci::Pipeline, feature_category: :continuous_integration do + it 'has expected permission fields' do + expected_permissions = [ + :admin_pipeline, :destroy_pipeline, :update_pipeline, :cancel_pipeline + ] + + expect(described_class).to have_graphql_fields(expected_permissions).only + end +end diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index ab92936440c5d7ef89818bebcc394d331e6d0a2a..ad568e60d5ce0abc11247a0818816416fe6c4d37 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -109,7 +109,7 @@ allow(project).to receive(:branch_allows_collaboration?).and_return(true) end - it 'enables update_build if user is maintainer' do + it 'enables updates if user is maintainer', :aggregate_failures do expect(policy).to be_allowed :cancel_build expect(policy).to be_allowed :update_build expect(policy).to be_allowed :update_commit_status diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 2ab112a8527655b827a0faa573f3cad3db993928..382aabd45a136333e6685549875e1c7a7a49d657 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -791,14 +791,14 @@ def go end context 'authorized user' do - context 'user with :update_build persmission' do + context 'user with :cancel_build permission' do it 'cancels running or pending job' do expect(response).to have_gitlab_http_status(:created) expect(project.builds.first.status).to eq('success') end end - context 'user without :update_build permission' do + context 'user without :cancel_build permission' do let(:api_user) { reporter } it 'does not cancel job' do diff --git a/spec/serializers/ci/job_entity_spec.rb b/spec/serializers/ci/job_entity_spec.rb index 6dce87a1fc50fb6528ee2421fb3fdac500384b95..c3d0de11405c537e4ad322572e8a2609d4698150 100644 --- a/spec/serializers/ci/job_entity_spec.rb +++ b/spec/serializers/ci/job_entity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::JobEntity do +RSpec.describe Ci::JobEntity, feature_category: :continuous_integration do let(:user) { create(:user) } let(:job) { create(:ci_build, :running) } let(:project) { job.project } diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index 5014a810f35d2f7f6d5b4e0e905e18a09e1d4b3d..68eb35398134f3a243c3b32eb6f466077cc6dcfc 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -54,7 +54,7 @@ create_environment create_merge_request_from admin_metrics_dashboard_annotation create_pipeline create_release create_wiki destroy_container_image push_code read_pod_logs - read_terraform_state resolve_note update_build update_commit_status + read_terraform_state resolve_note update_build cancel_build update_commit_status update_container_image update_deployment update_environment update_merge_request update_pipeline update_release destroy_release read_resource_group update_resource_group update_escalation_status