From d2f89b6e683d96d1d9cc4965f34deaa5c90093f5 Mon Sep 17 00:00:00 2001 From: nrosandich Date: Wed, 21 Dec 2022 15:23:03 +1300 Subject: [PATCH 1/3] Add audit event for protected branches --- .../audit/protected_branches_changes_auditor.rb | 16 ++++++++++++---- .../protected_branches_changes_auditor_spec.rb | 12 +++++++++--- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/ee/lib/audit/protected_branches_changes_auditor.rb b/ee/lib/audit/protected_branches_changes_auditor.rb index 6563a35939ccb7..9899f40bd1cf47 100644 --- a/ee/lib/audit/protected_branches_changes_auditor.rb +++ b/ee/lib/audit/protected_branches_changes_auditor.rb @@ -9,10 +9,18 @@ def initialize(current_user, model, old_merge_access_levels, old_push_access_lev end def execute - audit_changes(:allow_force_push, as: 'allow force push', entity: model.project, - model: model, event_type: event_type) - audit_changes(:code_owner_approval_required, as: 'code owner approval required', - entity: model.project, model: model, event_type: event_type) + audit_changes( + :allow_force_push, + as: 'allow force push', + entity: model.project, + model: model, event_type: 'protected_branch_allow_force_push_updated' + ) + audit_changes( + :code_owner_approval_required, + as: 'code owner approval required', + entity: model.project, model: model, + event_type: 'protected_branch_code_owner_approval_required_updated' + ) audit_access_levels end diff --git a/ee/spec/lib/audit/protected_branches_changes_auditor_spec.rb b/ee/spec/lib/audit/protected_branches_changes_auditor_spec.rb index d142ab8ba47720..f2b9839a61518f 100644 --- a/ee/spec/lib/audit/protected_branches_changes_auditor_spec.rb +++ b/ee/spec/lib/audit/protected_branches_changes_auditor_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Audit::ProtectedBranchesChangesAuditor, :request_store do +RSpec.describe Audit::ProtectedBranchesChangesAuditor, :request_store, feature_category: :audit_events do let_it_be(:author) { create(:user, :with_sign_ins) } let_it_be(:user) { create(:user, :with_sign_ins) } let_it_be(:group) { create(:group) } @@ -35,12 +35,12 @@ shared_examples 'settings' do |setting| context "when #{setting} changed" do + change_text = setting.to_s.humanize(capitalize: false) it 'creates an event' do protected_branch.update_attribute(setting, true) expect { service.execute }.to change(AuditEvent, :count).by(1) event = AuditEvent.last - change_text = setting.to_s.humanize(capitalize: false) expect(event.details).to eq({ change: change_text, author_name: author.name, author_class: author.class.name, @@ -62,7 +62,13 @@ it 'streams correct audit event stream' do protected_branch.update_attribute(setting, true) - expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with('protected_branch_updated', + event_type = if change_text === 'allow force push' + "protected_branch_allow_force_push_updated" + else + "protected_branch_code_owner_approval_required_updated" + end + + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with(event_type, anything, anything) -- GitLab From f5d5e57d066f438e8555c0c1b086861caee7b60a Mon Sep 17 00:00:00 2001 From: nrosandich Date: Fri, 23 Dec 2022 11:14:11 +1300 Subject: [PATCH 2/3] Pass event type name --- .../protected_branches_changes_auditor_spec.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/ee/spec/lib/audit/protected_branches_changes_auditor_spec.rb b/ee/spec/lib/audit/protected_branches_changes_auditor_spec.rb index f2b9839a61518f..af60093f32179d 100644 --- a/ee/spec/lib/audit/protected_branches_changes_auditor_spec.rb +++ b/ee/spec/lib/audit/protected_branches_changes_auditor_spec.rb @@ -33,7 +33,7 @@ subject(:service) { described_class.new(author, protected_branch, old_merge_access_levels, old_push_access_levels) } - shared_examples 'settings' do |setting| + shared_examples 'settings' do |setting, expected_event_type| context "when #{setting} changed" do change_text = setting.to_s.humanize(capitalize: false) it 'creates an event' do @@ -62,13 +62,7 @@ it 'streams correct audit event stream' do protected_branch.update_attribute(setting, true) - event_type = if change_text === 'allow force push' - "protected_branch_allow_force_push_updated" - else - "protected_branch_code_owner_approval_required_updated" - end - - expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with(event_type, + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with(expected_event_type, anything, anything) @@ -77,8 +71,8 @@ end end - include_examples 'settings', :allow_force_push - include_examples 'settings', :code_owner_approval_required + include_examples 'settings', :allow_force_push, 'protected_branch_allow_force_push_updated' + include_examples 'settings', :code_owner_approval_required, 'protected_branch_code_owner_approval_required_updated' where(:type, :old_access_levels, :new_access_levels, :change_text) do :push | ref(:old_push_access_levels) | ref(:new_push_access_levels) | 'allowed to push' -- GitLab From f07822d0d15fb48d1df7938eb18900d967fc5ad9 Mon Sep 17 00:00:00 2001 From: nrosandich Date: Sat, 24 Dec 2022 08:03:29 +1300 Subject: [PATCH 3/3] Remove unneeded def --- ee/lib/audit/protected_branches_changes_auditor.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ee/lib/audit/protected_branches_changes_auditor.rb b/ee/lib/audit/protected_branches_changes_auditor.rb index 9899f40bd1cf47..064bae0ebc4eec 100644 --- a/ee/lib/audit/protected_branches_changes_auditor.rb +++ b/ee/lib/audit/protected_branches_changes_auditor.rb @@ -40,7 +40,7 @@ def audit_access_levels scope: model.project, target: model, message: "Changed #{change} from #{from} to #{to}", - name: event_type, + name: 'protected_branch_updated', additional_details: { change: change, from: from, @@ -61,11 +61,5 @@ def attributes_from_auditable_model(column) to: new } end - - private - - def event_type - 'protected_branch_updated' - end end end -- GitLab