Merge branch 'pedropombeiro/335509/2-add-runner-registration-audit-event' into 'master'

Audit runner registration events

See merge request gitlab-org/gitlab!77109

app/models/audit_event.rb

@@ -69,8 +69,7 @@ class AuditEvent < ApplicationRecord
   end
 
   def author
-    lazy_author&.itself.presence ||
-      ::Gitlab::Audit::NullAuthor.for(author_id, (self[:author_name] || details[:author_name]))
+    lazy_author&.itself.presence || default_author_value
   end
 
   def lazy_author
@@ -98,7 +97,7 @@ class AuditEvent < ApplicationRecord
   end
 
   def default_author_value
-    ::Gitlab::Audit::NullAuthor.for(author_id, (self[:author_name] || details[:author_name]))
+    ::Gitlab::Audit::NullAuthor.for(author_id, self)
   end
 
   def parallel_persist

app/services/audit_event_service.rb

@@ -5,7 +5,7 @@ class AuditEventService
 
   # Instantiates a new service
   #
-  # @param [User] author the user who authors the change
+  # @param [User, token String] author the entity who authors the change
   # @param [User, Project, Group] entity the scope which audit event belongs to
   #   This param is also used to determine the visibility of the audit event.
   #   - Project: events are visible at Project and Instance level
@@ -44,7 +44,7 @@ class AuditEventService
 
   # Writes event to a file and creates an event record in DB
   #
-  # @return [AuditEvent] persited if saves and non-persisted if fails
+  # @return [AuditEvent] persisted if saves and non-persisted if fails
   def security_event
     log_security_event_to_file
     log_authentication_event_to_database

app/services/ci/register_runner_service.rb

@@ -3,7 +3,7 @@
 module Ci
   class RegisterRunnerService
     def execute(registration_token, attributes)
-      runner_type_attrs = check_token_and_extract_attrs(registration_token)
+      runner_type_attrs = extract_runner_type_attrs(registration_token)
 
       return unless runner_type_attrs
 
@@ -12,16 +12,32 @@ module Ci
 
     private
 
-    def check_token_and_extract_attrs(registration_token)
+    def extract_runner_type_attrs(registration_token)
+      @attrs_from_token ||= check_token(registration_token)
+
+      return unless @attrs_from_token
+
+      attrs = @attrs_from_token.clone
+      case attrs[:runner_type]
+      when :project_type
+        attrs[:projects] = [attrs.delete(:scope)]
+      when :group_type
+        attrs[:groups] = [attrs.delete(:scope)]
+      end
+
+      attrs
+    end
+
+    def check_token(registration_token)
       if runner_registration_token_valid?(registration_token)
         # Create shared runner. Requires admin access
         { runner_type: :instance_type }
       elsif runner_registrar_valid?('project') && project = ::Project.find_by_runners_token(registration_token)
         # Create a specific runner for the project
-        { runner_type: :project_type, projects: [project] }
+        { runner_type: :project_type, scope: project }
       elsif runner_registrar_valid?('group') && group = ::Group.find_by_runners_token(registration_token)
         # Create a specific runner for the group
-        { runner_type: :group_type, groups: [group] }
+        { runner_type: :group_type, scope: group }
       end
     end
 
@@ -32,5 +48,11 @@ module Ci
     def runner_registrar_valid?(type)
       Feature.disabled?(:runner_registration_control) || Gitlab::CurrentSettings.valid_runner_registrars.include?(type)
     end
+
+    def token_scope
+      @attrs_from_token[:scope]
+    end
   end
 end
+
+Ci::RegisterRunnerService.prepend_mod

ee/app/presenters/audit_event_presenter.rb

@@ -8,9 +8,11 @@ class AuditEventPresenter < Gitlab::View::Presenter::Simple
   end
 
   def author_url
-    return if author.is_a?(Gitlab::Audit::NullAuthor)
-
-    url_for(user_path(author))
+    if author.is_a?(Gitlab::Audit::NullAuthor)
+      author.full_path
+    else
+      url_for(user_path(author))
+    end
   end
 
   def target

