Merge branch 'pedropombeiro/349540/3-log-ci-unregister-events' into 'master'

Log CI runner unregistration audit events

See merge request gitlab-org/gitlab!79754

app/controllers/admin/runners_controller.rb

@@ -34,7 +34,7 @@ class Admin::RunnersController < Admin::ApplicationController
   end
 
   def destroy
-    Ci::UnregisterRunnerService.new(@runner).execute
+    Ci::UnregisterRunnerService.new(@runner, current_user).execute
 
     redirect_to admin_runners_path, status: :found
   end

app/controllers/groups/runners_controller.rb

@@ -35,7 +35,7 @@ class Groups::RunnersController < Groups::ApplicationController
     if @runner.belongs_to_more_than_one_project?
       redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), status: :found, alert: _('Runner was not deleted because it is assigned to multiple projects.')
     else
-      Ci::UnregisterRunnerService.new(@runner).execute
+      Ci::UnregisterRunnerService.new(@runner, current_user).execute
 
       redirect_to group_settings_ci_cd_path(@group, anchor: 'runners-settings'), status: :found
     end

app/controllers/projects/runners_controller.rb

@@ -23,7 +23,7 @@ class Projects::RunnersController < Projects::ApplicationController
 
   def destroy
     if @runner.only_for?(project)
-      Ci::UnregisterRunnerService.new(@runner).execute
+      Ci::UnregisterRunnerService.new(@runner, current_user).execute
     end
 
     redirect_to project_runners_path(@project), status: :found

app/graphql/mutations/ci/runner/delete.rb

@@ -20,7 +20,7 @@ module Mutations
           error = authenticate_delete_runner!(runner)
           return { errors: [error] } if error
 
-          ::Ci::UnregisterRunnerService.new(runner).execute
+          ::Ci::UnregisterRunnerService.new(runner, current_user).execute
 
           { errors: runner.errors.full_messages }
         end

app/services/ci/unregister_runner_service.rb

@@ -2,11 +2,13 @@
 
 module Ci
   class UnregisterRunnerService
-    attr_reader :runner
+    attr_reader :runner, :author
 
     # @param [Ci::Runner] runner the runner to unregister/destroy
-    def initialize(runner)
+    # @param [User, authentication token String] author the user or the authentication token that authorizes the removal
+    def initialize(runner, author)
       @runner = runner
+      @author = author
     end
 
     def execute
@@ -14,3 +16,5 @@ module Ci
     end
   end
 end
+
+Ci::UnregisterRunnerService.prepend_mod

ee/app/services/audit_events/unregister_runner_audit_event_service.rb

+# frozen_string_literal: true
+
+module AuditEvents
+  class UnregisterRunnerAuditEventService < RunnerRegistrationAuditEventService
+    def token_field
+      :runner_authentication_token
+    end
+
+    def message
+      "Unregistered #{runner_type} CI runner"
+    end
+  end
+end

ee/app/services/ee/ci/unregister_runner_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Ci
+    # Unregisters a CI Runner and logs an audit event
+    #
+    module UnregisterRunnerService
+      extend ::Gitlab::Utils::Override
+      include ::Audit::Changes
+
+      override :execute
+      def execute
+        scopes = runner_scopes # Save the scopes before destroying the record
+
+        result = super
+
+        audit_log_event(scopes)
+
+        result
+      end
+
+      private
+
+      def runner_scopes
+        case runner.runner_type
+        when 'instance_type'
+          [nil]
+        when 'group_type'
+          runner.groups.to_a
+        when 'project_type'
+          runner.projects.to_a
+        end
+      end
+
+      def audit_log_event(scopes)
+        scopes.each do |scope|
+          ::AuditEvents::UnregisterRunnerAuditEventService.new(runner, author, scope)
+            .track_event
+        end
+      end
+    end
+  end
+end

