From ee592f0df36aba995d04a969697fb7a064b6ef3c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 11:37:23 +0200
Subject: Improve specs related to CI/CD job environment

---
 spec/models/ci/build_spec.rb | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 8dbcf50ee0c..ae390afbd80 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -902,22 +902,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
 
-- 
cgit v1.2.3


From 5525db8b33bf1f76c4a8023ea4fb571d73e41b0f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 11:52:37 +0200
Subject: Check branch access when user triggers manual action

---
 app/models/ci/build.rb       | 10 +++++++
 spec/models/ci/build_spec.rb | 65 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 8431c5f228c..159b3b2e101 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -115,7 +115,17 @@ module Ci
       commands.present?
     end
 
+    def can_play?(current_user)
+      ::Gitlab::UserAccess
+        .new(current_user, project: project)
+        .can_push_to_branch?(ref)
+    end
+
     def play(current_user)
+      unless can_play?(current_user)
+        raise Gitlab::Access::AccessDeniedError
+      end
+
       # Try to queue a current build
       if self.enqueue
         self.update(user: current_user)
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index ae390afbd80..8124d263fd4 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -925,6 +925,33 @@ describe Ci::Build, :models do
     end
   end
 
+  describe '#can_play?' do
+    before do
+      project.add_developer(user)
+    end
+
+    let(:build) do
+      create(:ci_build, ref: 'some-ref', pipeline: pipeline)
+    end
+
+    context 'when branch build is running for is protected' do
+      before do
+        create(:protected_branch, :no_one_can_push,
+               name: 'some-ref', project: project)
+      end
+
+      it 'indicates that user can not trigger an action' do
+        expect(build.can_play?(user)).to be_falsey
+      end
+    end
+
+    context 'when branch build is running for is not protected' do
+      it 'indicates that user can trigger an action' do
+        expect(build.can_play?(user)).to be_truthy
+      end
+    end
+  end
+
   describe '#play' do
     let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
 
@@ -932,25 +959,39 @@ 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)
+    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
 
-        expect(new_build).to be_pending
-        expect(new_build).to eq(build)
+      it 'raises an error' do
+        expect { build.play(user) }
+          .to raise_error Gitlab::Access::AccessDeniedError
       end
     end
 
-    context 'when build is passed' do
-      before do
-        build.update(status: 'success')
+    context 'when user has ability to trigger manual action' do
+      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
 
-      it 'creates a new build' do
-        new_build = build.play(user)
+      context 'when build is not manual' 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)
+          expect(new_build).to be_pending
+          expect(new_build).not_to eq(build)
+        end
       end
     end
   end
-- 
cgit v1.2.3


From 3dbef510c187606ef12eb9f2aaf51f30ddc3e30d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 12:58:13 +0200
Subject: Expose manual action abilities from a build entity

---
 app/serializers/build_entity.rb       | 10 +++++++---
 spec/serializers/build_entity_spec.rb | 28 ++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/app/serializers/build_entity.rb b/app/serializers/build_entity.rb
index b804d6d0e8a..401a277dadc 100644
--- a/app/serializers/build_entity.rb
+++ b/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: proc { 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? && build.can_play?(request.user)
   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
diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/build_entity_spec.rb
index f76a5cf72d1..897a28b7305 100644
--- a/spec/serializers/build_entity_spec.rb
+++ b/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
-- 
cgit v1.2.3


From 5b6202cce1cf7ad5da1fcbfc4e089194311f205b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 13:08:45 +0200
Subject: Do not show play action if user not allowed to run it

---
 lib/gitlab/ci/status/build/play.rb           |  2 +-
 spec/lib/gitlab/ci/status/build/play_spec.rb | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb
index 3495b8d0448..29d0558a265 100644
--- a/lib/gitlab/ci/status/build/play.rb
+++ b/lib/gitlab/ci/status/build/play.rb
@@ -10,7 +10,7 @@ module Gitlab
           end
 
           def has_action?
-            can?(user, :update_build, subject)
+            can?(user, :update_build, subject) && subject.can_play?(user)
           end
 
           def action_icon
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index 6c97a4fe5ca..48aeb89e11f 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -17,9 +17,17 @@ describe Gitlab::Ci::Status::Build::Play do
 
     describe '#has_action?' do
       context 'when user is allowed to update build' do
-        before { build.project.team << [user, :developer] }
+        context 'when user can push to branch' do
+          before { build.project.add_master(user) }
 
-        it { is_expected.to have_action }
+          it { is_expected.to have_action }
+        end
+
+        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
 
       context 'when user is not allowed to update build' do
-- 
cgit v1.2.3


From f647ad8f6fd076c6adcc3876b5e4a521a49c0ca8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 13:40:04 +0200
Subject: Fix chatops specs with incorrectly defined project

---
 spec/lib/gitlab/chat_commands/command_spec.rb | 14 ++++++++++----
 spec/lib/gitlab/chat_commands/deploy_spec.rb  | 16 ++++++++++------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb
index b6e924d67be..eb4f06b371c 100644
--- a/spec/lib/gitlab/chat_commands/command_spec.rb
+++ b/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
diff --git a/spec/lib/gitlab/chat_commands/deploy_spec.rb b/spec/lib/gitlab/chat_commands/deploy_spec.rb
index b3358a32161..b33389d959e 100644
--- a/spec/lib/gitlab/chat_commands/deploy_spec.rb
+++ b/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
-- 
cgit v1.2.3


From e533d43a8c03a9b47a7016f3fea01a00ca797778 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 14:28:40 +0200
Subject: Fix specs related to new manual actions permissions

---
 spec/factories/environments.rb                     |  4 +-
 .../projects/environments/environment_spec.rb      |  4 ++
 spec/models/environment_spec.rb                    | 53 ++++++++++++++++------
 spec/services/ci/process_pipeline_service_spec.rb  |  7 +++
 4 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/spec/factories/environments.rb b/spec/factories/environments.rb
index 3fbf24b5c7d..f6595751d1e 100644
--- a/spec/factories/environments.rb
+++ b/spec/factories/environments.rb
@@ -18,6 +18,8 @@ FactoryGirl.define do
       # interconnected objects to simulate a review app.
       #
       after(:create) do |environment, evaluator|
+        pipeline = create(:ci_pipeline, project: environment.project)
+
         deployment = create(:deployment,
                             environment: environment,
                             project: environment.project,
@@ -26,7 +28,7 @@ FactoryGirl.define do
 
         teardown_build = create(:ci_build, :manual,
                                 name: "#{deployment.environment.name}:teardown",
-                                pipeline: deployment.deployable.pipeline)
+                                pipeline: pipeline)
 
         deployment.update_column(:on_stop, teardown_build.name)
         environment.update_attribute(:deployments, [deployment])
diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb
index acc3efe04e6..2b7f67eee32 100644
--- a/spec/features/projects/environments/environment_spec.rb
+++ b/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')
 
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 9f0e7fbbe26..9e00f2247e8 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -191,25 +191,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
diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb
index bb98fb37a90..7df6a81b0ab 100644
--- a/spec/services/ci/process_pipeline_service_spec.rb
+++ b/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
 
-- 
cgit v1.2.3


From 6c0fc62ef5c4fa4535174a9f187b9853f0fb90ac Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 15:19:52 +0200
Subject: Take branch access into account when stopping environment

---
 app/models/deployment.rb                           |  4 ++--
 app/models/environment.rb                          |  6 ++++++
 app/services/ci/stop_environments_service.rb       |  8 ++++---
 spec/factories/environments.rb                     |  6 +++++-
 spec/models/environment_spec.rb                    | 25 ++++++++++++++++++++++
 spec/services/ci/stop_environments_service_spec.rb | 16 +++++++++++++-
 6 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index afad001d50f..37adfb4de73 100644
--- a/app/models/deployment.rb
+++ b/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
diff --git a/app/models/environment.rb b/app/models/environment.rb
index bf33010fd21..f8b9a21c08e 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -122,6 +122,12 @@ class Environment < ActiveRecord::Base
     available? && stop_action.present?
   end
 
+  def can_trigger_stop_action?(current_user)
+    return false unless stop_action?
+
+    stop_action.can_play?(current_user)
+  end
+
   def stop_with_action!(current_user)
     return unless available?
 
diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb
index 42c72aba7dd..d1e341bc9b5 100644
--- a/app/services/ci/stop_environments_service.rb
+++ b/app/services/ci/stop_environments_service.rb
@@ -6,9 +6,10 @@ module Ci
       @ref = branch_name
 
       return unless has_ref?
+      return unless can?(current_user, :create_deployment, project)
 
       environments.each do |environment|
-        next unless can?(current_user, :create_deployment, project)
+        next unless environment.can_trigger_stop_action?(current_user)
 
         environment.stop_with_action!(current_user)
       end
@@ -21,8 +22,9 @@ module Ci
     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
diff --git a/spec/factories/environments.rb b/spec/factories/environments.rb
index f6595751d1e..d8d699fb3aa 100644
--- a/spec/factories/environments.rb
+++ b/spec/factories/environments.rb
@@ -20,14 +20,18 @@ FactoryGirl.define do
       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",
+                                name: "#{environment.name}:teardown",
                                 pipeline: pipeline)
 
         deployment.update_column(:on_stop, teardown_build.name)
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 9e00f2247e8..fb7cee47ae8 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -155,6 +155,31 @@ describe Environment, models: true do
     end
   end
 