ee/app/services/audit_events/runner_registration_audit_event_service.rb

+# frozen_string_literal: true
+
+module AuditEvents
+  class RunnerRegistrationAuditEventService < ::AuditEventService
+    def initialize(runner, registration_token, token_scope, action)
+      @token_scope = token_scope
+      @runner = runner
+      @action = action
+
+      raise ArgumentError, 'Missing token_scope' if token_scope.nil? && !runner.instance_type?
+
+      details = {
+        custom_message: message,
+        target_id: runner.id,
+        target_type: runner.class.name,
+        target_details: runner_path,
+        runner_registration_token: registration_token[0...8]
+      }
+      details[:errors] = @runner.errors.full_messages unless @runner.errors.empty?
+
+      super(details[:runner_registration_token], token_scope, details)
+    end
+
+    def track_event
+      return unless message
+      return security_event if @token_scope
+
+      unauth_security_event
+    end
+
+    def message
+      runner_type = @runner.runner_type.chomp('_type')
+
+      case @action
+      when :register
+        if @runner.valid?
+          "Registered #{runner_type} CI runner"
+        else
+          "Failed to register #{runner_type} CI runner"
+        end
+      end
+    end
+
+    def runner_path
+      return unless @runner.persisted?
+
+      url_helpers = ::Gitlab::Routing.url_helpers
+
+      if @runner.group_type?
+        url_helpers.group_runner_path(@token_scope, @runner)
+      elsif @runner.project_type?
+        url_helpers.project_runner_path(@token_scope, @runner)
+      else
+        url_helpers.admin_runner_path(@runner)
+      end
+    end
+  end
+end

ee/app/services/ee/ci/register_runner_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Ci
+    module RegisterRunnerService
+      extend ::Gitlab::Utils::Override
+      include ::Audit::Changes
+
+      override :execute
+      def execute(registration_token, attributes)
+        runner = super(registration_token, attributes)
+
+        audit_log_event(runner, registration_token) if runner
+
+        runner
+      end
+
+      private
+
+      def audit_log_event(runner, registration_token)
+        ::AuditEvents::RunnerRegistrationAuditEventService.new(runner, registration_token, token_scope, :register)
+          .track_event
+      end
+    end
+  end
+end

ee/spec/presenters/audit_event_presenter_spec.rb

@@ -67,6 +67,28 @@ RSpec.describe AuditEventPresenter do
         end
       end
     end
+
+    context 'event authored by a runner registration token user' do
+      let(:audit_event) { build(:audit_event, user: nil, details: details) }
+      let(:author_double) { double(:author) }
+      let(:details) do
+        {
+          author_name: nil,
+          ip_address: '127.0.0.1',
+          target_details: 'target name',
+          entity_path: 'path',
+          runner_registration_token: 'abc123'
+        }
+      end
+
+      it "returns author's full_path" do
+        allow(author_double).to receive(:is_a?).with(Gitlab::Audit::NullAuthor).and_return(true)
+        expect(author_double).to receive(:full_path).and_return('author path')
+
+        expect(audit_event).to receive(:author).at_least(:once).and_return(author_double)
+        expect(presenter.author_url).to eq('author path')
+      end
+    end
   end
 
   describe '#target' do

