Merge branch 'master' into 25226-realtime-pipelines-fe

* master: (40 commits)
  Use GitLab Pages v0.4.2
  Do not reprocess actions when user retries pipeline
  Add specs for extended status for manual actions
  Refine inheritance model of extended CI/CD statuses
  Introduce generic manual action extended status class
  Check ability to update build on the API resource
  Require build to be present in the controller
  Authorize build update on per object basis
  Use update build policy instead of new play policy
  Improve environment policy class
  Rephrase documentation for protected actions feature
  Improve code style related to protected actions
  Add changelog entry for external env URL btn fix
  Hide environment external URL button if not defined
  Fix builds controller spec related to protected actions
  Fix environment policy class name in specs
  Add Changelog entry for protected manual actions
  Document protected manual actions feature
  Improve specs for jobs API regarding manual actions
  Fix Rubocop offense in environments policy class
  ...

GITLAB_PAGES_VERSION

-0.4.1
+0.4.2

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 :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? }
 
-  expose :retry_path, if: proc { can_retry? }  do |pipeline|
+  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?

app/views/projects/environments/terminal.html.haml

@@ -16,8 +16,9 @@
 
       .col-sm-6
         .nav-controls
-          = link_to @environment.external_url, class: 'btn btn-default', target: '_blank', rel: 'noopener noreferrer nofollow' do
-            = icon('external-link')
+          - if @environment.external_url.present?
+            = link_to @environment.external_url, class: 'btn btn-default', target: '_blank', rel: 'noopener noreferrer nofollow' do
+              = icon('external-link')
           = render 'projects/deployments/actions', deployment: @environment.last_deployment
 
 .terminal-container{ class: container_class }

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

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

changelogs/unreleased/fix-gb-hide-environment-external-url-btn-when-not-provided.yml

+---
+title: Hide external environment URL button on terminal page if URL is not defined
+merge_request: 11029
+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

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

@@ -2,9 +2,7 @@ module Gitlab
   module Ci
     module Status
       module Build
-        class Cancelable < SimpleDelegator
-          include Status::Extended
-
+        class Cancelable < Status::Extended
           def has_action?
             can?(user, :update_build, subject)
           end

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

@@ -8,7 +8,8 @@ module Gitlab
               Status::Build::Retryable],
              [Status::Build::FailedAllowed,
               Status::Build::Play,
-              Status::Build::Stop]]
+              Status::Build::Stop],
+             [Status::Build::Action]]
           end
 
           def self.common_helpers

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

@@ -2,9 +2,7 @@ module Gitlab
   module Ci
     module Status
       module Build
-        class FailedAllowed < SimpleDelegator
-          include Status::Extended
-
+        class FailedAllowed < Status::Extended
           def label
             'failed (allowed to fail)'
           end

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

@@ -2,9 +2,7 @@ module Gitlab
   module Ci
     module Status
       module Build
-        class Play < SimpleDelegator
-          include Status::Extended
-
+        class Play < Status::Extended
           def label
             'manual play action'
           end

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

@@ -2,9 +2,7 @@ module Gitlab
   module Ci
     module Status
       module Build
-        class Retryable < SimpleDelegator
-          include Status::Extended
-
+        class Retryable < Status::Extended
           def has_action?
             can?(user, :update_build, subject)
           end

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

@@ -2,9 +2,7 @@ module Gitlab
   module Ci
     module Status
       module Build
-        class Stop < SimpleDelegator
-          include Status::Extended
-
+        class Stop < Status::Extended
           def label
             'manual stop action'
           end

lib/gitlab/ci/status/extended.rb

 module Gitlab
   module Ci
     module Status
-      module Extended
-        extend ActiveSupport::Concern
+      class Extended < SimpleDelegator
+        def initialize(status)
+          super(@status = status)
+        end
 
-        class_methods do
-          def matches?(_subject, _user)
-            raise NotImplementedError
-          end
+        def self.matches?(_subject, _user)
+          raise NotImplementedError
         end
       end
     end

lib/gitlab/ci/status/pipeline/blocked.rb

@@ -2,9 +2,7 @@ module Gitlab
   module Ci
     module Status
       module Pipeline
-        class Blocked < SimpleDelegator
-          include Status::Extended
-
+        class Blocked < Status::Extended
           def text
             'blocked'
           end

lib/gitlab/ci/status/success_warning.rb