+  describe '#can_trigger_stop_action?' do
+    let(:user) { create(:user) }
+    let(:project) { create(:project) }
+
+    let(:environment) do
+      create(:environment, :with_review_app, project: project)
+    end
+
+    context 'when user can trigger stop action' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'returns value that evaluates to true' do
+        expect(environment.can_trigger_stop_action?(user)).to be_truthy
+      end
+    end
+
+    context 'when user is not allowed to trigger stop action' do
+      it 'returns value that evaluates to false' do
+        expect(environment.can_trigger_stop_action?(user)).to be_falsey
+      end
+    end
+  end
+
   describe '#stop_with_action!' do
     let(:user) { create(:admin) }
 
diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb
index 32c72a9cf5e..98044ad232e 100644
--- a/spec/services/ci/stop_environments_service_spec.rb
+++ b/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
-- 
cgit v1.2.3


From d96e90734783a83ca4d87b961699072712077d9f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 15:24:48 +0200
Subject: Fix specs for build status factory and manual status

---
 spec/lib/gitlab/ci/status/build/factory_spec.rb | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index e648a3ac3a2..2ab67127b1e 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -218,9 +218,24 @@ describe Gitlab::Ci::Status::Build::Factory do
         expect(status.favicon).to eq 'favicon_status_manual'
         expect(status.label).to eq '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
-- 
cgit v1.2.3


From 3e55e0742218fe20b269dfbc4f44d3798e4b0eb6 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 15:32:23 +0200
Subject: Check if user can trigger manual action in the UI

---
 app/views/projects/ci/builds/_build.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml
index aeed293a724..ceec36e2440 100644
--- a/app/views/projects/ci/builds/_build.html.haml
+++ b/app/views/projects/ci/builds/_build.html.haml
@@ -101,7 +101,7 @@
           = link_to cancel_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Cancel', class: 'btn btn-build' do
             = icon('remove', class: 'cred')
         - elsif allow_retry
-          - if build.playable? && !admin
+          - if build.playable? && !admin && build.can_play?(current_user)
             = link_to play_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Play', class: 'btn btn-build' do
               = custom_icon('icon_play')
           - elsif build.retryable?
-- 
cgit v1.2.3


From 7bcca2284b09e18438e6163c6ead72e10fdd2f57 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 6 Apr 2017 17:16:19 +0200
Subject: Check branch permission in manual action entity

---
 app/serializers/build_action_entity.rb | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/app/serializers/build_action_entity.rb b/app/serializers/build_action_entity.rb
index 184b4b7a681..3d12c64b88a 100644
--- a/app/serializers/build_action_entity.rb
+++ b/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? && build.can_play?(request.user)
+  end
 end
-- 
cgit v1.2.3


From b09465f38d66d7ff6074843177bcdb7d72caf07f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 12 Apr 2017 11:26:18 +0200
Subject: Implement new rule for manual actions in policies

---
 app/policies/ci/build_policy.rb       | 14 +++++++++
 spec/policies/ci/build_policy_spec.rb | 53 +++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb
index 8b25332b73c..0522cbdb331 100644
--- a/app/policies/ci/build_policy.rb
+++ b/app/policies/ci/build_policy.rb
@@ -8,6 +8,20 @@ module Ci
       %w[read create update admin].each do |rule|
         cannot! :"#{rule}_commit_status" unless can? :"#{rule}_build"
       end
+
+      can! :play_build if can_play_action?
+    end
+
+    private
+
+    alias_method :build, :subject
+
+    def can_play_action?
+      return false unless build.playable?
+
+      ::Gitlab::UserAccess
+        .new(user, project: build.project)
+        .can_push_to_branch?(build.ref)
     end
   end
 end
diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
index 0f280f32eac..e4693cdcef0 100644
--- a/spec/policies/ci/build_policy_spec.rb
+++ b/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 play build' do
+            expect(policies).not_to include :play_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 'does not include ability to play build' do
+            expect(policies).not_to include :play_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 play build' do
+            expect(policies).to include :play_build
+          end
+        end
+
+        context 'when build is not a manual action' do
+          let(:build) { create(:ci_build,  pipeline: pipeline) }
+
+          it 'does not include ability to play build' do
+            expect(policies).not_to include :play_build
+          end
+        end
+      end
+    end
   end
 end
-- 
cgit v1.2.3


