Merge branch 'feature/gb/manual-actions-protected-branches-permissions' into 'master'

Check access to a branch when user triggers manual action

Closes #20261

See merge request !10494

app/controllers/projects/application_controller.rb

@@ -40,13 +40,15 @@ class Projects::ApplicationController < ApplicationController
       (current_user && current_user.already_forked?(project))
   end
-  def authorize_project!(action)
-    return access_denied! unless can?(current_user, action, project)
+  def authorize_action!(action)
+    unless can?(current_user, action, project)
+      return access_denied!
+    end
   end
   def method_missing(method_sym, *arguments, &block)
     if method_sym.to_s =~ /\Aauthorize_(.*)!\z/
-      authorize_project!($1.to_sym)
+      authorize_action!($1.to_sym)
     else
       super
     end

app/controllers/projects/builds_controller.rb

 class Projects::BuildsController < Projects::ApplicationController
   before_action :build, except: [:index, :cancel_all]
-  before_action :authorize_read_build!, only: [:index, :show, :status, :raw, :trace]
-  before_action :authorize_update_build!, except: [:index, :show, :status, :raw, :trace]
+
+  before_action :authorize_read_build!,
+    only: [:index, :show, :status, :raw, :trace]
+  before_action :authorize_update_build!,
+    except: [:index, :show, :status, :raw, :trace, :cancel_all]
+
   layout 'project'
   def index
@@ -28,7 +32,12 @@ class Projects::BuildsController < Projects::ApplicationController
   end
   def cancel_all
-    @project.builds.running_or_pending.each(&:cancel)
+    return access_denied! unless can?(current_user, :update_build, project)
+
+    @project.builds.running_or_pending.each do |build|
+      build.cancel if can?(current_user, :update_build, build)
+    end
+
     redirect_to namespace_project_builds_path(project.namespace, project)
   end
@@ -107,8 +116,13 @@ class Projects::BuildsController < Projects::ApplicationController
   private
+  def authorize_update_build!
+    return access_denied! unless can?(current_user, :update_build, build)
+  end
+
   def build
-    @build ||= project.builds.find_by!(id: params[:id]).present(current_user: current_user)
+    @build ||= project.builds.find(params[:id])
+      .present(current_user: current_user)
   end
   def build_path(build)

app/models/ci/build.rb

@@ -111,14 +111,9 @@ module Ci
     end
     def play(current_user)
-      # Try to queue a current build
-      if self.enqueue
-        self.update(user: current_user)
-        self
-      else
-        # Otherwise we need to create a duplicate
-        Ci::Build.retry(self, current_user)
-      end
+      Ci::PlayBuildService
+        .new(project, current_user)
+        .execute(self)
     end
     def cancelable?

app/models/deployment.rb

@@ -85,8 +85,8 @@ class Deployment < ActiveRecord::Base
   end
   def stop_action
-    return nil unless on_stop.present?
-    return nil unless manual_actions
+    return unless on_stop.present?
+    return unless manual_actions
     @stop_action ||= manual_actions.find_by(name: on_stop)
   end

app/policies/base_policy.rb

@@ -97,6 +97,10 @@ class BasePolicy
     rules
   end
+  def rules
+    raise NotImplementedError
+  end
+
   def delegate!(new_subject)
     @rule_set.merge(Ability.allowed(@user, new_subject))
   end

app/policies/ci/build_policy.rb

 module Ci
   class BuildPolicy < CommitStatusPolicy
+    alias_method :build, :subject
+
     def rules
       super
@@ -8,6 +10,20 @@ module Ci
       %w[read create update admin].each do |rule|
         cannot! :"#{rule}_commit_status" unless can? :"#{rule}_build"
       end
+
+      if can?(:update_build) && protected_action?
+        cannot! :update_build
+      end
+    end
+
+    private
+
+    def protected_action?
+      return false unless build.action?
+
+      !::Gitlab::UserAccess
+        .new(user, project: build.project)
+        .can_push_to_branch?(build.ref)
     end
   end
 end

app/policies/ci/pipeline_policy.rb

 module Ci
-  class PipelinePolicy < BuildPolicy
+  class PipelinePolicy < BasePolicy
+    def rules
+      delegate! @subject.project
+    end
   end
 end

app/policies/environment_policy.rb

 class EnvironmentPolicy < BasePolicy
+  alias_method :environment, :subject
+
   def rules
