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: [PATCH] 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 +++++++++++++++++++
 .../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