From 6c6bc400d1d8a96f6e443788cd0b2c14addd88e3 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 12 Apr 2017 11:46:24 +0200
Subject: Move code for playing an action to separate service

---
 app/models/ci/build.rb                | 15 +++------------
 app/policies/ci/build_policy.rb       |  2 +-
 app/services/ci/play_build_service.rb | 17 +++++++++++++++++
 3 files changed, 21 insertions(+), 13 deletions(-)
 create mode 100644 app/services/ci/play_build_service.rb

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 159b3b2e101..9edc4cd96b9 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -122,18 +122,9 @@ module Ci
     end
 
     def play(current_user)
-      unless can_play?(current_user)
-        raise Gitlab::Access::AccessDeniedError
-      end
-
-      # 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?
diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb
index 0522cbdb331..2c39d31488f 100644
--- a/app/policies/ci/build_policy.rb
+++ b/app/policies/ci/build_policy.rb
@@ -17,7 +17,7 @@ module Ci
     alias_method :build, :subject
 
     def can_play_action?
-      return false unless build.playable?
+      return false unless build.action?
 
       ::Gitlab::UserAccess
         .new(user, project: build.project)
diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb
new file mode 100644
index 00000000000..c9ed45408f2
--- /dev/null
+++ b/app/services/ci/play_build_service.rb
@@ -0,0 +1,17 @@
+module Ci
+  class PlayBuildService < ::BaseService
+    def execute(build)
+      unless can?(current_user, :play_build, build)
+        raise Gitlab::Access::AccessDeniedError
+      end
+
+      # Try to enqueue thebuild, 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
-- 
cgit v1.2.3


From 7fc6b5b6ff23e2faba7f06a1362ada31f6f3436a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 12 Apr 2017 12:19:39 +0200
Subject: Do not inherit build policy in pipeline policy

---
 app/policies/base_policy.rb        | 4 ++++
 app/policies/ci/pipeline_policy.rb | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb
index 8890409d056..623424c63e0 100644
--- a/app/policies/base_policy.rb
+++ b/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
diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb
index 3d2eef1c50c..10aa2d3e72a 100644
--- a/app/policies/ci/pipeline_policy.rb
+++ b/app/policies/ci/pipeline_policy.rb
@@ -1,4 +1,7 @@
 module Ci
-  class PipelinePolicy < BuildPolicy
+  class PipelinePolicy < BasePolicy
+    def rules
+      delegate! @subject.project
+    end
   end
 end
-- 
cgit v1.2.3


From 55aa727eff50a9472405b302645abb54f28bdba0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 12 Apr 2017 13:36:35 +0200
Subject: Extract build play specs and extend test examples

---
 spec/models/ci/build_spec.rb                |  36 +---------
 spec/services/ci/play_build_service_spec.rb | 105 ++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+), 34 deletions(-)
 create mode 100644 spec/services/ci/play_build_service_spec.rb

diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 8124d263fd4..09e9f5bd8ba 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -959,40 +959,8 @@ describe Ci::Build, :models do
       project.add_developer(user)
     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 { build.play(user) }
-          .to raise_error Gitlab::Access::AccessDeniedError
-      end
-    end
-
-    context 'when user has ability to trigger manual action' do
-      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 not manual' 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
-      end
+    it 'enqueues the build' do
+      expect(build.play(user)).to be_pending
     end
   end
 
diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb
new file mode 100644
index 00000000000..d6f9fa42045
--- /dev/null
+++ b/spec/services/ci/play_build_service_spec.rb
@@ -0,0 +1,105 @@
+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
-- 
cgit v1.2.3


From 2aa211fa69ffd02ba11757e06e19d34f6ca51865 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 12 Apr 2017 13:48:43 +0200
Subject: Use build policy to determine if user can play build

---
 app/models/ci/build.rb                        |  6 ------
 app/models/environment.rb                     |  6 ------
 app/serializers/build_action_entity.rb        |  2 +-
 app/serializers/build_entity.rb               |  2 +-
 app/services/ci/stop_environments_service.rb  |  3 ++-
 app/views/projects/ci/builds/_build.html.haml |  2 +-
 lib/gitlab/ci/status/build/play.rb            |  2 +-
 spec/models/ci/build_spec.rb                  | 27 ---------------------------
 spec/models/environment_spec.rb               | 25 -------------------------
 9 files changed, 6 insertions(+), 69 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 9edc4cd96b9..b3acb25b9ce 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -115,12 +115,6 @@ module Ci
       commands.present?
     end
 
-    def can_play?(current_user)
-      ::Gitlab::UserAccess
-        .new(current_user, project: project)
-        .can_push_to_branch?(ref)
-    end
-
     def play(current_user)
       Ci::PlayBuildService
         .new(project, current_user)
diff --git a/app/models/environment.rb b/app/models/environment.rb
index f8b9a21c08e..bf33010fd21 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -122,12 +122,6 @@ class Environment < ActiveRecord::Base
     available? && stop_action.present?
   end
 
-  def can_trigger_stop_action?(current_user)
-    return false unless stop_action?
-
-    stop_action.can_play?(current_user)
-  end
-
   def stop_with_action!(current_user)
     return unless available?
 
diff --git a/app/serializers/build_action_entity.rb b/app/serializers/build_action_entity.rb
index 3d12c64b88a..0bb7e561073 100644
--- a/app/serializers/build_action_entity.rb
+++ b/app/serializers/build_action_entity.rb
@@ -19,6 +19,6 @@ class BuildActionEntity < Grape::Entity
   alias_method :build, :object
 
   def playable?
-    build.playable? && build.can_play?(request.user)
+    can?(request.user, :play_build, build) && build.playable?
   end
 end
diff --git a/app/serializers/build_entity.rb b/app/serializers/build_entity.rb
index 401a277dadc..f301900c43c 100644
--- a/app/serializers/build_entity.rb
+++ b/app/serializers/build_entity.rb
@@ -26,7 +26,7 @@ class BuildEntity < Grape::Entity
   alias_method :build, :object
 
   def playable?
-    build.playable? && build.can_play?(request.user)
+    can?(request.user, :play_build, build) && build.playable?
   end
 
   def detailed_status
diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb
index d1e341bc9b5..bd9735fc0ac 100644
--- a/app/services/ci/stop_environments_service.rb
+++ b/app/services/ci/stop_environments_service.rb
@@ -9,7 +9,8 @@ module Ci
       return unless can?(current_user, :create_deployment, project)
 
       environments.each do |environment|
-        next unless environment.can_trigger_stop_action?(current_user)
+        next unless environment.stop_action?
+        next unless can?(current_user, :play_build, environment.stop_action)
 
         environment.stop_with_action!(current_user)
       end
diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml
index ceec36e2440..769640c4842 100644
--- a/app/views/projects/ci/builds/_build.html.haml
+++ b/app/views/projects/ci/builds/_build.html.haml
@@ -101,7 +101,7 @@
           = link_to cancel_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Cancel', class: 'btn btn-build' do
             = icon('remove', class: 'cred')
         - elsif allow_retry