@@ -5,9 +5,7 @@ module Gitlab
       # Extended status used when pipeline or stage passed conditionally.
       # This means that failed jobs that are allowed to fail were present.
       #
-      class SuccessWarning < SimpleDelegator
-        include Status::Extended
-
+      class SuccessWarning < Status::Extended
         def text
           'passed'
         end

spec/controllers/projects/builds_controller_spec.rb

@@ -261,7 +261,7 @@ describe Projects::BuildsController do
 
   describe 'POST play' do
     before do
-      project.add_developer(user)
+      project.add_master(user)
       sign_in(user)
 
       post_play

spec/factories/environments.rb

@@ -18,15 +18,21 @@ FactoryGirl.define do
       # interconnected objects to simulate a review app.
       #
       after(:create) do |environment, evaluator|
+        pipeline = create(:ci_pipeline, project: environment.project)
+
+        deployable = create(:ci_build, name: "#{environment.name}:deploy",
+                                       pipeline: pipeline)
+
         deployment = create(:deployment,
                             environment: environment,
                             project: environment.project,
+                            deployable: deployable,
                             ref: evaluator.ref,
                             sha: environment.project.commit(evaluator.ref).id)
 
         teardown_build = create(:ci_build, :manual,
-                                name: "#{deployment.environment.name}:teardown",
-                                pipeline: deployment.deployable.pipeline)
+                                name: "#{environment.name}:teardown",
+                                pipeline: pipeline)
 
         deployment.update_column(:on_stop, teardown_build.name)
         environment.update_attribute(:deployments, [deployment])

spec/features/projects/environments/environment_spec.rb

@@ -62,6 +62,8 @@ feature 'Environment', :feature do
                                        name: 'deploy to production')
           end
 
+          given(:role) { :master }
+
           scenario 'does show a play button' do
             expect(page).to have_link(action.name.humanize)
           end
@@ -132,6 +134,8 @@ feature 'Environment', :feature do
                                     on_stop: 'close_app')
               end
 
+              given(:role) { :master }
+
               scenario 'does allow to stop environment' do
                 click_link('Stop')
 

spec/lib/gitlab/chat_commands/command_spec.rb

@@ -40,11 +40,15 @@ describe Gitlab::ChatCommands::Command, service: true do
 
     context 'when trying to do deployment' do
       let(:params) { { text: 'deploy staging to production' } }
-      let!(:build) { create(:ci_build, project: project) }
+      let!(:build) { create(:ci_build, pipeline: pipeline) }
+      let!(:pipeline) { create(:ci_pipeline, project: project) }
       let!(:staging) { create(:environment, name: 'staging', project: project) }
       let!(:deployment) { create(:deployment, environment: staging, deployable: build) }
+
       let!(:manual) do
-        create(:ci_build, :manual, project: project, pipeline: build.pipeline, name: 'first', environment: 'production')
+        create(:ci_build, :manual, pipeline: pipeline,
+                                   name: 'first',
+                                   environment: 'production')
       end
 
       context 'and user can not create deployment' do
@@ -56,7 +60,7 @@ describe Gitlab::ChatCommands::Command, service: true do
 
       context 'and user does have deployment permission' do
         before do
-          project.team << [user, :developer]
+          build.project.add_master(user)
         end
 
         it 'returns action' do
@@ -66,7 +70,9 @@ describe Gitlab::ChatCommands::Command, service: true do
 
         context 'when duplicate action exists' do
           let!(:manual2) do
-            create(:ci_build, :manual, project: project, pipeline: build.pipeline, name: 'second', environment: 'production')
+            create(:ci_build, :manual, pipeline: pipeline,
+                                       name: 'second',
+                                       environment: 'production')
           end
 
           it 'returns error' do

spec/lib/gitlab/chat_commands/deploy_spec.rb

@@ -7,7 +7,7 @@ describe Gitlab::ChatCommands::Deploy, service: true do
     let(:regex_match) { described_class.match('deploy staging to production') }
 
     before do
-      project.team << [user, :master]
+      project.add_master(user)
     end
 
     subject do
@@ -23,7 +23,8 @@ describe Gitlab::ChatCommands::Deploy, service: true do
 
     context 'with environment' do
       let!(:staging) { create(:environment, name: 'staging', project: project) }