ee/spec/services/audit_events/unregister_runner_audit_event_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AuditEvents::UnregisterRunnerAuditEventService do
+  let_it_be(:user) { create(:user) }
+
+  let(:service) { described_class.new(runner, author, entity) }
+  let(:common_attrs) do
+    {
+      author_id: -1,
+      created_at: timestamp,
+      id: subject.id,
+      target_type: runner.class.name,
+      target_id: runner.id,
+      ip_address: nil,
+      details: {
+        target_type: runner.class.name,
+        target_id: runner.id,
+        ip_address: nil
+      }
+    }
+  end
+
+  shared_examples 'expected audit log' do
+    it 'returns audit event attributes' do
+      travel_to(timestamp) do
+        expect(subject.attributes).to eq(attrs.stringify_keys)
+      end
+    end
+  end
+
+  shared_context 'when unregistering runner' do
+    let(:extra_attrs) { {} }
+    let(:attrs) do
+      entity_class_name = entity.class.name if entity
+
+      common_attrs.deep_merge(
+        entity_id: entity&.id || -1,
+        entity_type: entity ? entity_class_name : 'User',
+        entity_path: entity&.full_path,
+        target_details: target_details,
+        details: {
+          custom_message: "Unregistered #{entity_class_name&.downcase || 'instance'} CI runner",
+          entity_path: entity&.full_path,
+          target_details: target_details
+        }
+      ).deep_merge(extra_attrs)
+    end
+
+    context 'with authentication token author' do
+      let(:author) { 'b6bce79c3a' }
+      let(:extra_attrs) do
+        {
+          author_name: author[0...8],
+          details: {
+            author_name: author[0...8],
+            runner_authentication_token: author[0...8]
+          }
+        }
+      end
+
+      it_behaves_like 'expected audit log'
+    end
+
+    context 'with User author' do
+      let(:author) { user }
+      let(:extra_attrs) do
+        {
+          author_id: author.id,
+          author_name: author.name,
+          details: { author_name: author.name }
+        }
+      end
+
+      it_behaves_like 'expected audit log'
+    end
+  end
+
+  describe '#track_event' do
+    before do
+      stub_licensed_features(admin_audit_log: true)
+    end
+
+    subject { service.track_event }
+
+    let(:timestamp) { Time.zone.local(2021, 12, 28) }
+
+    context 'for instance runner' do
+      before do
+        stub_licensed_features(extended_audit_events: true, admin_audit_log: true)
+      end
+
+      let_it_be(:runner) { create(:ci_runner) }
+
+      let(:entity) { }
+      let(:extra_attrs) { {} }
+      let(:target_details) { ::Gitlab::Routing.url_helpers.admin_runner_path(runner) }
+      let(:attrs) do
+        common_attrs.deep_merge(
+          author_name: nil,
+          entity_id: -1,
+          entity_type: 'User',
+          entity_path: nil,
+          target_details: target_details,
+          details: {
+            custom_message: 'Unregistered instance CI runner',
+            entity_path: nil,
+            target_details: target_details
+          }
+        ).deep_merge(extra_attrs)
+      end
+
+      context 'with authentication token author' do
+        let(:author) { 'b6bce79c3a' }
+        let(:extra_attrs) do
+          {
+            details: { runner_authentication_token: author[0...8] }
+          }
+        end
+
+        it_behaves_like 'expected audit log'
+      end
+
+      context 'with User author' do
+        let(:author) { user }
+
+        let(:extra_attrs) do
+          { author_id: author.id }
+        end
+
+        it_behaves_like 'expected audit log'
+      end
+    end
+
+    context 'for group runner' do
+      let_it_be(:entity) { create(:group) }
+      let_it_be(:runner) { create(:ci_runner, :group, groups: [entity]) }
+
+      include_context 'when unregistering runner' do
+        let(:extra_attrs) { {} }
+        let(:target_details) { ::Gitlab::Routing.url_helpers.group_runner_path(entity, runner) }
+      end
+    end
+
+    context 'for project runner' do
+      let_it_be(:entity) { create(:project) }
+      let_it_be(:runner) { create(:ci_runner, :project, projects: [entity]) }
+
+      include_context 'when unregistering runner' do
+        let(:extra_attrs) { {} }
+        let(:target_details) { ::Gitlab::Routing.url_helpers.project_runner_path(entity, runner) }
+      end
+    end
+  end
+end

ee/spec/services/ci/unregister_runner_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Ci::UnregisterRunnerService, '#execute' do
+  let(:audit_service) { instance_double(::AuditEvents::UnregisterRunnerAuditEventService) }
+  let(:current_user) { nil }
+  let(:token) { 'abc123' }
+
+  subject { described_class.new(runner, current_user || token).execute }
+
+  context 'on an instance runner' do
+    let(:runner) { create(:ci_runner) }
+
+    it 'logs an audit event with the instance scope' do
+      expect(audit_service).to receive(:track_event).once.and_return('track_event_return_value')
+      expect(::AuditEvents::UnregisterRunnerAuditEventService).to receive(:new)
+        .with(runner, token, nil)
+        .once.and_return(audit_service)
+
+      subject
+    end
+  end
+
+  context 'on a group runner' do
+    let(:group) { create(:group) }
+    let(:runner) { create(:ci_runner, :group, groups: [group]) }
+    let(:current_user) { build(:user) }
+
+    it 'logs an audit event with the group scope' do
+      expect(audit_service).to receive(:track_event).once.and_return('track_event_return_value')
+      expect(::AuditEvents::UnregisterRunnerAuditEventService).to receive(:new)
+        .with(runner, current_user, group)
+        .once.and_return(audit_service)
+
+      subject
+    end
+  end
+
+  context 'on a project runner' do
+    let(:project1) { create(:project) }
+    let(:project2) { create(:project) }
+    let(:runner) { create(:ci_runner, :project, projects: [project1, project2]) }
+
+    it 'logs an audit event per project' do
+      expect(audit_service).to receive(:track_event).twice.and_return('track_event_return_value')
+      expect(::AuditEvents::UnregisterRunnerAuditEventService).to receive(:new)
+        .with(runner, token, project1)
+        .once.and_return(audit_service)
+      expect(::AuditEvents::UnregisterRunnerAuditEventService).to receive(:new)
+        .with(runner, token, project2)
+        .once.and_return(audit_service)
+
+      subject
+    end
+  end
+end

