Execute environment stop actions as the owner of the action

Merge branch 'security-run-stop-actions-as-job-owner-17-1' into '17-1-stable-ee'

See merge request gitlab-org/security/gitlab!4406

Changelog: security

app/controllers/projects/environments_controller.rb

@@ -119,7 +119,7 @@ def update
   def stop
     return render_404 unless @environment.available?
-    stop_actions = @environment.stop_with_actions!(current_user)
+    stop_actions = @environment.stop_with_actions!
     job = stop_actions.first if stop_actions&.count == 1
     action_or_env_url =

app/models/environment.rb

@@ -353,7 +353,7 @@ def wait_for_stop?
     stop_actions.present?
   end
-  def stop_with_actions!(current_user)
+  def stop_with_actions!
     return unless available?
     stop!
@@ -365,7 +365,7 @@ def stop_with_actions!(current_user)
         stop_action,
         name: 'environment_stop_with_actions'
       ) do |job|
-        actions << job.play(current_user)
+        actions << job.play(job.user)
       rescue StateMachines::InvalidTransition
         # Ci::PlayBuildService rescues an error of StateMachines::InvalidTransition and fall back to retry. However,
         # Ci::PlayBridgeService doesn't rescue it, so we're ignoring the error if it's not playable.

app/services/environments/stop_service.rb

@@ -15,7 +15,7 @@ def execute(environment)
       if params[:force]
         environment.stop_complete!
       else
-        environment.stop_with_actions!(current_user)
+        environment.stop_with_actions!
       end
       unless environment.saved_change_to_attribute?(:state)

app/workers/environments/auto_stop_worker.rb

@@ -9,12 +9,7 @@ class AutoStopWorker
     feature_category :continuous_delivery
     def perform(environment_id, params = {})
-      Environment.find_by_id(environment_id).try do |environment|
-        stop_actions = environment.stop_actions
-
-        user = stop_actions.last&.user
-        environment.stop_with_actions!(user)
-      end
+      Environment.find_by_id(environment_id).try(&:stop_with_actions!)
     end
   end
 end

spec/factories/environments.rb

@@ -48,6 +48,7 @@
       transient do
         ref { 'master' }
+        user { nil }
       end
       # At this point `review app` is an ephemeral concept related to
@@ -56,9 +57,15 @@
       # interconnected objects to simulate a review app.
       #
       after(:create) do |environment, evaluator|
-        pipeline = create(:ci_pipeline, project: environment.project)
-        deployable = create(:ci_build, :success, name: "#{environment.name}:deploy", pipeline: pipeline)
+        pipeline = create(:ci_pipeline, project: environment.project, user: evaluator.user)
+        deployable = create(
+          :ci_build,
+          :success,
+          name: "#{environment.name}:deploy",
+          pipeline: pipeline,
+          user: evaluator.user
+        )
         deployment = create(
           :deployment,
@@ -66,11 +73,18 @@
           environment: environment,
           project: environment.project,
           deployable: deployable,
+          user: evaluator.user,
           ref: evaluator.ref,
           sha: environment.project.commit(evaluator.ref).id
         )
-        teardown_build = create(:ci_build, :manual, name: "#{environment.name}:teardown", pipeline: pipeline)
+        teardown_build = create(
+          :ci_build,
+          :manual,
+          name: "#{environment.name}:teardown",
+          pipeline: pipeline,
+          user: evaluator.user
+        )
         deployment.update_column(:on_stop, teardown_build.name)
         environment.update_attribute(:deployments, [deployment])

spec/features/projects/environments/environment_spec.rb

@@ -294,7 +294,7 @@ def auto_stop_button_selector
     let(:project) { create(:project, :repository) }
     let!(:environment) do
-      create(:environment, :with_review_app, project: project, ref: 'feature')
+      create(:environment, :with_review_app, project: project, ref: 'feature', user: user)
     end
     it 'user visits environment page', :js do

spec/models/environment_spec.rb

@@ -777,7 +777,7 @@
   describe '#stop_with_actions!' do
     let(:user) { create(:user) }
-    subject { environment.stop_with_actions!(user) }
+    subject { environment.stop_with_actions! }
     shared_examples_for 'stop with playing a teardown job' do
       before do
@@ -980,12 +980,12 @@
     it_behaves_like 'stop with playing a teardown job' do
       let(:factory_type) { :ci_build }
-      let(:factory_options) { {} }
+      let(:factory_options) { { user: user } }
     end
     it_behaves_like 'stop with playing a teardown job' do
       let(:factory_type) { :ci_bridge }
-      let(:factory_options) { { downstream: project } }
+      let(:factory_options) { { user: user, downstream: project } }
     end
   end

spec/services/environments/stop_service_spec.rb

@@ -23,7 +23,7 @@
       let!(:environment) { review_job.persisted_environment }
       let!(:pipeline) { create(:ci_pipeline, project: project) }
       let!(:review_job) { create(:ci_build, :with_deployment, :start_review_app, pipeline: pipeline, project: project) }
-      let!(:stop_review_job) { create(:ci_build, :with_deployment, :stop_review_app, :manual, pipeline: pipeline, project: project) }
+      let!(:stop_review_job) { create(:ci_build, :with_deployment, :stop_review_app, :manual, pipeline: pipeline, project: project, user: user) }
       before do
         review_job.success!
@@ -184,8 +184,8 @@
         merge_requests_as_head_pipeline: [merge_request])
     end
-    let!(:review_job) { create(:ci_build, :with_deployment, :start_review_app, :success, pipeline: pipeline, project: project) }
-    let!(:stop_review_job) { create(:ci_build, :with_deployment, :stop_review_app, :manual, pipeline: pipeline, project: project) }
+    let!(:review_job) { create(:ci_build, :with_deployment, :start_review_app, :success, pipeline: pipeline, project: project, user: user) }
+    let!(:stop_review_job) { create(:ci_build, :with_deployment, :stop_review_app, :manual, pipeline: pipeline, project: project, user: user) }
     before do
       review_job.deployment.success!
@@ -244,8 +244,8 @@
       context 'with environment related jobs ' do
         let!(:environment) { create(:environment, :available, name: 'staging', project: project) }
         let!(:prepare_staging_job) { create(:ci_build, :prepare_staging, pipeline: pipeline, project: project) }
-        let!(:start_staging_job) { create(:ci_build, :start_staging, :with_deployment, :manual, pipeline: pipeline, project: project) }
-        let!(:stop_staging_job) { create(:ci_build, :stop_staging, :manual, pipeline: pipeline, project: project) }
+        let!(:start_staging_job) { create(:ci_build, :start_staging, :with_deployment, :manual, pipeline: pipeline, project: project, user: user) }
+        let!(:stop_staging_job) { create(:ci_build, :stop_staging, :manual, pipeline: pipeline, project: project, user: user) }
         it 'does not stop environments that was not started by the merge request' do
           subject
@@ -308,6 +308,6 @@ def expect_environment_not_stopped_on(branch, environment)
   end
   def feature_environment
-    create(:environment, :with_review_app, project: project, ref: 'feature')
+    create(:environment, :with_review_app, project: project, ref: 'feature', user: user)
   end
 end