-      let!(:build) { create(:ci_build, project: project) }
+      let!(:pipeline) { create(:ci_pipeline, project: project) }
+      let!(:build) { create(:ci_build, pipeline: pipeline) }
       let!(:deployment) { create(:deployment, environment: staging, deployable: build) }
 
       context 'without actions' do
@@ -35,7 +36,9 @@ describe Gitlab::ChatCommands::Deploy, service: true do
 
       context 'with action' do
         let!(:manual1) do
-          create(:ci_build, :manual, project: project, pipeline: build.pipeline, name: 'first', environment: 'production')
+          create(:ci_build, :manual, pipeline: pipeline,
+                                     name: 'first',
+                                     environment: 'production')
         end
 
         it 'returns success result' do
@@ -45,7 +48,9 @@ describe Gitlab::ChatCommands::Deploy, service: true do
 
         context 'when duplicate action exists' do
           let!(:manual2) do
-            create(:ci_build, :manual, project: project, pipeline: build.pipeline, name: 'second', environment: 'production')
+            create(:ci_build, :manual, pipeline: pipeline,
+                                       name: 'second',
+                                       environment: 'production')
           end
 
           it 'returns error' do
@@ -57,8 +62,7 @@ describe Gitlab::ChatCommands::Deploy, service: true do
         context 'when teardown action exists' do
           let!(:teardown) do
             create(:ci_build, :manual, :teardown_environment,
-                   project: project, pipeline: build.pipeline,
-                   name: 'teardown', environment: 'production')
+                   pipeline: pipeline, name: 'teardown', environment: 'production')
           end
 
           it 'returns the success message' do

spec/lib/gitlab/ci/status/build/action_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Status::Build::Action do
+  let(:status) { double('core status') }
+  let(:user) { double('user') }
+
+  subject do
+    described_class.new(status)
+  end
+
+  describe '#label' do
+    before do
+      allow(status).to receive(:label).and_return('label')
+    end
+
+    context 'when status has action' do
+      before do
+        allow(status).to receive(:has_action?).and_return(true)
+      end
+
+      it 'does not append text' do
+        expect(subject.label).to eq 'label'
+      end
+    end
+
+    context 'when status does not have action' do
+      before do
+        allow(status).to receive(:has_action?).and_return(false)
+      end
+
+      it 'appends text about action not allowed' do
+        expect(subject.label).to eq 'label (not allowed)'
+      end
+    end
+  end
+
+  describe '.matches?' do
+    subject { described_class.matches?(build, user) }
+
+    context 'when build is an action' do
+      let(:build) { create(:ci_build, :manual) }
+
+      it 'is a correct match' do
+        expect(subject).to be true
+      end
+    end
+
+    context 'when build is not manual' do
+      let(:build) { create(:ci_build) }
+
+      it 'does not match' do
+        expect(subject).to be false
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/status/build/factory_spec.rb

@@ -204,11 +204,12 @@ describe Gitlab::Ci::Status::Build::Factory do
 
       it 'matches correct extended statuses' do
         expect(factory.extended_statuses)
-          .to eq [Gitlab::Ci::Status::Build::Play]
+          .to eq [Gitlab::Ci::Status::Build::Play,
+                  Gitlab::Ci::Status::Build::Action]
       end
 
-      it 'fabricates a play detailed status' do
-        expect(status).to be_a Gitlab::Ci::Status::Build::Play
+      it 'fabricates action detailed status' do
+        expect(status).to be_a Gitlab::Ci::Status::Build::Action
       end
 
       it 'fabricates status with correct details' do
@@ -216,11 +217,26 @@ describe Gitlab::Ci::Status::Build::Factory do
         expect(status.group).to eq 'manual'
         expect(status.icon).to eq 'icon_status_manual'
         expect(status.favicon).to eq 'favicon_status_manual'
-        expect(status.label).to eq 'manual play action'
+        expect(status.label).to include 'manual play action'
         expect(status).to have_details
-        expect(status).to have_action
         expect(status.action_path).to include 'play'
       end
+
+      context 'when user has ability to play action' do
+        before do
+          build.project.add_master(user)
+        end
+
+        it 'fabricates status that has action' do
+          expect(status).to have_action
+        end
+      end
+
+      context 'when user does not have ability to play action' do
+        it 'fabricates status that has no action' do
+          expect(status).not_to have_action
+        end
+      end
     end
 
     context 'when build is an environment stop action' do
@@ -232,21 +248,24 @@ describe Gitlab::Ci::Status::Build::Factory do
 
       it 'matches correct extended statuses' do
         expect(factory.extended_statuses)