ee/spec/services/audit_events/runner_registration_audit_event_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AuditEvents::RunnerRegistrationAuditEventService do
+  let(:registration_token) { 'b6bce79c3a' }
+  let(:service) { described_class.new(runner, registration_token, entity, action) }
+  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,
+        runner_registration_token: registration_token[0...8],
+        ip_address: nil
+      }
+    }
+  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(:entity) { }
+
+      context 'with action as :register' do
+        let(:action) { :register }
+        let(:extra_attrs) { {} }
+        let(:target_details) { }
+        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: 'Registered instance CI runner',
+              entity_path: nil,
+              target_details: target_details
+            }
+          ).deep_merge(extra_attrs)
+        end
+
+        context 'on runner that failed to create' do
+          let(:runner) { build(:ci_runner) }
+          let(:extra_attrs) do
+            {
+              details: {
+                custom_message: 'Failed to register instance CI runner',
+                errors: ['Runner some error']
+              }
+            }
+          end
+
+          before do
+            allow(runner).to receive(:valid?) do
+              runner.errors.add :runner, 'some error'
+              false
+            end
+          end
+
+          it 'returns audit event attributes of a failed runner registration', :aggregate_failures do
+            travel_to(timestamp) do
+              expect(subject.attributes).to eq(attrs.stringify_keys)
+              expect(runner.persisted?).to be_falsey
+            end
+          end
+        end
+
+        context 'on persisted runner' do
+          let(:runner) { create(:ci_runner) }
+          let(:target_details) { ::Gitlab::Routing.url_helpers.admin_runner_path(runner) }
+          let(:extra_attrs) do
+            { details: { custom_message: 'Registered instance CI runner' } }
+          end
+
+          it 'returns audit event attributes' do
+            travel_to(timestamp) do
+              expect(subject.attributes).to eq(attrs.stringify_keys)
+            end
+          end
+        end
+      end
+
+      context 'with unknown action' do
+        let(:runner) { create(:ci_runner) }
+        let(:action) { :unknown }
+
+        it 'is not logged' do
+          is_expected.to be_nil
+        end
+      end
+    end
+
+    context 'for group runner' do
+      let(:entity) { create(:group) }
+
+      context 'with action as :register' do
+        let(:action) { :register }
+        let(:extra_attrs) { {} }
+        let(:target_details) { }
+        let(:attrs) do
+          common_attrs.deep_merge(
+            author_name: registration_token[0...8],
+            entity_id: entity.id,
+            entity_type: entity.class.name,
+            entity_path: entity.full_path,
+            target_details: target_details,
+            details: {
+              author_name: registration_token[0...8],
+              custom_message: 'Registered group CI runner',
+              entity_path: entity.full_path,
+              target_details: target_details
+            }
+          ).deep_merge(extra_attrs)
+        end
+
+        context 'on runner that failed to create' do
+          let(:runner) { build(:ci_runner, :group, groups: [entity]) }
+          let(:extra_attrs) do
+            {
+              details: {
+                custom_message: 'Failed to register group CI runner',
+                errors: ['Runner some error']
+              }
+            }
+          end
+
+          before do
+            allow(runner).to receive(:valid?) do
+              runner.errors.add :runner, 'some error'
+              false
+            end
+          end
+
+          it 'returns audit event attributes of a failed runner registration', :aggregate_failures do
+            travel_to(timestamp) do
+              expect(subject.attributes).to eq(attrs.stringify_keys)
+              expect(runner.persisted?).to be_falsey
+            end
+          end
+        end
+
+        context 'on persisted runner' do
+          let(:runner) { create(:ci_runner, :group, groups: [entity]) }
+          let(:target_details) { ::Gitlab::Routing.url_helpers.group_runner_path(entity, runner) }
+          let(:extra_attrs) do
+            { details: { custom_message: 'Registered group CI runner' } }
+          end
+
+          it 'returns audit event attributes' do
+            travel_to(timestamp) do
+              expect(subject.attributes).to eq(attrs.stringify_keys)
+            end
+          end
+        end
+      end
+
+      context 'with unknown action' do
+        let(:runner) { create(:ci_runner, :group, groups: [entity]) }
+        let(:action) { :unknown }
+
+        it 'is not logged' do
+          is_expected.to be_nil
+        end
+      end
+    end
+
+    context 'for project runner' do
+      let(:entity) { create(:project) }
+
+      context 'with action as :register' do
+        let(:action) { :register }
+        let(:extra_attrs) { {} }
+        let(:target_details) { }
+        let(:attrs) do
+          common_attrs.deep_merge(
+            author_name: registration_token[0...8],
+            entity_id: entity.id,
+            entity_type: entity.class.name,
+            entity_path: entity.full_path,
+            target_details: target_details,
+            details: {
+              author_name: registration_token[0...8],
+              entity_path: entity.full_path,
+              target_details: target_details
+            }
+          ).deep_merge(extra_attrs)
+        end
+
+        context 'on runner that failed to create' do
+          let(:runner) { build(:ci_runner, :project, projects: [entity]) }
+          let(:extra_attrs) do
+            {
+              details: {
+                custom_message: 'Failed to register project CI runner',
+                errors: ['Runner some error']
+              }
+            }
+          end
+
+          before do
+            allow(runner).to receive(:valid?) do
+              runner.errors.add :runner, 'some error'
+              false
+            end
+          end
+
+          it 'returns audit event attributes of a failed runner registration', :aggregate_failures do
+            travel_to(timestamp) do
+              expect(subject.attributes).to eq(attrs.stringify_keys)
+              expect(runner.persisted?).to be_falsey
+            end
+          end
+        end
+
+        context 'on persisted runner' do
+          let(:runner) { create(:ci_runner, :project, projects: [entity]) }
+          let(:target_details) { ::Gitlab::Routing.url_helpers.project_runner_path(entity, runner) }
+          let(:extra_attrs) do
+            { details: { custom_message: 'Registered project CI runner' } }
+          end
+
+          it 'returns audit event attributes' do
+            travel_to(timestamp) do
+              expect(subject.attributes).to eq(attrs.stringify_keys)
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/services/ci/register_runner_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Ci::RegisterRunnerService, '#execute' do
+  let(:registration_token) { 'abcdefg123456' }
+  let(:token) { }
+  let(:audit_service) { instance_double(::AuditEvents::RunnerRegistrationAuditEventService) }
+
+  before do
+    stub_feature_flags(runner_registration_control: false)
+    stub_application_setting(runners_registration_token: registration_token)
+    stub_application_setting(valid_runner_registrars: ApplicationSetting::VALID_RUNNER_REGISTRAR_TYPES)
+
+    expect(audit_service).to receive(:track_event).once.and_return('track_event_return_value')
+  end
+
+  subject { described_class.new.execute(token, {}) }
+
+  RSpec::Matchers.define :last_ci_runner do
+    match { |runner| runner == ::Ci::Runner.last }
+  end
+
+  RSpec::Matchers.define :a_ci_runner_with_errors do
+    match { |runner| runner.errors.any? }
+  end
+
+  shared_examples 'a service logging a runner registration audit event' do
+    it 'returns newly-created Runner' do
+      expect(::AuditEvents::RunnerRegistrationAuditEventService).to receive(:new)
+        .with(last_ci_runner, token, token_scope, :register)
+        .once.and_return(audit_service)
+
+      is_expected.to eq(::Ci::Runner.last)
+    end
+  end
+
+  shared_examples 'a service logging a failed runner registration audit event' do
+    before do
+      expect(::AuditEvents::RunnerRegistrationAuditEventService).to receive(:new)
+        .with(a_ci_runner_with_errors, token, token_scope, :register)
+        .once.and_return(audit_service)
+    end
+
+    it 'returns a Runner' do
+      is_expected.to be_a ::Ci::Runner
+    end
+
+    it 'returns a non-persisted Runner' do
+      expect(subject.persisted?).to be_falsey
+    end
+  end
+
+  context 'with a registration token' do
+    let(:token) { registration_token }
+    let(:token_scope) { }
+
+    it_behaves_like 'a service logging a runner registration audit event'
+  end
+
+  context 'when project token is used' do
+    let(:project) { create(:project) }
+    let(:token) { project.runners_token }
+    let(:token_scope) { project }
+
+    it_behaves_like 'a service logging a runner registration audit event'
+
+    context 'when it exceeds the application limits' do
+      before do
+        create(:ci_runner, runner_type: :project_type, projects: [project], contacted_at: 1.second.ago)
+        create(:plan_limits, :default_plan, ci_registered_project_runners: 1)
+      end
+
+      it_behaves_like 'a service logging a failed runner registration audit event'
+    end
+  end
+
+  context 'when group token is used' do
+    let(:group) { create(:group) }
+    let(:token) { group.runners_token }
+    let(:token_scope) { group }
+
+    it_behaves_like 'a service logging a runner registration audit event'
+
+    context 'when it exceeds the application limits' do
+      before do
+        create(:ci_runner, runner_type: :group_type, groups: [group], contacted_at: 1.second.ago)
+        create(:plan_limits, :default_plan, ci_registered_group_runners: 1)
+      end
+
+      it_behaves_like 'a service logging a failed runner registration audit event'
+    end
+  end
+end