-          - if build.playable? && !admin && build.can_play?(current_user)
+          - if build.playable? && !admin && can?(current_user, :play_build, build)
             = link_to play_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Play', class: 'btn btn-build' do
               = custom_icon('icon_play')
           - elsif build.retryable?
diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb
index 29d0558a265..4c893f62925 100644
--- a/lib/gitlab/ci/status/build/play.rb
+++ b/lib/gitlab/ci/status/build/play.rb
@@ -10,7 +10,7 @@ module Gitlab
           end
 
           def has_action?
-            can?(user, :update_build, subject) && subject.can_play?(user)
+            can?(user, :play_build, subject)
           end
 
           def action_icon
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 09e9f5bd8ba..cdceca975e5 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -925,33 +925,6 @@ describe Ci::Build, :models do
     end
   end
 
-  describe '#can_play?' do
-    before do
-      project.add_developer(user)
-    end
-
-    let(:build) do
-      create(:ci_build, ref: 'some-ref', pipeline: pipeline)
-    end
-
-    context 'when branch build is running for is protected' do
-      before do
-        create(:protected_branch, :no_one_can_push,
-               name: 'some-ref', project: project)
-      end
-
-      it 'indicates that user can not trigger an action' do
-        expect(build.can_play?(user)).to be_falsey
-      end
-    end
-
-    context 'when branch build is running for is not protected' do
-      it 'indicates that user can trigger an action' do
-        expect(build.can_play?(user)).to be_truthy
-      end
-    end
-  end
-
   describe '#play' do
     let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
 
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index fb7cee47ae8..9e00f2247e8 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -155,31 +155,6 @@ describe Environment, models: true do
     end
   end
 
-  describe '#can_trigger_stop_action?' do
-    let(:user) { create(:user) }
-    let(:project) { create(:project) }
-
-    let(:environment) do
-      create(:environment, :with_review_app, project: project)
-    end
-
-    context 'when user can trigger stop action' do
-      before do
-        project.add_developer(user)
-      end
-
-      it 'returns value that evaluates to true' do
-        expect(environment.can_trigger_stop_action?(user)).to be_truthy
-      end
-    end
-
-    context 'when user is not allowed to trigger stop action' do
-      it 'returns value that evaluates to false' do
-        expect(environment.can_trigger_stop_action?(user)).to be_falsey
-      end
-    end
-  end
-
   describe '#stop_with_action!' do
     let(:user) { create(:admin) }
 
-- 
cgit v1.2.3


From 0b75541559c0550bd3e195ca5aca55016fa614ef Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 12 Apr 2017 15:22:32 +0200
Subject: Fix manual action entity specs

---
 spec/serializers/build_action_entity_spec.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/serializers/build_action_entity_spec.rb b/spec/serializers/build_action_entity_spec.rb
index 54ac17447b1..059deba5416 100644
--- a/spec/serializers/build_action_entity_spec.rb
+++ b/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
-- 
cgit v1.2.3


From 57f8f2d7ff851fc4f5d1c81a28a023855f1985b7 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 13 Apr 2017 15:06:45 +0200
Subject: Fix detailed build status specs for manual actions

---
 spec/lib/gitlab/ci/status/build/play_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index 48aeb89e11f..95f1388a9b9 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -12,7 +12,7 @@ describe Gitlab::Ci::Status::Build::Play do
 
   describe 'action details' do
     let(:user) { create(:user) }
-    let(:build) { create(:ci_build) }
+    let(:build) { create(:ci_build, :manual) }
     let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
 
     describe '#has_action?' do
-- 
cgit v1.2.3


From 52bfc0efa95f991f17618aa049f799f4e44e13ac Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 22 Apr 2017 21:02:31 +0200
Subject: Fix typo in CI/CD build partial view

---
 app/views/projects/ci/builds/_build.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml
index 3e1c8f25dea..2227d36eed2 100644
--- a/app/views/projects/ci/builds/_build.html.haml
+++ b/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 && can?(current_user, :play_build, jop)
+          - if job.playable? && !admin && can?(current_user, :play_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?
-- 
cgit v1.2.3


From 6baaa8a98e10ff93ba3f481052bb68fdafb6e2c1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 1 May 2017 13:38:57 +0200
Subject: Add new ability check for stopping environment

---
 app/policies/environment_policy.rb           | 13 ++++++-
 app/services/ci/stop_environments_service.rb |  9 +----
 spec/policies/environment_policy_spec.rb     | 57 ++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 8 deletions(-)
 create mode 100644 spec/policies/environment_policy_spec.rb

diff --git a/app/policies/environment_policy.rb b/app/policies/environment_policy.rb
index f4219569161..0b976e5664d 100644
--- a/app/policies/environment_policy.rb
+++ b/app/policies/environment_policy.rb
@@ -1,5 +1,16 @@
 class EnvironmentPolicy < BasePolicy
+
+  alias_method :environment, :subject
+
   def rules
-    delegate! @subject.project
+    delegate! environment.project
+
+    if environment.stop_action?
+      delegate! environment.stop_action
+    end
+
+    if can?(:create_deployment) && can?(:play_build)
+      can! :stop_environment
+    end
   end
 end
diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb
index bd9735fc0ac..43c9a065fcf 100644
--- a/app/services/ci/stop_environments_service.rb
+++ b/app/services/ci/stop_environments_service.rb
@@ -5,12 +5,11 @@ module Ci
     def execute(branch_name)
       @ref = branch_name
 
-      return unless has_ref?
-      return unless can?(current_user, :create_deployment, project)
+      return unless @ref.present?
 
       environments.each do |environment|
         next unless environment.stop_action?
-        next unless can?(current_user, :play_build, environment.stop_action)
+        next unless can?(current_user, :stop_environment, environment)
 
         environment.stop_with_action!(current_user)
       end
@@ -18,10 +17,6 @@ module Ci
 
     private
 
-    def has_ref?
-      @ref.present?
-    end
-
     def environments
       @environments ||= EnvironmentsFinder
         .new(project, current_user, ref: @ref, recently_updated: true)
diff --git a/spec/policies/environment_policy_spec.rb b/spec/policies/environment_policy_spec.rb
new file mode 100644
index 00000000000..f43caffa946
--- /dev/null
+++ b/spec/policies/environment_policy_spec.rb
@@ -0,0 +1,57 @@
+require 'spec_helper'
+
+describe Ci::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
-- 
cgit v1.2.3


From 4f2cc5951f3d2862a44fe124ef9a879162028e62 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 1 May 2017 14:29:20 +0200
Subject: Extend action tooltop to show info about abilities

---
 lib/gitlab/ci/status/build/play.rb              |  6 ++-
 spec/lib/gitlab/ci/status/build/factory_spec.rb |  2 +-
 spec/lib/gitlab/ci/status/build/play_spec.rb    | 65 ++++++++++++++-----------
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb
index 4c893f62925..8130c7d1c90 100644
--- a/lib/gitlab/ci/status/build/play.rb
+++ b/lib/gitlab/ci/status/build/play.rb
@@ -6,7 +6,11 @@ module Gitlab
           include Status::Extended
 
           def label
-            'manual play action'
+            if has_action?
+              'manual play action'
+            else
+              'manual play action (not allowed)'
+            end
           end
 
           def has_action?
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index 2ab67127b1e..2de00c28945 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -216,7 +216,7 @@ 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.action_path).to include 'play'
       end
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index 95f1388a9b9..abefdbe7c66 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -1,51 +1,58 @@
 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' }