-          .to eq [Gitlab::Ci::Status::Build::Stop]
+          .to eq [Gitlab::Ci::Status::Build::Stop,
+                  Gitlab::Ci::Status::Build::Action]
       end
 
-      it 'fabricates a stop detailed status' do
-        expect(status).to be_a Gitlab::Ci::Status::Build::Stop
+      it 'fabricates action detailed status' do
+        expect(status).to be_a Gitlab::Ci::Status::Build::Action
       end
 
-      it 'fabricates status with correct details' do
-        expect(status.text).to eq 'manual'
-        expect(status.group).to eq 'manual'
-        expect(status.icon).to eq 'icon_status_manual'
-        expect(status.favicon).to eq 'favicon_status_manual'
-        expect(status.label).to eq 'manual stop action'
-        expect(status).to have_details
-        expect(status).to have_action
+      context 'when user is not allowed to execute manual action' do
+        it 'fabricates status with correct details' do
+          expect(status.text).to eq 'manual'
+          expect(status.group).to eq 'manual'
+          expect(status.icon).to eq 'icon_status_manual'
+          expect(status.favicon).to eq 'favicon_status_manual'
+          expect(status.label).to eq 'manual stop action (not allowed)'
+          expect(status).to have_details
+          expect(status).not_to have_action
+        end
       end
     end
   end

spec/lib/gitlab/ci/status/build/play_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Ci::Status::Build::Play do
-  let(:status) { double('core') }
-  let(:user) { double('user') }
+  let(:user) { create(:user) }
+  let(:build) { create(:ci_build, :manual) }
+  let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
   subject { described_class.new(status) }
 
   describe '#label' do
-    it { expect(subject.label).to eq 'manual play action' }
+    it 'has a label that says it is a manual action' do
+      expect(subject.label).to eq 'manual play action'
+    end
   end
 
-  describe 'action details' do
-    let(:user) { create(:user) }
-    let(:build) { create(:ci_build) }
-    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
-
-    describe '#has_action?' do
-      context 'when user is allowed to update build' do
-        before { build.project.team << [user, :developer] }
+  describe '#has_action?' do
+    context 'when user is allowed to update build' do
+      context 'when user can push to branch' do
+        before { build.project.add_master(user) }
 
         it { is_expected.to have_action }
       end
 
-      context 'when user is not allowed to update build' do
+      context 'when user can not push to the branch' do
+        before { build.project.add_developer(user) }
+
         it { is_expected.not_to have_action }
       end
     end
 
-    describe '#action_path' do
-      it { expect(subject.action_path).to include "#{build.id}/play" }
+    context 'when user is not allowed to update build' do
+      it { is_expected.not_to have_action }
     end
+  end
 
-    describe '#action_icon' do
-      it { expect(subject.action_icon).to eq 'icon_action_play' }
-    end
+  describe '#action_path' do
+    it { expect(subject.action_path).to include "#{build.id}/play" }
+  end
 
-    describe '#action_title' do
-      it { expect(subject.action_title).to eq 'Play' }
-    end
+  describe '#action_icon' do
+    it { expect(subject.action_icon).to eq 'icon_action_play' }
+  end
+
+  describe '#action_title' do
+    it { expect(subject.action_title).to eq 'Play' }
   end
 
   describe '.matches?' do

spec/lib/gitlab/ci/status/extended_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Ci::Status::Extended do
-  subject do
-    Class.new.include(described_class)
-  end
-
   it 'requires subclass to implement matcher' do
-    expect { subject.matches?(double, double) }
+    expect { described_class.matches?(double, double) }
       .to raise_error(NotImplementedError)
   end
 end

spec/models/ci/build_spec.rb

@@ -897,22 +897,26 @@ describe Ci::Build, :models do
   end
 
   describe '#persisted_environment' do
-    before do
-      @environment = create(:environment, project: project, name: "foo-#{project.default_branch}")
+    let!(:environment) do
+      create(:environment, project: project, name: "foo-#{project.default_branch}")
     end
 
     subject { build.persisted_environment }
 
-    context 'referenced literally' do
-      let(:build) { create(:ci_build, pipeline: pipeline, environment: "foo-#{project.default_branch}") }
+    context 'when referenced literally' do
+      let(:build) do
+        create(:ci_build, pipeline: pipeline, environment: "foo-#{project.default_branch}")
+      end
 