-    delegate! @subject.project
+    delegate! environment.project
+
+    if can?(:create_deployment) && environment.stop_action?
+      can! :stop_environment if can_play_stop_action?
+    end
+  end
+
+  private
+
+  def can_play_stop_action?
+    Ability.allowed?(user, :update_build, environment.stop_action)
   end
 end

app/serializers/build_action_entity.rb

@@ -13,4 +13,12 @@ class BuildActionEntity < Grape::Entity
   end
   expose :playable?, as: :playable
+
+  private
+
+  alias_method :build, :object
+
+  def playable?
+    build.playable? && can?(request.user, :update_build, build)
+  end
 end

app/serializers/build_entity.rb

@@ -12,7 +12,7 @@ class BuildEntity < Grape::Entity
     path_to(:retry_namespace_project_build, build)
   end
-  expose :play_path, if: ->(build, _) { build.playable? } do |build|
+  expose :play_path, if: -> (*) { playable? } do |build|
     path_to(:play_namespace_project_build, build)
   end
@@ -25,11 +25,15 @@ class BuildEntity < Grape::Entity
   alias_method :build, :object
-  def path_to(route, build)
-    send("#{route}_path", build.project.namespace, build.project, build)
+  def playable?
+    build.playable? && can?(request.user, :update_build, build)
   end
   def detailed_status
     build.detailed_status(request.user)
   end
+
+  def path_to(route, build)
+    send("#{route}_path", build.project.namespace, build.project, build)
+  end
 end

app/serializers/pipeline_entity.rb

@@ -48,15 +48,15 @@ class PipelineEntity < Grape::Entity
   end
   expose :commit, using: CommitEntity
-  expose :yaml_errors, if: ->(pipeline, _) { pipeline.has_yaml_errors? }
-  expose :retry_path, if: proc { can_retry? }  do |pipeline|
+  expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? }
+  expose :retry_path, if: -> (*) { can_retry? }  do |pipeline|
     retry_namespace_project_pipeline_path(pipeline.project.namespace,
                                           pipeline.project,
                                           pipeline.id)
   end
-  expose :cancel_path, if: proc { can_cancel? } do |pipeline|
+  expose :cancel_path, if: -> (*) { can_cancel? } do |pipeline|
     cancel_namespace_project_pipeline_path(pipeline.project.namespace,
                                            pipeline.project,
                                            pipeline.id)

app/services/ci/play_build_service.rb

+module Ci
+  class PlayBuildService < ::BaseService
+    def execute(build)
+      unless can?(current_user, :update_build, build)
+        raise Gitlab::Access::AccessDeniedError
+      end
+
+      # Try to enqueue the build, otherwise create a duplicate.
+      #
+      if build.enqueue
+        build.tap { |action| action.update(user: current_user) }
+      else
+        Ci::Build.retry(build, current_user)
+      end
+    end
+  end
+end

app/services/ci/retry_pipeline_service.rb

@@ -8,6 +8,8 @@ module Ci
       end
       pipeline.retryable_builds.find_each do |build|
+        next unless can?(current_user, :update_build, build)
+
         Ci::RetryBuildService.new(project, current_user)
           .reprocess(build)
       end

app/services/ci/stop_environments_service.rb

@@ -5,10 +5,11 @@ module Ci
     def execute(branch_name)
       @ref = branch_name
-      return unless has_ref?
+      return unless @ref.present?
       environments.each do |environment|
-        next unless can?(current_user, :create_deployment, project)
+        next unless environment.stop_action?
+        next unless can?(current_user, :stop_environment, environment)
         environment.stop_with_action!(current_user)
       end
@@ -16,13 +17,10 @@ module Ci
     private
-    def has_ref?
-      @ref.present?
-    end
-
     def environments
-      @environments ||=
-        EnvironmentsFinder.new(project, current_user, ref: @ref, recently_updated: true).execute
+      @environments ||= EnvironmentsFinder
+        .new(project, current_user, ref: @ref, recently_updated: true)
+        .execute
     end
   end
 end

app/views/projects/ci/builds/_build.html.haml

@@ -102,7 +102,7 @@
           = link_to cancel_namespace_project_build_path(job.project.namespace, job.project, job, return_to: request.original_url), method: :post, title: 'Cancel', class: 'btn btn-build' do
             = icon('remove', class: 'cred')
         - elsif allow_retry
-          - if job.playable? && !admin
+          - if job.playable? && !admin && can?(current_user, :update_build, job)
             = link_to play_namespace_project_build_path(job.project.namespace, job.project, job, return_to: request.original_url), method: :post, title: 'Play', class: 'btn btn-build' do
               = custom_icon('icon_play')
           - elsif job.retryable?