-  end
-
-  describe 'action details' do
-    let(:user) { create(:user) }
-    let(:build) { create(:ci_build, :manual) }
-    let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
+  context 'when user is allowed to update build' do
+    context 'when user can push to branch' do
+      before { build.project.add_master(user) }
 
-    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) }
+      describe '#has_action?' do
+        it { is_expected.to have_action }
+      end
 
-          it { is_expected.to have_action }
+      describe '#label' do
+        it 'has a label that says it is a manual action' do
+          expect(subject.label).to eq 'manual play action'
         end
+      end
+    end
 
-        context 'when user can not push to the branch' do
-          before { build.project.add_developer(user) }
+    context 'when user can not push to the branch' do
+      before { build.project.add_developer(user) }
 
-          it { is_expected.not_to have_action }
-        end
+      describe 'has_action?' do
+        it { is_expected.not_to have_action }
       end
 
-      context 'when user is not allowed to update build' do
-        it { is_expected.not_to have_action }
+      describe '#label' do
+        it 'has a label that says user is not allowed to play it' do
+          expect(subject.label).to eq 'manual play action (not allowed)'
+        end
       end
     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
+    describe '#has_action?' 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
-- 
cgit v1.2.3


From e5df86f54d6165e0b3871678dd987956dddfa061 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 1 May 2017 14:30:50 +0200
Subject: Fix Rubocop offense in environments policy class

---
 app/policies/environment_policy.rb | 1 -
 spec/models/ci/build_spec.rb       | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/policies/environment_policy.rb b/app/policies/environment_policy.rb
index 0b976e5664d..c0c9bbf2d9e 100644
--- a/app/policies/environment_policy.rb
+++ b/app/policies/environment_policy.rb
@@ -1,5 +1,4 @@
 class EnvironmentPolicy < BasePolicy
-
   alias_method :environment, :subject
 
   def rules
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index b2f9a61f7f3..5231ce28c9d 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -911,7 +911,7 @@ describe Ci::Build, :models do
       it { is_expected.to eq(environment) }
     end
 
-    context 'when  referenced with a variable' do
+    context 'when referenced with a variable' do
       let(:build) do
         create(:ci_build, pipeline: pipeline, environment: "foo-$CI_COMMIT_REF_NAME")
       end
-- 
cgit v1.2.3


From 02fc1846832593b2e803a83446690183715f5df1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 2 May 2017 10:29:09 +0200
Subject: Improve specs for jobs API regarding manual actions

---
 spec/requests/api/jobs_spec.rb | 67 +++++++++++++++++++++++++++++++-----------
 1 file changed, 50 insertions(+), 17 deletions(-)

diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index d8a56c02a63..e5e5872dc1f 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -1,16 +1,26 @@
 require 'spec_helper'
 
-describe API::Jobs, api: true do
-  include ApiHelpers
+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 }
@@ -213,7 +223,7 @@ describe API::Jobs, api: true 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
@@ -237,7 +247,7 @@ describe API::Jobs, api: true do
     end
 
     context 'when logging as guest' do
-      let(:api_user) { guest.user }
+      let(:api_user) { guest }
 
       before do
         get_for_ref
@@ -347,7 +357,7 @@ describe API::Jobs, api: true 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)
@@ -381,7 +391,7 @@ describe API::Jobs, api: true 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)
@@ -457,16 +467,39 @@ describe API::Jobs, api: true 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
 
-- 
cgit v1.2.3


From f5441c93203cd5c8ccd208c00bee59432facae7f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 2 May 2017 10:35:51 +0200
Subject: Document protected manual actions feature

---
 doc/ci/yaml/README.md | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index ad3ebd144df..0cab3270d94 100644
--- a/doc/ci/yaml/README.md
+++ b/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,9 @@ 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 do inherit permissions of protected branches. 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
 
-- 
cgit v1.2.3


From c9def85844531ffdd2984707a1bc8cbca18f6742 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 2 May 2017 10:37:50 +0200
Subject: Add Changelog entry for protected manual actions

---
 .../feature-gb-manual-actions-protected-branches-permissions.yml      | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 changelogs/unreleased/feature-gb-manual-actions-protected-branches-permissions.yml

diff --git a/changelogs/unreleased/feature-gb-manual-actions-protected-branches-permissions.yml b/changelogs/unreleased/feature-gb-manual-actions-protected-branches-permissions.yml
new file mode 100644
index 00000000000..6f8e80e7d64
--- /dev/null
+++ b/changelogs/unreleased/feature-gb-manual-actions-protected-branches-permissions.yml
@@ -0,0 +1,4 @@
+---
+title: Implement protected manual actions
+merge_request: 10494
+author:
-- 
cgit v1.2.3


From 4ddce55315167b362056616151244f137c81945f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 2 May 2017 11:27:10 +0200
Subject: Fix environment policy class name in specs

---
 spec/policies/environment_policy_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/policies/environment_policy_spec.rb b/spec/policies/environment_policy_spec.rb
index f43caffa946..0e15beaa5e8 100644
--- a/spec/policies/environment_policy_spec.rb
+++ b/spec/policies/environment_policy_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Ci::EnvironmentPolicy do
+describe EnvironmentPolicy do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
 
-- 
cgit v1.2.3


From eb45582f55cae42acc41fa228f4797b4067c01dc Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 2 May 2017 12:25:30 +0200
Subject: Fix builds controller spec related to protected actions

---
 spec/controllers/projects/builds_controller_spec.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb
index 22193eac672..3bf03c88ff5 100644
--- a/spec/controllers/projects/builds_controller_spec.rb
+++ b/spec/controllers/projects/builds_controller_spec.rb
@@ -260,6 +260,8 @@ describe Projects::BuildsController do
   end
 
   describe 'POST play' do
+    let(:project) { create(:project, :public) }
+
     before do
       project.add_developer(user)
       sign_in(user)
-- 
cgit v1.2.3


From 72307940bb04d29dc751adb5e76330d2ce45e9ce Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 3 May 2017 12:32:39 +0200
Subject: Improve code style related to protected actions