-      it { is_expected.to eq(@environment) }
+      it { is_expected.to eq(environment) }
     end
 
-    context 'referenced with a variable' do
-      let(:build) { create(:ci_build, pipeline: pipeline, environment: "foo-$CI_COMMIT_REF_NAME") }
+    context 'when referenced with a variable' do
+      let(:build) do
+        create(:ci_build, pipeline: pipeline, environment: "foo-$CI_COMMIT_REF_NAME")
+      end
 
-      it { is_expected.to eq(@environment) }
+      it { is_expected.to eq(environment) }
     end
   end
 
@@ -923,26 +927,8 @@ describe Ci::Build, :models do
       project.add_developer(user)
     end
 
-    context 'when build is manual' do
-      it 'enqueues a build' do
-        new_build = build.play(user)
-
-        expect(new_build).to be_pending
-        expect(new_build).to eq(build)
-      end
-    end
-
-    context 'when build is passed' do
-      before do
-        build.update(status: 'success')
-      end
-
-      it 'creates a new build' do
-        new_build = build.play(user)
-
-        expect(new_build).to be_pending
-        expect(new_build).not_to eq(build)
-      end
+    it 'enqueues the build' do
+      expect(build.play(user)).to be_pending
     end
   end
 

spec/models/environment_spec.rb

@@ -206,25 +206,52 @@ describe Environment, models: true do
     end
 
     context 'when matching action is defined' do