lib/api/ci/runner.rb

@@ -57,7 +57,7 @@ module API
         delete '/', feature_category: :runner do
           authenticate_runner!
 
-          destroy_conditionally!(current_runner) { ::Ci::UnregisterRunnerService.new(current_runner).execute }
+          destroy_conditionally!(current_runner) { ::Ci::UnregisterRunnerService.new(current_runner, params[:token]).execute }
         end
 
         desc 'Validates authentication credentials' do

lib/api/ci/runners.rb

@@ -110,7 +110,7 @@ module API
 
           authenticate_delete_runner!(runner)
 
-          destroy_conditionally!(runner) { ::Ci::UnregisterRunnerService.new(runner).execute }
+          destroy_conditionally!(runner) { ::Ci::UnregisterRunnerService.new(runner, current_user).execute }
         end
 
         desc 'List jobs running on a runner' do

spec/controllers/admin/runners_controller_spec.rb

@@ -105,7 +105,7 @@ RSpec.describe Admin::RunnersController do
 
   describe '#destroy' do
     it 'destroys the runner' do
-      expect_next_instance_of(Ci::UnregisterRunnerService, runner) do |service|
+      expect_next_instance_of(Ci::UnregisterRunnerService, runner, user) do |service|
         expect(service).to receive(:execute).once.and_call_original
       end
 

spec/controllers/groups/runners_controller_spec.rb

@@ -190,7 +190,7 @@ RSpec.describe Groups::RunnersController do
       end
 
       it 'destroys the runner and redirects' do
-        expect_next_instance_of(Ci::UnregisterRunnerService, runner) do |service|
+        expect_next_instance_of(Ci::UnregisterRunnerService, runner, user) do |service|
           expect(service).to receive(:execute).once.and_call_original
         end
 

spec/controllers/projects/runners_controller_spec.rb

@@ -37,7 +37,7 @@ RSpec.describe Projects::RunnersController do
 
   describe '#destroy' do
     it 'destroys the runner' do
-      expect_next_instance_of(Ci::UnregisterRunnerService, runner) do |service|
+      expect_next_instance_of(Ci::UnregisterRunnerService, runner, user) do |service|
         expect(service).to receive(:execute).once.and_call_original
       end
 

spec/graphql/mutations/ci/runner/delete_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe Mutations::Ci::Runner::Delete do
         it 'deletes runner' do
           mutation_params[:id] = project_runner.to_global_id
 
-          expect_next_instance_of(::Ci::UnregisterRunnerService, project_runner) do |service|
+          expect_next_instance_of(::Ci::UnregisterRunnerService, project_runner, current_ctx[:current_user]) do |service|
             expect(service).to receive(:execute).once.and_call_original
           end
 
@@ -76,7 +76,7 @@ RSpec.describe Mutations::Ci::Runner::Delete do
           mutation_params[:id] = two_projects_runner.to_global_id
 
           allow_next_instance_of(::Ci::UnregisterRunnerService) do |service|
-            expect(service).not_to receive(:execute).once
+            expect(service).not_to receive(:execute)
           end
           expect { subject }.not_to change { Ci::Runner.count }
           expect(subject[:errors]).to contain_exactly("Runner #{two_projects_runner.to_global_id} associated with more than one project")
@@ -89,7 +89,7 @@ RSpec.describe Mutations::Ci::Runner::Delete do
       let(:current_ctx) { { current_user: admin_user } }
 
       it 'deletes runner' do
-        expect_next_instance_of(::Ci::UnregisterRunnerService, runner) do |service|
+        expect_next_instance_of(::Ci::UnregisterRunnerService, runner, current_ctx[:current_user]) do |service|
           expect(service).to receive(:execute).once.and_call_original
         end
 

spec/requests/api/ci/runners_spec.rb

@@ -530,7 +530,7 @@ RSpec.describe API::Ci::Runners do
     context 'admin user' do
       context 'when runner is shared' do
         it 'deletes runner' do
-          expect_next_instance_of(Ci::UnregisterRunnerService, shared_runner) do |service|
+          expect_next_instance_of(Ci::UnregisterRunnerService, shared_runner, admin) do |service|
             expect(service).to receive(:execute).once.and_call_original
           end
 
@@ -548,7 +548,7 @@ RSpec.describe API::Ci::Runners do
 
       context 'when runner is not shared' do
         it 'deletes used project runner' do
-          expect_next_instance_of(Ci::UnregisterRunnerService, project_runner) do |service|
+          expect_next_instance_of(Ci::UnregisterRunnerService, project_runner, admin) do |service|
             expect(service).to receive(:execute).once.and_call_original
           end
 

spec/services/ci/unregister_runner_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe ::Ci::UnregisterRunnerService, '#execute' do
-  subject { described_class.new(runner).execute }
+  subject { described_class.new(runner, 'some_token').execute }
 
   let(:runner) { create(:ci_runner) }