---
 app/serializers/build_action_entity.rb              | 2 +-
 app/serializers/build_entity.rb                     | 4 ++--
 app/serializers/pipeline_entity.rb                  | 6 +++---
 app/services/ci/play_build_service.rb               | 2 +-
 spec/controllers/projects/builds_controller_spec.rb | 4 +---
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/app/serializers/build_action_entity.rb b/app/serializers/build_action_entity.rb
index 0bb7e561073..3eff724ae91 100644
--- a/app/serializers/build_action_entity.rb
+++ b/app/serializers/build_action_entity.rb
@@ -19,6 +19,6 @@ class BuildActionEntity < Grape::Entity
   alias_method :build, :object
 
   def playable?
-    can?(request.user, :play_build, build) && build.playable?
+    build.playable? && can?(request.user, :play_build, build)
   end
 end
diff --git a/app/serializers/build_entity.rb b/app/serializers/build_entity.rb
index f301900c43c..10ba7c90c10 100644
--- a/app/serializers/build_entity.rb
+++ b/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: proc { playable? } do |build|
+  expose :play_path, if: -> (*) { playable? } do |build|
     path_to(:play_namespace_project_build, build)
   end
 
@@ -26,7 +26,7 @@ class BuildEntity < Grape::Entity
   alias_method :build, :object
 
   def playable?
-    can?(request.user, :play_build, build) && build.playable?
+    build.playable? && can?(request.user, :play_build, build)
   end
 
   def detailed_status
diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb
index ad8b4d43e8f..7eb7aac72eb 100644
--- a/app/serializers/pipeline_entity.rb
+++ b/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)
diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb
index c9ed45408f2..5b3ff1ced38 100644
--- a/app/services/ci/play_build_service.rb
+++ b/app/services/ci/play_build_service.rb
@@ -5,7 +5,7 @@ module Ci
         raise Gitlab::Access::AccessDeniedError
       end
 
-      # Try to enqueue thebuild, otherwise create a duplicate.
+      # Try to enqueue the build, otherwise create a duplicate.
       #
       if build.enqueue
         build.tap { |action| action.update(user: current_user) }
diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb
index 3bf03c88ff5..3ce23c17cdc 100644
--- a/spec/controllers/projects/builds_controller_spec.rb
+++ b/spec/controllers/projects/builds_controller_spec.rb
@@ -260,10 +260,8 @@ describe Projects::BuildsController do
   end
 
   describe 'POST play' do
-    let(:project) { create(:project, :public) }
-
     before do
-      project.add_developer(user)
+      project.add_master(user)
       sign_in(user)
 
       post_play
-- 
cgit v1.2.3


From 88ace4a7ad0809d64f688dc1ee6fc8ea6cda9045 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 3 May 2017 12:35:13 +0200
Subject: Rephrase documentation for protected actions feature

---
 doc/ci/yaml/README.md | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 0cab3270d94..16308a957cb 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -580,9 +580,10 @@ Optional manual actions have `allow_failure: true` set by default.
 
 **Statuses of optional actions do not contribute to overall pipeline status.**
 
-**Manual actions do inherit permissions of protected branches. 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.**
+**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
 
-- 
cgit v1.2.3


From 83154f21542ec04076d20ce9c4a8997d55fc5f43 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 3 May 2017 15:42:29 +0200
Subject: Improve environment policy class

---
 app/policies/environment_policy.rb | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/app/policies/environment_policy.rb b/app/policies/environment_policy.rb
index c0c9bbf2d9e..cc94d4a7e05 100644
--- a/app/policies/environment_policy.rb
+++ b/app/policies/environment_policy.rb
@@ -4,12 +4,14 @@ class EnvironmentPolicy < BasePolicy
   def rules
     delegate! environment.project
 
-    if environment.stop_action?
-      delegate! environment.stop_action
+    if can?(:create_deployment) && environment.stop_action?
+      can! :stop_environment if can_play_stop_action?
     end
+  end
 
-    if can?(:create_deployment) && can?(:play_build)
-      can! :stop_environment
-    end
+  private
+
+  def can_play_stop_action?
+    Ability.allowed?(user, :play_build, environment.stop_action)
   end
 end
-- 
cgit v1.2.3


From 936367538043854c7b093b71ca315b8e469c55a4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 5 May 2017 12:25:24 +0200
Subject: Use update build policy instead of new play policy

---
 app/policies/ci/build_policy.rb               | 12 +++++++-----
 app/policies/environment_policy.rb            |  2 +-
 app/serializers/build_action_entity.rb        |  2 +-
 app/serializers/build_entity.rb               |  2 +-
 app/services/ci/play_build_service.rb         |  2 +-
 app/views/projects/ci/builds/_build.html.haml |  2 +-
 lib/gitlab/ci/status/build/play.rb            |  2 +-
 spec/policies/ci/build_policy_spec.rb         | 18 +++++++++---------
 8 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb
index 2c39d31488f..d4af4490608 100644
--- a/app/policies/ci/build_policy.rb
+++ b/app/policies/ci/build_policy.rb
@@ -1,5 +1,7 @@
 module Ci
   class BuildPolicy < CommitStatusPolicy
+    alias_method :build, :subject
+
     def rules
       super
 
@@ -9,17 +11,17 @@ module Ci
         cannot! :"#{rule}_commit_status" unless can? :"#{rule}_build"
       end
 
-      can! :play_build if can_play_action?
+      if can?(:update_build) && protected_action?
+        cannot! :update_build
+      end
     end
 
     private
 
-    alias_method :build, :subject
-
-    def can_play_action?
+    def protected_action?
       return false unless build.action?
 
-      ::Gitlab::UserAccess
+      !::Gitlab::UserAccess
         .new(user, project: build.project)
         .can_push_to_branch?(build.ref)
     end
diff --git a/app/policies/environment_policy.rb b/app/policies/environment_policy.rb
index cc94d4a7e05..2fa15e64562 100644
--- a/app/policies/environment_policy.rb
+++ b/app/policies/environment_policy.rb
@@ -12,6 +12,6 @@ class EnvironmentPolicy < BasePolicy
   private
 
   def can_play_stop_action?
-    Ability.allowed?(user, :play_build, environment.stop_action)
+    Ability.allowed?(user, :update_build, environment.stop_action)
   end
 end
diff --git a/app/serializers/build_action_entity.rb b/app/serializers/build_action_entity.rb
index 3eff724ae91..75dda1af709 100644
--- a/app/serializers/build_action_entity.rb
+++ b/app/serializers/build_action_entity.rb
@@ -19,6 +19,6 @@ class BuildActionEntity < Grape::Entity
   alias_method :build, :object
 
   def playable?
-    build.playable? && can?(request.user, :play_build, build)
+    build.playable? && can?(request.user, :update_build, build)
   end
 end
diff --git a/app/serializers/build_entity.rb b/app/serializers/build_entity.rb
index 10ba7c90c10..1380b347d8e 100644
--- a/app/serializers/build_entity.rb
+++ b/app/serializers/build_entity.rb
@@ -26,7 +26,7 @@ class BuildEntity < Grape::Entity
   alias_method :build, :object
 
   def playable?