-      let(:build) { create(:ci_build) }
-      let!(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') }
+      let(:pipeline) { create(:ci_pipeline, project: project) }
+      let(:build) { create(:ci_build, pipeline: pipeline) }
+
+      let!(:deployment) do
+        create(:deployment, environment: environment,
+                            deployable: build,
+                            on_stop: 'close_app')
+      end
 
-      context 'when action did not yet finish' do
-        let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') }
+      context 'when user is not allowed to stop environment' do
+        let!(:close_action) do
+          create(:ci_build, :manual, pipeline: pipeline, name: 'close_app')
+        end
 
-        it 'returns the same action' do
-          expect(subject).to eq(close_action)
-          expect(subject.user).to eq(user)
+        it 'raises an exception' do
+          expect { subject }.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
 
-      context 'if action did finish' do
-        let!(:close_action) { create(:ci_build, :manual, :success, pipeline: build.pipeline, name: 'close_app') }
+      context 'when user is allowed to stop environment' do
+        before do
+          project.add_master(user)
+        end
+
+        context 'when action did not yet finish' do
+          let!(:close_action) do
+            create(:ci_build, :manual, pipeline: pipeline, name: 'close_app')
+          end
+
+          it 'returns the same action' do
+            expect(subject).to eq(close_action)
+            expect(subject.user).to eq(user)
+          end
+        end
 
-        it 'returns a new action of the same type' do
-          is_expected.to be_persisted
-          expect(subject.name).to eq(close_action.name)
-          expect(subject.user).to eq(user)
+        context 'if action did finish' do
+          let!(:close_action) do
+            create(:ci_build, :manual, :success,
+                   pipeline: pipeline, name: 'close_app')
+          end
+
+          it 'returns a new action of the same type' do
+            expect(subject).to be_persisted
+            expect(subject.name).to eq(close_action.name)
+            expect(subject.user).to eq(user)
+          end
         end
       end
     end

spec/policies/ci/build_policy_spec.rb

@@ -89,5 +89,58 @@ describe Ci::BuildPolicy, :models do
         end
       end
     end
+
+    describe 'rules for manual actions' do
+      let(:project) { create(:project) }
+
+      before do
+        project.add_developer(user)
+      end
+
+      context 'when branch build is assigned to is protected' do
+        before do
+          create(:protected_branch, :no_one_can_push,
+                 name: 'some-ref', project: project)
+        end
+
+        context 'when build is a manual action' do
+          let(:build) do
+            create(:ci_build, :manual, ref: 'some-ref', pipeline: pipeline)
+          end
+
+          it 'does not include ability to update build' do
+            expect(policies).not_to include :update_build
+          end
+        end
+
+        context 'when build is not a manual action' do
+          let(:build) do
+            create(:ci_build, ref: 'some-ref', pipeline: pipeline)
+          end
+
+          it 'includes ability to update build' do
+            expect(policies).to include :update_build
+          end
+        end
+      end
+
+      context 'when branch build is assigned to is not protected' do
+        context 'when build is a manual action' do
+          let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
+
+          it 'includes ability to update build' do
+            expect(policies).to include :update_build
+          end
+        end
+
+        context 'when build is not a manual action' do
+          let(:build) { create(:ci_build, pipeline: pipeline) }
+
+          it 'includes ability to update build' do
+            expect(policies).to include :update_build
+          end
+        end
+      end
+    end
   end
 end

spec/policies/environment_policy_spec.rb

+require 'spec_helper'
+
+describe EnvironmentPolicy do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+
+  let(:environment) do
+    create(:environment, :with_review_app, project: project)
+  end
+
+  let(:policies) do
+    described_class.abilities(user, environment).to_set
+  end
+
+  describe '#rules' do
+    context 'when user does not have access to the project' do
+      let(:project) { create(:project, :private) }
+
+      it 'does not include ability to stop environment' do
+        expect(policies).not_to include :stop_environment
+      end
+    end
+
+    context 'when anonymous user has access to the project' do
+      let(:project) { create(:project, :public) }
+
+      it 'does not include ability to stop environment' do
+        expect(policies).not_to include :stop_environment
+      end
+    end
+
+    context 'when team member has access to the project' do
+      let(:project) { create(:project, :public) }
+
+      before do
+        project.add_master(user)
+      end
+
+      context 'when team member has ability to stop environment' do
+        it 'does includes ability to stop environment' do
+          expect(policies).to include :stop_environment
+        end
+      end
+
+      context 'when team member has no ability to stop environment' do
+        before do
+          create(:protected_branch, :no_one_can_push,
+                 name: 'master', project: project)
+        end
+
+        it 'does not include ability to stop environment' do
+          expect(policies).not_to include :stop_environment
+        end
+      end
+    end
+  end
+end

spec/requests/api/jobs_spec.rb

 require 'spec_helper'
 
-describe API::Jobs do
+describe API::Jobs, :api do
+  let!(:project) do
+    create(:project, :repository, public_builds: false)
+  end
+
+  let!(:pipeline) do
+    create(:ci_empty_pipeline, project: project,
+                               sha: project.commit.id,
+                               ref: project.default_branch)
+  end
+
+  let!(:build) { create(:ci_build, pipeline: pipeline) }
+
   let(:user) { create(:user) }
   let(:api_user) { user }
-  let!(:project) { create(:project, :repository, creator: user, public_builds: false) }
-  let!(:developer) { create(:project_member, :developer, user: user, project: project) }
-  let(:reporter) { create(:project_member, :reporter, project: project) }
-  let(:guest) { create(:project_member, :guest, project: project) }
-  let!(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) }
-  let!(:build) { create(:ci_build, pipeline: pipeline) }
+  let(:reporter) { create(:project_member, :reporter, project: project).user }
+  let(:guest) { create(:project_member, :guest, project: project).user }
+
+  before do
+    project.add_developer(user)
+  end
 
   describe 'GET /projects/:id/jobs' do
     let(:query) { Hash.new }
@@ -211,7 +223,7 @@ describe API::Jobs do
   end
 
   describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do
-    let(:api_user) { reporter.user }
+    let(:api_user) { reporter }
     let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) }
 
     before do
@@ -235,7 +247,7 @@ describe API::Jobs do
     end
 
     context 'when logging as guest' do
-      let(:api_user) { guest.user }
+      let(:api_user) { guest }
 
       before do
         get_for_ref
@@ -345,7 +357,7 @@ describe API::Jobs do
       end
 
       context 'user without :update_build permission' do
-        let(:api_user) { reporter.user }
+        let(:api_user) { reporter }
 
         it 'does not cancel job' do
           expect(response).to have_http_status(403)
@@ -379,7 +391,7 @@ describe API::Jobs do
       end
 
       context 'user without :update_build permission' do
-        let(:api_user) { reporter.user }
+        let(:api_user) { reporter }
 
         it 'does not retry job' do
           expect(response).to have_http_status(403)
@@ -455,16 +467,39 @@ describe API::Jobs do
 
   describe 'POST /projects/:id/jobs/:job_id/play' do
     before do
-      post api("/projects/#{project.id}/jobs/#{build.id}/play", user)
+      post api("/projects/#{project.id}/jobs/#{build.id}/play", api_user)
     end
 
     context 'on an playable job' do
       let(:build) { create(:ci_build, :manual, project: project, pipeline: pipeline) }
 
