From 263254f3ca499da459fa800868e313f4d1528279 Mon Sep 17 00:00:00 2001
From: Fabio Pitino <fpitino@gitlab.com>
Date: Fri, 13 Sep 2019 07:40:00 +0100
Subject: [PATCH] Cancel all running CI jobs when user is blocked

This prevents a MITM attack where attacker could
still access Git repository if any jobs were
running long enough.
---
 app/models/user.rb                            | 10 ++++++++
 .../ci/cancel_user_pipelines_service.rb       | 13 +++++++++++
 ...curity-fp-stop-jobs-when-blocking-user.yml |  5 ++++
 spec/models/user_spec.rb                      | 18 ++++++++++++++-
 .../ci/cancel_user_pipelines_service_spec.rb  | 23 +++++++++++++++++++
 5 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 app/services/ci/cancel_user_pipelines_service.rb
 create mode 100644 changelogs/unreleased/security-fp-stop-jobs-when-blocking-user.yml
 create mode 100644 spec/services/ci/cancel_user_pipelines_service_spec.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 66defb4c707..a69db121a0b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -267,6 +267,16 @@ class User < ApplicationRecord
         BLOCKED_MESSAGE
       end
     end
+
+    # rubocop: disable CodeReuse/ServiceClass
+    # Ideally we should not call a service object here but user.block
+    # is also bcalled by Users::MigrateToGhostUserService which references
+    # this state transition object in order to do a rollback.
+    # For this reason the tradeoff is to disable this cop.
+    after_transition any => :blocked do |user|
+      Ci::CancelUserPipelinesService.new.execute(user)
+    end
+    # rubocop: enable CodeReuse/ServiceClass
   end
 
   # Scopes
diff --git a/app/services/ci/cancel_user_pipelines_service.rb b/app/services/ci/cancel_user_pipelines_service.rb
new file mode 100644
index 00000000000..bcafb6b4a35
--- /dev/null
+++ b/app/services/ci/cancel_user_pipelines_service.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module Ci
+  class CancelUserPipelinesService
+    # rubocop: disable CodeReuse/ActiveRecord
+    # This is a bug with CodeReuse/ActiveRecord cop
+    # https://gitlab.com/gitlab-org/gitlab/issues/32332
+    def execute(user)
+      user.pipelines.cancelable.find_each(&:cancel_running)
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+end
diff --git a/changelogs/unreleased/security-fp-stop-jobs-when-blocking-user.yml b/changelogs/unreleased/security-fp-stop-jobs-when-blocking-user.yml
new file mode 100644
index 00000000000..1bc4345d5b6
--- /dev/null
+++ b/changelogs/unreleased/security-fp-stop-jobs-when-blocking-user.yml
@@ -0,0 +1,5 @@
+---
+title: Cancel all running CI jobs triggered by the user who is just blocked
+merge_request:
+author:
+type: security
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 228d1ce9964..a701b858783 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1097,11 +1097,27 @@ describe User do
   describe 'blocking user' do
     let(:user) { create(:user, name: 'John Smith') }
 
-    it "blocks user" do
+    it 'blocks user' do
       user.block
 
       expect(user.blocked?).to be_truthy
     end
+
+    context 'when user has running CI pipelines' do
+      let(:service) { double }
+
+      before do
+        pipeline = create(:ci_pipeline, :running, user: user)
+        create(:ci_build, :running, pipeline: pipeline)
+      end
+
+      it 'cancels all running pipelines and related jobs' do
+        expect(Ci::CancelUserPipelinesService).to receive(:new).and_return(service)
+        expect(service).to receive(:execute).with(user)
+
+        user.block
+      end
+    end
   end
 
   describe '.filter_items' do
diff --git a/spec/services/ci/cancel_user_pipelines_service_spec.rb b/spec/services/ci/cancel_user_pipelines_service_spec.rb
new file mode 100644
index 00000000000..251f21feaef
--- /dev/null
+++ b/spec/services/ci/cancel_user_pipelines_service_spec.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::CancelUserPipelinesService do
+  describe '#execute' do
+    let(:user) { create(:user) }
+
+    subject { described_class.new.execute(user) }
+
+    context 'when user has running CI pipelines' do
+      let(:pipeline) { create(:ci_pipeline, :running, user: user) }
+      let!(:build) { create(:ci_build, :running, pipeline: pipeline) }
+
+      it 'cancels all running pipelines and related jobs' do
+        subject
+
+        expect(pipeline.reload).to be_canceled
+        expect(build.reload).to be_canceled
+      end
+    end
+  end
+end
-- 
GitLab