From 92b0906d64b5c8ec380fb6241bb83e2f4ac9069e Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Mon, 14 Feb 2022 14:19:12 +0100 Subject: [PATCH 1/5] Split CiRunnerTokenAuthor class This allows representing different types of runner tokens, e.g. authentication and registration tokens. --- .../ci_runner_authentication_token_author.rb | 15 +++++ .../ci_runner_registration_token_author.rb | 15 +++++ lib/gitlab/audit/ci_runner_token_author.rb | 34 ++++++++++- lib/gitlab/audit/null_author.rb | 8 +-- ...runner_authentication_token_author_spec.rb | 19 +++++++ ...i_runner_registration_token_author_spec.rb | 19 +++++++ .../ci_runner_token_author_shared_examples.rb | 40 +++++++++++++ .../audit/ci_runner_token_author_spec.rb | 56 +++++++++---------- spec/lib/gitlab/audit/null_author_spec.rb | 19 ++++++- spec/models/audit_event_spec.rb | 14 ++--- 10 files changed, 193 insertions(+), 46 deletions(-) create mode 100644 lib/gitlab/audit/ci_runner_authentication_token_author.rb create mode 100644 lib/gitlab/audit/ci_runner_registration_token_author.rb create mode 100644 spec/lib/gitlab/audit/ci_runner_authentication_token_author_spec.rb create mode 100644 spec/lib/gitlab/audit/ci_runner_registration_token_author_spec.rb create mode 100644 spec/lib/gitlab/audit/ci_runner_token_author_shared_examples.rb diff --git a/lib/gitlab/audit/ci_runner_authentication_token_author.rb b/lib/gitlab/audit/ci_runner_authentication_token_author.rb new file mode 100644 index 00000000000000..dfba85d8fea52f --- /dev/null +++ b/lib/gitlab/audit/ci_runner_authentication_token_author.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Gitlab + module Audit + class CiRunnerAuthenticationTokenAuthor < CiRunnerTokenAuthor + def self.token_field + :runner_authentication_token + end + + def message(token) + "Authentication token: #{token}" + end + end + end +end diff --git a/lib/gitlab/audit/ci_runner_registration_token_author.rb b/lib/gitlab/audit/ci_runner_registration_token_author.rb new file mode 100644 index 00000000000000..6453a286d270e1 --- /dev/null +++ b/lib/gitlab/audit/ci_runner_registration_token_author.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Gitlab + module Audit + class CiRunnerRegistrationTokenAuthor < CiRunnerTokenAuthor + def self.token_field + :runner_registration_token + end + + def message(token) + "Registration token: #{token}" + end + end + end +end diff --git a/lib/gitlab/audit/ci_runner_token_author.rb b/lib/gitlab/audit/ci_runner_token_author.rb index cc140a29260311..8a711a0471a13c 100644 --- a/lib/gitlab/audit/ci_runner_token_author.rb +++ b/lib/gitlab/audit/ci_runner_token_author.rb @@ -3,8 +3,34 @@ module Gitlab module Audit class CiRunnerTokenAuthor < Gitlab::Audit::NullAuthor + class << self + def for(audit_event) + subclass = subclass_for(audit_event.details) + subclass.new( + token: audit_event.details[subclass.token_field], + entity_type: audit_event.entity_type, + entity_path: audit_event.entity_path + ) + end + + private + + def subclass_for(details) + if details.include?(:runner_authentication_token) + CiRunnerAuthenticationTokenAuthor + elsif details.include?(:runner_registration_token) + CiRunnerRegistrationTokenAuthor + end + end + end + + # Represents a CI Runner token (registration or authentication) + # + # @param [String] token the token that owns the operation + # @param [String] entity_type the scope of the runner - 'Group', 'Project' or nil for instance runners + # @param [String] entity_path full path to the entity associated with the runner def initialize(token:, entity_type:, entity_path:) - super(id: -1, name: "Registration token: #{token}") + super(id: -1, name: message(token)) @entity_type = entity_type @entity_path = entity_path @@ -23,6 +49,12 @@ def full_path url_helpers.admin_runners_path end end + + private + + def message(token) + raise NotImplementedError + end end end end diff --git a/lib/gitlab/audit/null_author.rb b/lib/gitlab/audit/null_author.rb index 64aec51471a920..adb7c2ce5fae0a 100644 --- a/lib/gitlab/audit/null_author.rb +++ b/lib/gitlab/audit/null_author.rb @@ -18,12 +18,8 @@ class NullAuthor 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.for(audit_event) elsif id == -1 Gitlab::Audit::UnauthenticatedAuthor.new(name: name) else diff --git a/spec/lib/gitlab/audit/ci_runner_authentication_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_authentication_token_author_spec.rb new file mode 100644 index 00000000000000..ad582d1fd273c5 --- /dev/null +++ b/spec/lib/gitlab/audit/ci_runner_authentication_token_author_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative 'ci_runner_token_author_shared_examples' + +RSpec.describe Gitlab::Audit::CiRunnerAuthenticationTokenAuthor do + describe '#initialize' do + it 'sets correct attributes' do + expect(described_class.new(token: 'abc1234568', entity_type: 'Project', entity_path: 'd/e')) + .to have_attributes(id: -1, name: 'Authentication token: abc1234568') + end + end + + describe '#full_path' do + subject { author.full_path } + + it_behaves_like 'an author returning correct urls for all scopes' + end +end diff --git a/spec/lib/gitlab/audit/ci_runner_registration_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_registration_token_author_spec.rb new file mode 100644 index 00000000000000..d2295c417e3448 --- /dev/null +++ b/spec/lib/gitlab/audit/ci_runner_registration_token_author_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative 'ci_runner_token_author_shared_examples' + +RSpec.describe Gitlab::Audit::CiRunnerRegistrationTokenAuthor 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 } + + it_behaves_like 'an author returning correct urls for all scopes' + end +end diff --git a/spec/lib/gitlab/audit/ci_runner_token_author_shared_examples.rb b/spec/lib/gitlab/audit/ci_runner_token_author_shared_examples.rb new file mode 100644 index 00000000000000..d1297e3f5841b5 --- /dev/null +++ b/spec/lib/gitlab/audit/ci_runner_token_author_shared_examples.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples 'an author returning correct urls for all scopes' do + 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 diff --git a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb index 4d2356fc58e928..4270e4c1d8f1c4 100644 --- a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb +++ b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb @@ -3,48 +3,44 @@ 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') + # Workaround: Ensure #messages has coverage + describe '.initialize' do + subject { described_class.new(token: 'some_token', entity_type: nil, entity_path: nil) } + + specify 'raises NotImplementedError' do + expect { subject }.to raise_error NotImplementedError end end - describe '#full_path' do - subject { author.full_path } + describe '.for' do + subject { described_class.for(audit_event) } - context 'with instance registration token' do - let(:author) { described_class.new(token: 'abc1234567', entity_type: 'User', entity_path: nil) } + let(:details) { } + let(:audit_event) { instance_double(AuditEvent) } - it 'returns correct url' do - is_expected.to eq('/admin/runners') - end + before do + allow(audit_event).to receive(:details).and_return(details) + allow(audit_event).to receive(:entity_type).and_return('Project') + allow(audit_event).to receive(:entity_path).and_return('d/e') 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') + context 'with runner_authentication_token' do + let(:details) do + { runner_authentication_token: 'abc1234567' } + end - is_expected.to eq('/path/to/group/runners') + it 'returns CiRunnerAuthenticationTokenAuthor with expected attributes' do + is_expected.to have_attributes(id: -1, name: 'Authentication token: abc1234567') 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') + context 'with runner_registration_token' do + let(:details) do + { runner_registration_token: 'abc1234567' } + end - is_expected.to eq('/path/to/project/runners') + it 'returns CiRunnerRegistrationTokenAuthor with expected attributes' do + is_expected.to have_attributes(id: -1, name: 'Registration token: abc1234567') end end end diff --git a/spec/lib/gitlab/audit/null_author_spec.rb b/spec/lib/gitlab/audit/null_author_spec.rb index 51e4a744111ee7..776d9dcb4eb477 100644 --- a/spec/lib/gitlab/audit/null_author_spec.rb +++ b/spec/lib/gitlab/audit/null_author_spec.rb @@ -11,6 +11,7 @@ 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,21 +19,35 @@ 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') end - it 'returns a CiRunnerTokenAuthor when details contain runner registration token', :aggregate_failures do + it 'returns a CiRunnerRegistrationTokenAuthor 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(: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 be_a(Gitlab::Audit::CiRunnerRegistrationTokenAuthor) expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Registration token: cde456') end + + it 'returns a CiRunnerAuthenticationTokenAuthor 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::CiRunnerAuthenticationTokenAuthor) + expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Authentication token: cde456') + end end describe '#current_sign_in_ip' do diff --git a/spec/models/audit_event_spec.rb b/spec/models/audit_event_spec.rb index 957813ec3a0f30..42cd36feee6097 100644 --- a/spec/models/audit_event_spec.rb +++ b/spec/models/audit_event_spec.rb @@ -97,8 +97,8 @@ 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,16 +109,16 @@ 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 }) + expect(::Gitlab::Audit::CiRunnerRegistrationTokenAuthor).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::CiRunnerTokenAuthor) + is_expected.to be_an_instance_of(::Gitlab::Audit::CiRunnerRegistrationTokenAuthor) end it 'name consists of prefix and token' do -- GitLab From 6f3cea29aff2d53cbb27b4859ecbfe548266918e Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Tue, 15 Feb 2022 18:27:08 +0100 Subject: [PATCH 2/5] Apply MR review suggestion --- spec/lib/gitlab/audit/ci_runner_token_author_spec.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb index 4270e4c1d8f1c4..f7ab743a134e9c 100644 --- a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb +++ b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb @@ -16,13 +16,7 @@ subject { described_class.for(audit_event) } let(:details) { } - let(:audit_event) { instance_double(AuditEvent) } - - before do - allow(audit_event).to receive(:details).and_return(details) - allow(audit_event).to receive(:entity_type).and_return('Project') - allow(audit_event).to receive(:entity_path).and_return('d/e') - end + let(:audit_event) { instance_double(AuditEvent, details: details, entity_type: 'Project', entity_path: 'd/e') } context 'with runner_authentication_token' do let(:details) do -- GitLab From 1becdd94488622935afed8ff7d546b6c5d2beee7 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Wed, 16 Feb 2022 12:15:26 +0100 Subject: [PATCH 3/5] Address MR review comments --- .../ci_runner_authentication_token_author.rb | 15 ----- .../ci_runner_registration_token_author.rb | 15 ----- lib/gitlab/audit/ci_runner_token_author.rb | 49 +++++--------- lib/gitlab/audit/null_author.rb | 2 +- ...runner_authentication_token_author_spec.rb | 19 ------ ...i_runner_registration_token_author_spec.rb | 19 ------ .../ci_runner_token_author_shared_examples.rb | 40 ------------ .../audit/ci_runner_token_author_spec.rb | 64 +++++++++++++++---- spec/lib/gitlab/audit/null_author_spec.rb | 6 +- spec/models/audit_event_spec.rb | 6 +- 10 files changed, 75 insertions(+), 160 deletions(-) delete mode 100644 lib/gitlab/audit/ci_runner_authentication_token_author.rb delete mode 100644 lib/gitlab/audit/ci_runner_registration_token_author.rb delete mode 100644 spec/lib/gitlab/audit/ci_runner_authentication_token_author_spec.rb delete mode 100644 spec/lib/gitlab/audit/ci_runner_registration_token_author_spec.rb delete mode 100644 spec/lib/gitlab/audit/ci_runner_token_author_shared_examples.rb diff --git a/lib/gitlab/audit/ci_runner_authentication_token_author.rb b/lib/gitlab/audit/ci_runner_authentication_token_author.rb deleted file mode 100644 index dfba85d8fea52f..00000000000000 --- a/lib/gitlab/audit/ci_runner_authentication_token_author.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Audit - class CiRunnerAuthenticationTokenAuthor < CiRunnerTokenAuthor - def self.token_field - :runner_authentication_token - end - - def message(token) - "Authentication token: #{token}" - end - end - end -end diff --git a/lib/gitlab/audit/ci_runner_registration_token_author.rb b/lib/gitlab/audit/ci_runner_registration_token_author.rb deleted file mode 100644 index 6453a286d270e1..00000000000000 --- a/lib/gitlab/audit/ci_runner_registration_token_author.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Audit - class CiRunnerRegistrationTokenAuthor < CiRunnerTokenAuthor - def self.token_field - :runner_registration_token - end - - def message(token) - "Registration token: #{token}" - end - end - end -end diff --git a/lib/gitlab/audit/ci_runner_token_author.rb b/lib/gitlab/audit/ci_runner_token_author.rb index 8a711a0471a13c..5f83725b5a3035 100644 --- a/lib/gitlab/audit/ci_runner_token_author.rb +++ b/lib/gitlab/audit/ci_runner_token_author.rb @@ -3,37 +3,24 @@ module Gitlab module Audit class CiRunnerTokenAuthor < Gitlab::Audit::NullAuthor - class << self - def for(audit_event) - subclass = subclass_for(audit_event.details) - subclass.new( - token: audit_event.details[subclass.token_field], - entity_type: audit_event.entity_type, - entity_path: audit_event.entity_path - ) - end - - private - - def subclass_for(details) - if details.include?(:runner_authentication_token) - CiRunnerAuthenticationTokenAuthor - elsif details.include?(:runner_registration_token) - CiRunnerRegistrationTokenAuthor - end - end - end - # Represents a CI Runner token (registration or authentication) # - # @param [String] token the token that owns the operation - # @param [String] entity_type the scope of the runner - 'Group', 'Project' or nil for instance runners - # @param [String] entity_path full path to the entity associated with the runner - def initialize(token:, entity_type:, entity_path:) - super(id: -1, name: message(token)) + # @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 @@ -49,12 +36,6 @@ def full_path url_helpers.admin_runners_path end end - - private - - def message(token) - raise NotImplementedError - end end end end diff --git a/lib/gitlab/audit/null_author.rb b/lib/gitlab/audit/null_author.rb index adb7c2ce5fae0a..80e0c4ddf58c30 100644 --- a/lib/gitlab/audit/null_author.rb +++ b/lib/gitlab/audit/null_author.rb @@ -19,7 +19,7 @@ def self.for(id, audit_event) name = audit_event[:author_name] || audit_event.details[:author_name] if audit_event.target_type == ::Ci::Runner.name - Gitlab::Audit::CiRunnerTokenAuthor.for(audit_event) + Gitlab::Audit::CiRunnerTokenAuthor.new(audit_event) elsif id == -1 Gitlab::Audit::UnauthenticatedAuthor.new(name: name) else diff --git a/spec/lib/gitlab/audit/ci_runner_authentication_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_authentication_token_author_spec.rb deleted file mode 100644 index ad582d1fd273c5..00000000000000 --- a/spec/lib/gitlab/audit/ci_runner_authentication_token_author_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require_relative 'ci_runner_token_author_shared_examples' - -RSpec.describe Gitlab::Audit::CiRunnerAuthenticationTokenAuthor do - describe '#initialize' do - it 'sets correct attributes' do - expect(described_class.new(token: 'abc1234568', entity_type: 'Project', entity_path: 'd/e')) - .to have_attributes(id: -1, name: 'Authentication token: abc1234568') - end - end - - describe '#full_path' do - subject { author.full_path } - - it_behaves_like 'an author returning correct urls for all scopes' - end -end diff --git a/spec/lib/gitlab/audit/ci_runner_registration_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_registration_token_author_spec.rb deleted file mode 100644 index d2295c417e3448..00000000000000 --- a/spec/lib/gitlab/audit/ci_runner_registration_token_author_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require_relative 'ci_runner_token_author_shared_examples' - -RSpec.describe Gitlab::Audit::CiRunnerRegistrationTokenAuthor 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 } - - it_behaves_like 'an author returning correct urls for all scopes' - end -end diff --git a/spec/lib/gitlab/audit/ci_runner_token_author_shared_examples.rb b/spec/lib/gitlab/audit/ci_runner_token_author_shared_examples.rb deleted file mode 100644 index d1297e3f5841b5..00000000000000 --- a/spec/lib/gitlab/audit/ci_runner_token_author_shared_examples.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.shared_examples 'an author returning correct urls for all scopes' do - 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 diff --git a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb index f7ab743a134e9c..096631f83f71e1 100644 --- a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb +++ b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb @@ -3,17 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Audit::CiRunnerTokenAuthor do - # Workaround: Ensure #messages has coverage describe '.initialize' do - subject { described_class.new(token: 'some_token', entity_type: nil, entity_path: nil) } - - specify 'raises NotImplementedError' do - expect { subject }.to raise_error NotImplementedError - end - end - - describe '.for' do - subject { described_class.for(audit_event) } + subject { described_class.new(audit_event) } let(:details) { } let(:audit_event) { instance_double(AuditEvent, details: details, entity_type: 'Project', entity_path: 'd/e') } @@ -33,9 +24,60 @@ { runner_registration_token: 'abc1234567' } end - it 'returns CiRunnerRegistrationTokenAuthor with expected attributes' do + 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(: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') + end + end + + context 'with group registration token' do + 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) + .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(: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 + 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 diff --git a/spec/lib/gitlab/audit/null_author_spec.rb b/spec/lib/gitlab/audit/null_author_spec.rb index 776d9dcb4eb477..0c06b27a951a69 100644 --- a/spec/lib/gitlab/audit/null_author_spec.rb +++ b/spec/lib/gitlab/audit/null_author_spec.rb @@ -25,7 +25,7 @@ expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Frank') end - it 'returns a CiRunnerRegistrationTokenAuthor when details contain runner registration token', :aggregate_failures do + it 'returns a CiRunnerTokenAuthor 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') @@ -33,7 +33,7 @@ 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::CiRunnerRegistrationTokenAuthor) + 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 @@ -45,7 +45,7 @@ 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::CiRunnerAuthenticationTokenAuthor) + 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 diff --git a/spec/models/audit_event_spec.rb b/spec/models/audit_event_spec.rb index 42cd36feee6097..9f2724cebeee6e 100644 --- a/spec/models/audit_event_spec.rb +++ b/spec/models/audit_event_spec.rb @@ -113,12 +113,12 @@ 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::CiRunnerRegistrationTokenAuthor).to receive(:new) - .with(token: 'abc123', entity_type: 'Project', entity_path: audit_event.entity_path) + expect(::Gitlab::Audit::CiRunnerTokenAuthor).to receive(:new) + .with(audit_event) .and_call_original .once - is_expected.to be_an_instance_of(::Gitlab::Audit::CiRunnerRegistrationTokenAuthor) + is_expected.to be_an_instance_of(::Gitlab::Audit::CiRunnerTokenAuthor) end it 'name consists of prefix and token' do -- GitLab From 5212fbf4b9d8847b3cb9456391042c05c87e5b18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20K=C3=A4ppler?= Date: Wed, 16 Feb 2022 12:19:36 +0000 Subject: [PATCH 4/5] Apply 1 suggestion(s) to 1 file(s) --- spec/lib/gitlab/audit/ci_runner_token_author_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb index 096631f83f71e1..f55e1b4493656d 100644 --- a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb +++ b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb @@ -14,7 +14,7 @@ { runner_authentication_token: 'abc1234567' } end - it 'returns CiRunnerAuthenticationTokenAuthor with expected attributes' do + it 'returns CiRunnerTokenAuthor with expected attributes' do is_expected.to have_attributes(id: -1, name: 'Authentication token: abc1234567') end end -- GitLab From 0560bf687ac09521bb17d3d8b774e4ba4b5e98eb Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Wed, 16 Feb 2022 12:15:26 +0100 Subject: [PATCH 5/5] Address MR review comments --- spec/lib/gitlab/audit/null_author_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/audit/null_author_spec.rb b/spec/lib/gitlab/audit/null_author_spec.rb index 0c06b27a951a69..7203a0cd816623 100644 --- a/spec/lib/gitlab/audit/null_author_spec.rb +++ b/spec/lib/gitlab/audit/null_author_spec.rb @@ -37,7 +37,7 @@ expect(subject.for(-1, audit_event)).to have_attributes(id: -1, name: 'Registration token: cde456') end - it 'returns a CiRunnerAuthenticationTokenAuthor when details contain runner authentication token', :aggregate_failures do + 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') -- GitLab