-    build.playable? && can?(request.user, :play_build, build)
+    build.playable? && can?(request.user, :update_build, build)
   end
 
   def detailed_status
diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb
index 5b3ff1ced38..e24f48c2d16 100644
--- a/app/services/ci/play_build_service.rb
+++ b/app/services/ci/play_build_service.rb
@@ -1,7 +1,7 @@
 module Ci
   class PlayBuildService < ::BaseService
     def execute(build)
-      unless can?(current_user, :play_build, build)
+      unless can?(current_user, :update_build, build)
         raise Gitlab::Access::AccessDeniedError
       end
 
diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml
index 2227d36eed2..c0019996176 100644
--- a/app/views/projects/ci/builds/_build.html.haml
+++ b/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 && can?(current_user, :play_build, job)
+          - 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?
diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb
index 8130c7d1c90..fae34a2f927 100644
--- a/lib/gitlab/ci/status/build/play.rb
+++ b/lib/gitlab/ci/status/build/play.rb
@@ -14,7 +14,7 @@ module Gitlab
           end
 
           def has_action?
-            can?(user, :play_build, subject)
+            can?(user, :update_build, subject)
           end
 
           def action_icon
diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
index e4693cdcef0..3f4ce222b60 100644
--- a/spec/policies/ci/build_policy_spec.rb
+++ b/spec/policies/ci/build_policy_spec.rb
@@ -108,8 +108,8 @@ describe Ci::BuildPolicy, :models do
             create(:ci_build, :manual, ref: 'some-ref', pipeline: pipeline)
           end
 
-          it 'does not include ability to play build' do
-            expect(policies).not_to include :play_build
+          it 'does not include ability to update build' do
+            expect(policies).not_to include :update_build
           end
         end
 
@@ -118,8 +118,8 @@ describe Ci::BuildPolicy, :models do
             create(:ci_build, ref: 'some-ref', pipeline: pipeline)
           end
 
-          it 'does not include ability to play build' do
-            expect(policies).not_to include :play_build
+          it 'includes ability to update build' do
+            expect(policies).to include :update_build
           end
         end
       end
@@ -128,16 +128,16 @@ describe Ci::BuildPolicy, :models do
         context 'when build is a manual action' do
           let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
 
-          it 'includes ability to play build' do
-            expect(policies).to include :play_build
+          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) }
+          let(:build) { create(:ci_build, pipeline: pipeline) }
 
-          it 'does not include ability to play build' do
-            expect(policies).not_to include :play_build
+          it 'includes ability to update build' do
+            expect(policies).to include :update_build
           end
         end
       end
-- 
cgit v1.2.3


From 61dd92aaff822759941bb224de9f45bfc5f7cc9b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 5 May 2017 13:24:07 +0200
Subject: Authorize build update on per object basis

---
 app/controllers/projects/application_controller.rb |  8 +++++---
 app/controllers/projects/builds_controller.rb      | 23 ++++++++++++++++++----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 89f1128ec36..afed0ac05a0 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -55,13 +55,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
diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb
index e24fc45d166..d97bc93f8dc 100644
--- a/app/controllers/projects/builds_controller.rb
+++ b/app/controllers/projects/builds_controller.rb
@@ -1,7 +1,11 @@
 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,14 @@ 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_by!(id: params[:id])
+      .present(current_user: current_user)
   end
 
   def build_path(build)
-- 
cgit v1.2.3


From 3264e09c6fbe07831db74b83d6a6620d9f8f47d9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 5 May 2017 13:25:48 +0200
Subject: Require build to be present in the controller

---
 app/controllers/projects/builds_controller.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb
index d97bc93f8dc..0fd35bcb790 100644
--- a/app/controllers/projects/builds_controller.rb
+++ b/app/controllers/projects/builds_controller.rb
@@ -121,8 +121,7 @@ class Projects::BuildsController < Projects::ApplicationController
   end
 
   def build
-    @build ||= project.builds
-      .find_by!(id: params[:id])
+    @build ||= project.builds.find(params[:id])
       .present(current_user: current_user)
   end
 
-- 
cgit v1.2.3


From 53219857dd9f97516c6f24f6efb4f405998d9ff2 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 5 May 2017 13:56:07 +0200
Subject: Check ability to update build on the API resource

---
 lib/api/jobs.rb      |  9 +++++++--
 lib/api/v3/builds.rb | 10 +++++++---
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb
index 288b03d940c..0223957fde1 100644
--- a/lib/api/jobs.rb
+++ b/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)
diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb
index 4dd03cdf24b..21935922414 100644
--- a/lib/api/v3/builds.rb
+++ b/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)
-- 
cgit v1.2.3


From 2cc8f43e54d2b653a4f2e80c57339acb11dcba86 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 5 May 2017 15:13:58 +0200
Subject: Introduce generic manual action extended status class

---
 lib/gitlab/ci/status/build/action.rb            | 23 ++++++++++++++++
 lib/gitlab/ci/status/build/factory.rb           |  3 ++-
 lib/gitlab/ci/status/build/play.rb              |  6 +----
 spec/lib/gitlab/ci/status/build/factory_spec.rb | 32 ++++++++++++----------
 spec/lib/gitlab/ci/status/build/play_spec.rb    | 36 +++++++++----------------
 5 files changed, 57 insertions(+), 43 deletions(-)
 create mode 100644 lib/gitlab/ci/status/build/action.rb

diff --git a/lib/gitlab/ci/status/build/action.rb b/lib/gitlab/ci/status/build/action.rb
new file mode 100644
index 00000000000..1397c35145a
--- /dev/null
+++ b/lib/gitlab/ci/status/build/action.rb
@@ -0,0 +1,23 @@
+module Gitlab
+  module Ci
+    module Status
+      module Build
+        class Action < SimpleDelegator
+          include Status::Extended
+
+          def label
+            if has_action?
+              __getobj__.label
+            else
+              "#{__getobj__.label} (not allowed)"
+            end
+          end
+
+          def self.matches?(build, user)
+            build.action?
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb
index 38ac6edc9f1..c852d607373 100644
--- a/lib/gitlab/ci/status/build/factory.rb
+++ b/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
diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb
index fae34a2f927..3495b8d0448 100644
--- a/lib/gitlab/ci/status/build/play.rb
+++ b/lib/gitlab/ci/status/build/play.rb
@@ -6,11 +6,7 @@ module Gitlab
           include Status::Extended
 
           def label
-            if has_action?
-              'manual play action'
-            else
-              'manual play action (not allowed)'
-            end
+            'manual play action'
           end
 
           def has_action?
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index 2de00c28945..39c5e3658d9 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/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
@@ -247,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
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index abefdbe7c66..f5d0f977768 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -7,38 +7,28 @@ describe Gitlab::Ci::Status::Build::Play do
 
   subject { described_class.new(status) }
 