lib/gitlab/audit/null_author.rb

@@ -14,9 +14,17 @@ module Gitlab
       # @param [Integer] id
       # @param [String] name
       #
-      # @return [Gitlab::Audit::UnauthenticatedAuthor, Gitlab::Audit::DeletedAuthor]
-      def self.for(id, name)
-        if id == -1
+      # @return [Gitlab::Audit::UnauthenticatedAuthor, Gitlab::Audit::DeletedAuthor, Gitlab::Audit::RunnerRegistrationTokenAuthor]
+      def self.for(id, audit_event)
+        name = audit_event[:author_name] || audit_event.details[:author_name]
+
+        if audit_event.details.include?(:runner_registration_token)
+          ::Gitlab::Audit::RunnerRegistrationTokenAuthor.new(
+            token: audit_event.details[:runner_registration_token],
+            entity_type: audit_event.entity_type || audit_event.details[:entity_type],
+            entity_path: audit_event.entity_path || audit_event.details[:entity_path]
+          )
+        elsif id == -1
           Gitlab::Audit::UnauthenticatedAuthor.new(name: name)
         else
           Gitlab::Audit::DeletedAuthor.new(id: id, name: name)
@@ -31,6 +39,10 @@ module Gitlab
       def current_sign_in_ip
         nil
       end
