Merge branch 'pedropombeiro/349540/1-split-audit-token' into 'master'

Split CiRunnerTokenAuthor class

See merge request gitlab-org/gitlab!80621

lib/gitlab/audit/ci_runner_token_author.rb

@@ -3,11 +3,24 @@
 module Gitlab
   module Audit
     class CiRunnerTokenAuthor < Gitlab::Audit::NullAuthor
-      def initialize(token:, entity_type:, entity_path:)
-        super(id: -1, name: "Registration token: #{token}")
+      # Represents a CI Runner token (registration or authentication)
+      #
+      # @param [AuditEvent] audit_event event representing a runner registration/un-registration operation
+      def initialize(audit_event)
+        if audit_event.details.include?(:runner_authentication_token)
+          token = audit_event.details[:runner_authentication_token]
+          name = "Authentication token: #{token}"
+        elsif audit_event.details.include?(:runner_registration_token)
+          token = audit_event.details[:runner_registration_token]
+          name = "Registration token: #{token}"
+        else
+          raise ArgumentError, 'Runner token missing'
+        end
+
+        super(id: -1, name: name)
 
-        @entity_type = entity_type
-        @entity_path = entity_path
+        @entity_type = audit_event.entity_type
+        @entity_path = audit_event.entity_path
       end
 
       def full_path

lib/gitlab/audit/null_author.rb

@@ -18,12 +18,8 @@ module Gitlab
       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::CiRunnerTokenAuthor.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]
-          )
+        if audit_event.target_type == ::Ci::Runner.name
+          Gitlab::Audit::CiRunnerTokenAuthor.new(audit_event)
         elsif id == -1
           Gitlab::Audit::UnauthenticatedAuthor.new(name: name)
         else

spec/lib/gitlab/audit/ci_runner_token_author_spec.rb

@@ -3,18 +3,50 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Audit::CiRunnerTokenAuthor 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')
+  describe '.initialize' do
+    subject { described_class.new(audit_event) }
+
+    let(:details) { }
+    let(:audit_event) { instance_double(AuditEvent, details: details, entity_type: 'Project', entity_path: 'd/e') }
+
+    context 'with runner_authentication_token' do
+      let(:details) do
+        { runner_authentication_token: 'abc1234567' }
+      end
+
+      it 'returns CiRunnerTokenAuthor with expected attributes' do
+        is_expected.to have_attributes(id: -1, name: 'Authentication token: abc1234567')
+      end
+    end
+
+    context 'with runner_registration_token' do
+      let(:details) do
+        { runner_registration_token: 'abc1234567' }
+      end
+
+      it 'returns CiRunnerTokenAuthor with expected attributes' do
+        is_expected.to have_attributes(id: -1, name: 'Registration token: abc1234567')
+      end
+    end
+
+    context 'with runner token missing' do
+      let(:details) do
+        {}
+      end
+
+      it 'raises ArgumentError' do
+        expect { subject }.to raise_error ArgumentError, 'Runner token missing'
+      end
     end
   end
 
   describe '#full_path' do
     subject { author.full_path }
 
+    let(:author) { described_class.new(audit_event) }
+
     context 'with instance registration token' do
-      let(:author) { described_class.new(token: 'abc1234567', entity_type: 'User', entity_path: nil) }
+      let(:audit_event) { instance_double(AuditEvent, details: { runner_registration_token: 'abc1234567' }, entity_type: 'User', entity_path: nil) }
 
       it 'returns correct url' do
         is_expected.to eq('/admin/runners')
@@ -22,7 +54,7 @@ RSpec.describe Gitlab::Audit::CiRunnerTokenAuthor do
     end
 
     context 'with group registration token' do
-      let(:author) { described_class.new(token: 'abc1234567', entity_type: 'Group', entity_path: 'a/b') }
+      let(:audit_event) { instance_double(AuditEvent, details: { runner_registration_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)
@@ -35,7 +67,7 @@ RSpec.describe Gitlab::Audit::CiRunnerTokenAuthor do
     end
 
     context 'with project registration token' do
-      let(:author) { described_class.new(token: 'abc1234567', entity_type: 'Project', entity_path: project.full_path) }
+      let(:audit_event) { instance_double(AuditEvent, details: { runner_registration_token: 'abc1234567' }, entity_type: 'Project', entity_path: project.full_path) }
       let(:project) { create(:project) }
 
       it 'returns correct url' do

spec/lib/gitlab/audit/null_author_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe Gitlab::Audit::NullAuthor do
     it 'returns an DeletedAuthor' do
       allow(audit_event).to receive(:[]).with(:author_name).and_return('Old Hat')
       allow(audit_event).to receive(:details).and_return({})
+      allow(audit_event).to receive(:target_type)
 
       expect(subject.for(666, audit_event)).to be_a(Gitlab::Audit::DeletedAuthor)
     end
@@ -18,6 +19,7 @@ RSpec.describe Gitlab::Audit::NullAuthor do
     it 'returns an UnauthenticatedAuthor when id equals -1', :aggregate_failures do
       allow(audit_event).to receive(:[]).with(:author_name).and_return('Frank')
       allow(audit_event).to receive(:details).and_return({})
+      allow(audit_event).to receive(:target_type)
 
       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')
@@ -27,12 +29,25 @@ RSpec.describe Gitlab::Audit::NullAuthor 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(:target_type).and_return(::Ci::Runner.name)
       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::CiRunnerTokenAuthor)
       expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Registration token: cde456')
     end
+
+    it 'returns a CiRunnerTokenAuthor when details contain runner authentication 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(:target_type).and_return(::Ci::Runner.name)
+      allow(audit_event).to receive(:details)
+        .and_return({ runner_authentication_token: 'cde456', author_name: 'cde456', entity_type: 'User', entity_path: '/a/b' })
+
+      expect(subject.for(-1, audit_event)).to be_a(Gitlab::Audit::CiRunnerTokenAuthor)
+      expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Authentication token: cde456')
+    end
   end
 
   describe '#current_sign_in_ip' do

spec/models/audit_event_spec.rb

@@ -97,8 +97,8 @@ RSpec.describe AuditEvent do
   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 }) }
+    context "when the target type is not Ci::Runner" do
+      let(:audit_event) { build(:project_audit_event, target_id: 678) }
 
       it 'returns a NullAuthor' do
         expect(::Gitlab::Audit::NullAuthor).to receive(:for)
@@ -109,12 +109,12 @@ RSpec.describe AuditEvent do
       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' }) }
+    context 'when the target type is Ci::Runner and details contain runner_registration_token' do
+      let(:audit_event) { build(:project_audit_event, target_type: ::Ci::Runner.name, target_id: 678, details: { runner_registration_token: 'abc123' }) }
 
       it 'returns a CiRunnerTokenAuthor' do
         expect(::Gitlab::Audit::CiRunnerTokenAuthor).to receive(:new)
-          .with({ token: 'abc123', entity_type: 'Project', entity_path: audit_event.entity_path })
+          .with(audit_event)
           .and_call_original
           .once