changelogs/unreleased/feature-gb-manual-actions-protected-branches-permissions.yml

+---
+title: Implement protected manual actions
+merge_request: 10494
+author:

doc/ci/yaml/README.md

@@ -553,6 +553,8 @@ The above script will:
 #### Manual actions
 > Introduced in GitLab 8.10.
+> Blocking manual actions were introduced in GitLab 9.0
+> Protected actions were introduced in GitLab 9.2
 Manual actions are a special type of job that are not executed automatically;
 they need to be explicitly started by a user. Manual actions can be started
@@ -578,7 +580,10 @@ Optional manual actions have `allow_failure: true` set by default.
 **Statuses of optional actions do not contribute to overall pipeline status.**
-> Blocking manual actions were introduced in GitLab 9.0
+**Manual actions are considered to be write actions, so permissions for
+protected branches are used when user wants to trigger an action. In other
+words, in order to trigger a manual action assigned to a branch that the
+pipeline is running for, user needs to have ability to push to this branch.**
 ### environment

lib/api/jobs.rb

@@ -132,6 +132,7 @@ module API
         authorize_update_builds!
         build = get_build!(params[:job_id])
+        authorize!(:update_build, build)
         build.cancel
@@ -148,6 +149,7 @@ module API
         authorize_update_builds!
         build = get_build!(params[:job_id])
+        authorize!(:update_build, build)
         return forbidden!('Job is not retryable') unless build.retryable?
         build = Ci::Build.retry(build, current_user)
@@ -165,6 +167,7 @@ module API
         authorize_update_builds!
         build = get_build!(params[:job_id])
+        authorize!(:update_build, build)
         return forbidden!('Job is not erasable!') unless build.erasable?
         build.erase(erased_by: current_user)
@@ -181,6 +184,7 @@ module API
         authorize_update_builds!
         build = get_build!(params[:job_id])
+        authorize!(:update_build, build)
         return not_found!(build) unless build.artifacts?
         build.keep_artifacts!
@@ -201,6 +205,7 @@ module API
         build = get_build!(params[:job_id])
+        authorize!(:update_build, build)
         bad_request!("Unplayable Job") unless build.playable?
         build.play(current_user)
@@ -211,12 +216,12 @@ module API
     end
     helpers do
-      def get_build(id)
+      def find_build(id)
         user_project.builds.find_by(id: id.to_i)
       end
       def get_build!(id)
-        get_build(id) || not_found!
+        find_build(id) || not_found!
       end
       def present_artifacts!(artifacts_file)

lib/api/v3/builds.rb

@@ -134,6 +134,7 @@ module API
           authorize_update_builds!
           build = get_build!(params[:build_id])
+          authorize!(:update_build, build)
           build.cancel
@@ -150,6 +151,7 @@ module API
           authorize_update_builds!
           build = get_build!(params[:build_id])
+          authorize!(:update_build, build)
           return forbidden!('Build is not retryable') unless build.retryable?
           build = Ci::Build.retry(build, current_user)
@@ -167,6 +169,7 @@ module API
           authorize_update_builds!
           build = get_build!(params[:build_id])
+          authorize!(:update_build, build)
           return forbidden!('Build is not erasable!') unless build.erasable?
           build.erase(erased_by: current_user)
@@ -183,6 +186,7 @@ module API
           authorize_update_builds!
           build = get_build!(params[:build_id])
+          authorize!(:update_build, build)
           return not_found!(build) unless build.artifacts?
           build.keep_artifacts!
@@ -202,7 +206,7 @@ module API
           authorize_read_builds!
           build = get_build!(params[:build_id])
-
+          authorize!(:update_build, build)
           bad_request!("Unplayable Job") unless build.playable?
           build.play(current_user)
@@ -213,12 +217,12 @@ module API
       end
       helpers do
-        def get_build(id)
+        def find_build(id)
           user_project.builds.find_by(id: id.to_i)
         end
         def get_build!(id)
-          get_build(id) || not_found!
+          find_build(id) || not_found!
         end
         def present_artifacts!(artifacts_file)

lib/gitlab/ci/status/build/action.rb

+module Gitlab
+  module Ci
+    module Status
+      module Build
+        class Action < Status::Extended
+          def label
+            if has_action?
+              @status.label
+            else
+              "#{@status.label} (not allowed)"
+            end
+          end
+
+          def self.matches?(build, user)
+            build.action?
+          end
+        end
+      end
+    end
+  end
+end