+
+      def full_path
+        nil
+      end
     end
   end
 end

lib/gitlab/audit/runner_registration_token_author.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Audit
+    class RunnerRegistrationTokenAuthor < Gitlab::Audit::NullAuthor
+      def initialize(token:, entity_type:, entity_path:)
+        super(id: -1, name: "Registration token: #{token}")
+
+        @entity_type = entity_type
+        @entity_path = entity_path
+      end
+
+      def full_path
+        url_helpers = ::Gitlab::Routing.url_helpers
+
+        case @entity_type
+        when 'Group'
+          url_helpers.group_settings_ci_cd_path(@entity_path, anchor: 'js-runners-settings')
+        when 'Project'
+          project = Project.find_by_full_path(@entity_path)
+          url_helpers.project_settings_ci_cd_path(project, anchor: 'js-runners-settings') if project
+        else
+          url_helpers.admin_runners_path
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/audit/null_author_spec.rb

@@ -6,13 +6,32 @@ RSpec.describe Gitlab::Audit::NullAuthor do
   subject { described_class }
 
   describe '.for' do
+    let(:audit_event) { instance_double(AuditEvent) }
+
     it 'returns an DeletedAuthor' do
-      expect(subject.for(666, 'Old Hat')).to be_a(Gitlab::Audit::DeletedAuthor)
+      allow(audit_event).to receive(:[]).with(:author_name).and_return('Old Hat')
+      allow(audit_event).to receive(:details).and_return({})
+
+      expect(subject.for(666, audit_event)).to be_a(Gitlab::Audit::DeletedAuthor)
     end
 
     it 'returns an UnauthenticatedAuthor when id equals -1', :aggregate_failures do