-  context 'when user is allowed to update build' do
-    context 'when user can push to branch' do
-      before { build.project.add_master(user) }
+  describe '#label' do
+    it 'has a label that says it is a manual action' do
+      expect(subject.label).to eq 'manual play action'
+    end
+  end
+
+  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) }
 
-      describe '#has_action?' do
         it { is_expected.to have_action }
       end
 
-      describe '#label' do
-        it 'has a label that says it is a manual action' do
-          expect(subject.label).to eq 'manual play action'
-        end
-      end
-    end
+      context 'when user can not push to the branch' do
+        before { build.project.add_developer(user) }
 
-    context 'when user can not push to the branch' do
-      before { build.project.add_developer(user) }
-
-      describe 'has_action?' do
         it { is_expected.not_to have_action }
       end
-
-      describe '#label' do
-        it 'has a label that says user is not allowed to play it' do
-          expect(subject.label).to eq 'manual play action (not allowed)'
-        end
-      end
     end
-  end
 
-  context 'when user is not allowed to update build' do
-    describe '#has_action?' do
+    context 'when user is not allowed to update build' do
       it { is_expected.not_to have_action }
     end
   end
-- 
cgit v1.2.3


From 55cec2177cd97a5c0939ab34b01aa408de887d1a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 5 May 2017 15:21:06 +0200
Subject: Refine inheritance model of extended CI/CD statuses

---
 lib/gitlab/ci/status/build/action.rb            |  8 +++-----
 lib/gitlab/ci/status/build/cancelable.rb        |  4 +---
 lib/gitlab/ci/status/build/failed_allowed.rb    |  4 +---
 lib/gitlab/ci/status/build/play.rb              |  4 +---
 lib/gitlab/ci/status/build/retryable.rb         |  4 +---
 lib/gitlab/ci/status/build/stop.rb              |  4 +---
 lib/gitlab/ci/status/extended.rb                | 12 ++++++------
 lib/gitlab/ci/status/pipeline/blocked.rb        |  4 +---
 lib/gitlab/ci/status/success_warning.rb         |  4 +---
 spec/lib/gitlab/ci/status/build/factory_spec.rb |  2 +-
 spec/lib/gitlab/ci/status/extended_spec.rb      |  6 +-----
 11 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/lib/gitlab/ci/status/build/action.rb b/lib/gitlab/ci/status/build/action.rb
index 1397c35145a..45fd0d4aa07 100644
--- a/lib/gitlab/ci/status/build/action.rb
+++ b/lib/gitlab/ci/status/build/action.rb
@@ -2,14 +2,12 @@ module Gitlab
   module Ci
     module Status
       module Build
-        class Action < SimpleDelegator
-          include Status::Extended
-
+        class Action < Status::Extended
           def label
             if has_action?
-              __getobj__.label
+              @status.label
             else
-              "#{__getobj__.label} (not allowed)"
+              "#{@status.label} (not allowed)"
             end
           end
 
diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb
index 67bbc3c4849..57b533bad99 100644
--- a/lib/gitlab/ci/status/build/cancelable.rb
+++ b/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
diff --git a/lib/gitlab/ci/status/build/failed_allowed.rb b/lib/gitlab/ci/status/build/failed_allowed.rb
index 807afe24bd5..e42d3574357 100644
--- a/lib/gitlab/ci/status/build/failed_allowed.rb
+++ b/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
diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb
index 3495b8d0448..c6139f1b716 100644
--- a/lib/gitlab/ci/status/build/play.rb
+++ b/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
diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb
index 6b362af7634..505f80848b2 100644
--- a/lib/gitlab/ci/status/build/retryable.rb
+++ b/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
diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb
index e8530f2aaae..0b5199e5483 100644
--- a/lib/gitlab/ci/status/build/stop.rb
+++ b/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
diff --git a/lib/gitlab/ci/status/extended.rb b/lib/gitlab/ci/status/extended.rb
index d367c9bda69..1e8101f8949 100644
--- a/lib/gitlab/ci/status/extended.rb
+++ b/lib/gitlab/ci/status/extended.rb
@@ -1,13 +1,13 @@
 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
diff --git a/lib/gitlab/ci/status/pipeline/blocked.rb b/lib/gitlab/ci/status/pipeline/blocked.rb
index a250c3fcb41..37dfe43fb62 100644
--- a/lib/gitlab/ci/status/pipeline/blocked.rb
+++ b/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
diff --git a/lib/gitlab/ci/status/success_warning.rb b/lib/gitlab/ci/status/success_warning.rb
index d4cdab6957a..df6e76b0151 100644
--- a/lib/gitlab/ci/status/success_warning.rb
+++ b/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
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index 39c5e3658d9..185bb9098da 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -205,7 +205,7 @@ describe Gitlab::Ci::Status::Build::Factory do
       it 'matches correct extended statuses' do
         expect(factory.extended_statuses)
           .to eq [Gitlab::Ci::Status::Build::Play,
-                 Gitlab::Ci::Status::Build::Action]
+                  Gitlab::Ci::Status::Build::Action]
       end
 
       it 'fabricates action detailed status' do
diff --git a/spec/lib/gitlab/ci/status/extended_spec.rb b/spec/lib/gitlab/ci/status/extended_spec.rb
index c2d74ca5cde..6eacb07078b 100644
--- a/spec/lib/gitlab/ci/status/extended_spec.rb
+++ b/spec/lib/gitlab/ci/status/extended_spec.rb
@@ -1,12 +1,8 @@
 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
-- 
cgit v1.2.3


From e5f24c5490294a4e990103da24e9861f21d7bfd9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 5 May 2017 15:31:00 +0200
Subject: Add specs for extended status for manual actions

---
 spec/lib/gitlab/ci/status/build/action_spec.rb | 56 ++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 spec/lib/gitlab/ci/status/build/action_spec.rb

diff --git a/spec/lib/gitlab/ci/status/build/action_spec.rb b/spec/lib/gitlab/ci/status/build/action_spec.rb
new file mode 100644
index 00000000000..8c25f72804b
--- /dev/null
+++ b/spec/lib/gitlab/ci/status/build/action_spec.rb
@@ -0,0 +1,56 @@
+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
-- 
cgit v1.2.3


From fc121cca5ba87abd24afbc8da2f76e14e386e4c8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 5 May 2017 19:35:32 +0200
Subject: Do not reprocess actions when user retries pipeline

User who is not allowed to trigger manual actions should not be
allowed to reprocess / retrigger / retry these actions.
---
 app/services/ci/retry_pipeline_service.rb       |  2 ++
 spec/services/ci/retry_pipeline_service_spec.rb | 44 ++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb
index ecc6173a96a..5b207157345 100644
--- a/app/services/ci/retry_pipeline_service.rb
+++ b/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
diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb
index f1b2d3a4798..40e151545c9 100644
--- a/spec/services/ci/retry_pipeline_service_spec.rb
+++ b/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
-- 
cgit v1.2.3