-      it 'plays the job' do
-        expect(response).to have_http_status(200)
-        expect(json_response['user']['id']).to eq(user.id)
-        expect(json_response['id']).to eq(build.id)
+      context 'when user is authorized to trigger a manual action' do
+        it 'plays the job' do
+          expect(response).to have_http_status(200)
+          expect(json_response['user']['id']).to eq(user.id)
+          expect(json_response['id']).to eq(build.id)
+          expect(build.reload).to be_pending
+        end
+      end
+
+      context 'when user is not authorized to trigger a manual action' do
+        context 'when user does not have access to the project' do
+          let(:api_user) { create(:user) }
+
+          it 'does not trigger a manual action' do
+            expect(build.reload).to be_manual
+            expect(response).to have_http_status(404)
+          end
+        end
+
+        context 'when user is not allowed to trigger the manual action' do
+          let(:api_user) { reporter }
+
+          it 'does not trigger a manual action' do
+            expect(build.reload).to be_manual
+            expect(response).to have_http_status(403)
+          end
+        end
       end
     end
 

spec/serializers/build_action_entity_spec.rb

@@ -2,9 +2,10 @@ require 'spec_helper'
 
 describe BuildActionEntity do
   let(:build) { create(:ci_build, name: 'test_build') }
+  let(:request) { double('request') }
 
   let(:entity) do
-    described_class.new(build, request: double)
+    described_class.new(build, request: spy('request'))
   end
 
   describe '#as_json' do

spec/serializers/build_entity_spec.rb

@@ -41,13 +41,37 @@ describe BuildEntity do
     it 'does not contain path to play action' do
       expect(subject).not_to include(:play_path)
     end
+
+    it 'is not a playable job' do
+      expect(subject[:playable]).to be false
+    end
   end
 
   context 'when build is a manual action' do
     let(:build) { create(:ci_build, :manual) }
 
-    it 'contains path to play action' do
-      expect(subject).to include(:play_path)
+    context 'when user is allowed to trigger action' do
+      before do
+        build.project.add_master(user)
+      end
+
+      it 'contains path to play action' do
+        expect(subject).to include(:play_path)
+      end
+
+      it 'is a playable action' do
+        expect(subject[:playable]).to be true
+      end
+    end
+
+    context 'when user is not allowed to trigger action' do
+      it 'does not contain path to play action' do
+        expect(subject).not_to include(:play_path)
+      end
+
+      it 'is not a playable action' do
+        expect(subject[:playable]).to be false
+      end
     end
   end
 end

spec/services/ci/play_build_service_spec.rb

+require 'spec_helper'
+
+describe Ci::PlayBuildService, '#execute', :services do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+  let(:pipeline) { create(:ci_pipeline, project: project) }
+  let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
+
+  let(:service) do
+    described_class.new(project, user)
+  end
+
+  context 'when project does not have repository yet' do
+    let(:project) { create(:empty_project) }
+
+    it 'allows user with master role to play build' do
+      project.add_master(user)
+
+      service.execute(build)
+
+      expect(build.reload).to be_pending
+    end
+
+    it 'does not allow user with developer role to play build' do
+      project.add_developer(user)
+
+      expect { service.execute(build) }
+        .to raise_error Gitlab::Access::AccessDeniedError
+    end
+  end
+
+  context 'when project has repository' do
+    let(:project) { create(:project) }
+
+    it 'allows user with developer role to play a build' do
+      project.add_developer(user)
+
+      service.execute(build)
+
+      expect(build.reload).to be_pending
+    end
+  end
+
+  context 'when build is a playable manual action' do
+    let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
+
+    before do
+      project.add_master(user)
+    end
+
+    it 'enqueues the build' do
+      expect(service.execute(build)).to eq build
+      expect(build.reload).to be_pending
+    end
+
+    it 'reassignes build user correctly' do
+      service.execute(build)
+
+      expect(build.reload.user).to eq user
+    end
+  end
+
+  context 'when build is not a playable manual action' do
+    let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) }
+
+    before do
+      project.add_master(user)
+    end
+
+    it 'duplicates the build' do
+      duplicate = service.execute(build)
+
+      expect(duplicate).not_to eq build
+      expect(duplicate).to be_pending
+    end
+
+    it 'assigns users correctly' do
+      duplicate = service.execute(build)
+
+      expect(build.user).not_to eq user
+      expect(duplicate.user).to eq user
+    end
+  end
+
+  context 'when build is not action' do
+    let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+
+    it 'raises an error' do
+      expect { service.execute(build) }
+        .to raise_error Gitlab::Access::AccessDeniedError
+    end
+  end
+
+  context 'when user does not have ability to trigger action' do
+    before do
+      create(:protected_branch, :no_one_can_push,
+             name: build.ref, project: project)
+    end
+
+    it 'raises an error' do
+      expect { service.execute(build) }
+        .to raise_error Gitlab::Access::AccessDeniedError
+    end
+  end
+end