-      expect(subject.for(-1, 'Frank')).to be_a(Gitlab::Audit::UnauthenticatedAuthor)
-      expect(subject.for(-1, 'Frank')).to have_attributes(id: -1, name: 'Frank')
+      allow(audit_event).to receive(:[]).with(:author_name).and_return('Frank')
+      allow(audit_event).to receive(:details).and_return({})
+
+      expect(subject.for(-1, audit_event)).to be_a(Gitlab::Audit::UnauthenticatedAuthor)
+      expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Frank')
+    end
+
+    it 'returns an RunnerRegistrationTokenAuthor when details contain runner registration token', :aggregate_failures do
+      allow(audit_event).to receive(:[]).with(:author_name).and_return('cde456')
+      allow(audit_event).to receive(:entity_type).and_return('User')
+      allow(audit_event).to receive(:entity_path).and_return('/a/b')
+      allow(audit_event).to receive(:details)
+        .and_return({ runner_registration_token: 'cde456', author_name: 'cde456', entity_type: 'User', entity_path: '/a/b' })
+
+      expect(subject.for(-1, audit_event)).to be_a(Gitlab::Audit::RunnerRegistrationTokenAuthor)
+      expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Registration token: cde456')
     end
   end
 

spec/lib/gitlab/audit/runner_registration_token_author_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Audit::RunnerRegistrationTokenAuthor do
+  describe '#initialize' do
+    it 'sets correct attributes' do
+      expect(described_class.new(token: 'abc1234567', entity_type: 'Project', entity_path: 'd/e'))
+        .to have_attributes(id: -1, name: 'Registration token: abc1234567')
+    end
+  end
+
+  describe '#full_path' do
+    subject { author.full_path }
+
+    context 'with instance registration token' do
+      let(:author) { described_class.new(token: 'abc1234567', entity_type: 'User', entity_path: nil) }
+
+      it 'returns correct url' do
+        is_expected.to eq('/admin/runners')
+      end
+    end
+
+    context 'with group registration token' do
+      let(:author) { described_class.new(token: 'abc1234567', entity_type: 'Group', entity_path: 'a/b') }
+
+      it 'returns correct url' do
+        expect(::Gitlab::Routing.url_helpers).to receive(:group_settings_ci_cd_path)
+          .once
+          .with('a/b', { anchor: 'js-runners-settings' })
+          .and_return('/path/to/group/runners')
+
+        is_expected.to eq('/path/to/group/runners')
+      end
+    end
+
+    context 'with project registration token' do
+      let(:author) { described_class.new(token: 'abc1234567', entity_type: 'Project', entity_path: project.full_path) }
+      let(:project) { create(:project) }
+
+      it 'returns correct url' do
+        expect(::Gitlab::Routing.url_helpers).to receive(:project_settings_ci_cd_path)
+          .once
+          .with(project, { anchor: 'js-runners-settings' })
+          .and_return('/path/to/project/runners')
+
+        is_expected.to eq('/path/to/project/runners')
+      end
+    end
+  end
+end

spec/models/audit_event_spec.rb

@@ -93,4 +93,37 @@ RSpec.describe AuditEvent do
       end
     end
   end
+
+  describe '#author' do
+    subject { audit_event.author }
+
+    context "when a runner_registration_token's present" do
+      let(:audit_event) { build(:project_audit_event, details: { target_id: 678 }) }
+
+      it 'returns a NullAuthor' do
+        expect(::Gitlab::Audit::NullAuthor).to receive(:for)
+          .and_call_original
+          .once
+
+        is_expected.to be_a_kind_of(::Gitlab::Audit::NullAuthor)
+      end
+    end
+
+    context "when a runner_registration_token's present" do
+      let(:audit_event) { build(:project_audit_event, details: { target_id: 678, runner_registration_token: 'abc123' }) }
+
+      it 'returns a RunnerRegistrationTokenAuthor' do
+        expect(::Gitlab::Audit::RunnerRegistrationTokenAuthor).to receive(:new)
+          .with({ token: 'abc123', entity_type: 'Project', entity_path: audit_event.entity_path })
+          .and_call_original
+          .once
+
+        is_expected.to be_an_instance_of(::Gitlab::Audit::RunnerRegistrationTokenAuthor)
+      end
+
+      it 'name consists of prefix and token' do
+        expect(subject.name).to eq('Registration token: abc123')
+      end
+    end
+  end
 end