spec/services/ci/process_pipeline_service_spec.rb

@@ -314,6 +314,13 @@ describe Ci::ProcessPipelineService, '#execute', :services do
     end
 
     context 'when pipeline is promoted sequentially up to the end' do
+      before do
+        # We are using create(:empty_project), and users has to be master in
+        # order to execute manual action when repository does not exist.
+        #
+        project.add_master(user)
+      end
+
       it 'properly processes entire pipeline' do
         process_pipeline
 

spec/services/ci/retry_pipeline_service_spec.rb

@@ -7,7 +7,9 @@ describe Ci::RetryPipelineService, '#execute', :services do
   let(:service) { described_class.new(project, user) }
 
   context 'when user has ability to modify pipeline' do
-    let(:user) { create(:admin) }
+    before do
+      project.add_master(user)
+    end
 
     context 'when there are already retried jobs present' do
       before do
@@ -227,6 +229,46 @@ describe Ci::RetryPipelineService, '#execute', :services do
     end
   end
 
+  context 'when user is not allowed to trigger manual action' do
+    before do
+      project.add_developer(user)
+    end
+
+    context 'when there is a failed manual action present' do
+      before do
+        create_build('test', :failed, 0)
+        create_build('deploy', :failed, 0, when: :manual)
+        create_build('verify', :canceled, 1)
+      end
+
+      it 'does not reprocess manual action' do
+        service.execute(pipeline)
+
+        expect(build('test')).to be_pending
+        expect(build('deploy')).to be_failed
+        expect(build('verify')).to be_created
+        expect(pipeline.reload).to be_running
+      end
+    end
+
+    context 'when there is a failed manual action in later stage' do
+      before do
+        create_build('test', :failed, 0)
+        create_build('deploy', :failed, 1, when: :manual)
+        create_build('verify', :canceled, 2)
+      end
+
+      it 'does not reprocess manual action' do
+        service.execute(pipeline)
+
+        expect(build('test')).to be_pending
+        expect(build('deploy')).to be_failed
+        expect(build('verify')).to be_created
+        expect(pipeline.reload).to be_running
+      end
+    end
+  end
+
   def statuses
     pipeline.reload.statuses
   end

spec/services/ci/stop_environments_service_spec.rb

@@ -55,8 +55,22 @@ describe Ci::StopEnvironmentsService, services: true do
       end
 
       context 'when user does not have permission to stop environment' do
+        context 'when user has no access to manage deployments' do
+          before do
+            project.team << [user, :guest]
+          end
+
+          it 'does not stop environment' do
+            expect_environment_not_stopped_on('master')
+          end
+        end
+      end
+
+      context 'when branch for stop action is protected' do
         before do
-          project.team << [user, :guest]
+          project.add_developer(user)
+          create(:protected_branch, :no_one_can_push,
+                 name: 'master', project: project)
         end
 
         it 'does not stop environment' do

spec/views/projects/environments/terminal.html.haml_spec.rb

+require 'spec_helper'
+
+describe 'projects/environments/terminal' do
+  let!(:environment) { create(:environment, :with_review_app) }
+
+  before do
+    assign(:environment, environment)
+    assign(:project, environment.project)
+
+    allow(view).to receive(:can?).and_return(true)
+  end
+
+  context 'when environment has external URL' do
+    it 'shows external URL button' do
+      environment.update_attribute(:external_url, 'https://gitlab.com')
+
+      render
+
+      expect(rendered).to have_link(nil, href: 'https://gitlab.com')
+    end
+  end
+
+  context 'when environment does not have external URL' do
+    it 'shows external URL button' do
+      environment.update_attribute(:external_url, nil)
+
+      render
+
+      expect(rendered).not_to have_link(nil, href: 'https://gitlab.com')
+    end
+  end
+end