From 45d8bc02374aefcf852f1a61b8849a11db930b6f Mon Sep 17 00:00:00 2001
From: Harsimar Sandhu <hsandhu@gitlab.com>
Date: Mon, 1 Apr 2024 21:54:23 +0000
Subject: [PATCH] New Audit Event when approval rule is updated
New audit event when project approval rule name or
target branch is changed
EE: true
Changelog: added
---
.../concerns/approval_rules/updater.rb | 58 +++++++-
.../approval_rules/update_service_spec.rb | 134 ++++++++++++++++++
2 files changed, 185 insertions(+), 7 deletions(-)
diff --git a/ee/app/services/concerns/approval_rules/updater.rb b/ee/app/services/concerns/approval_rules/updater.rb
index 6340124d92845..ab641a686209a 100644
--- a/ee/app/services/concerns/approval_rules/updater.rb
+++ b/ee/app/services/concerns/approval_rules/updater.rb
@@ -4,6 +4,9 @@ module ApprovalRules
module Updater
include ::Audit::Changes
+ APPROVAL_RULE_UPDATE_EVENT_NAME = 'update_approval_rules'
+ APPROVAL_RULE_CREATE_EVENT_NAME = 'approval_rule_created'
+
def execute
if group_rule? && Feature.disabled?(:approval_group_rules, rule.group)
return ServiceResponse.error(message: "The feature approval_group_rules is not enabled.")
@@ -29,7 +32,11 @@ def action
filter_eligible_groups!
filter_eligible_protected_branches!
- update_rule ? success : error
+ old_protected_branches_names = nil
+
+ old_protected_branches_names = rule.protected_branches.map(&:name) if rule.is_a? ApprovalProjectRule
+
+ update_rule ? success_with_audit_logging(old_protected_branches_names) : error
end
def filter_eligible_users!
@@ -85,7 +92,7 @@ def update_rule
return rule.update(params) unless current_user
audit_context = {
- name: rule.new_record? ? 'approval_rule_created' : 'update_approval_rules',
+ name: rule.new_record? ? APPROVAL_RULE_CREATE_EVENT_NAME : APPROVAL_RULE_UPDATE_EVENT_NAME,
author: current_user,
scope: container,
target: rule
@@ -94,21 +101,40 @@ def update_rule
::Gitlab::Audit::Auditor.audit(audit_context) { rule.update(params) }
end
- def success
- audit_changes_to_approvals_required if current_user
+ def success_with_audit_logging(old_protected_branches_names)
+ log_audit_changes(old_protected_branches_names) if current_user
rule.reset
- super
+ success
end
- def audit_changes_to_approvals_required
+ def log_audit_changes(old_protected_branches_names)
audit_changes(
:approvals_required,
as: 'number of required approvals',
entity: container,
model: rule,
- event_type: 'update_approval_rules'
+ event_type: APPROVAL_RULE_UPDATE_EVENT_NAME
+ )
+ audit_changes(:name,
+ as: 'name',
+ entity: container,
+ model: rule,
+ event_type: APPROVAL_RULE_UPDATE_EVENT_NAME)
+
+ return unless rule.is_a? ApprovalProjectRule
+
+ audit_message = protected_branch_change_audit_message(rule, old_protected_branches_names)
+
+ return unless audit_message.present?
+
+ ::Gitlab::Audit::Auditor.audit(
+ author: current_user,
+ name: APPROVAL_RULE_UPDATE_EVENT_NAME,
+ scope: container,
+ target: rule,
+ message: audit_message
)
end
@@ -118,5 +144,23 @@ def can_create_rule_for_protected_branches?
skip_authorization || can?(current_user, :admin_project, project)
end
+
+ def protected_branch_change_audit_message(rule, old_protected_branches_names)
+ new_protected_branches_names = rule.protected_branches.map(&:name)
+ recently_added_branch = new_protected_branches_names - old_protected_branches_names
+ enabled_all_protected_branches = rule.previous_changes["applies_to_all_protected_branches"] == [false, true]
+ disabled_all_protected_branches = rule.previous_changes["applies_to_all_protected_branches"] == [true, false]
+ from_protected_branch_to_empty = (rule.protected_branches.empty? && old_protected_branches_names.present?)
+
+ if enabled_all_protected_branches
+ "Changed target branch to all protected branches"
+ elsif disabled_all_protected_branches && new_protected_branches_names.present?
+ "Changed target branch to #{new_protected_branches_names.first} branch"
+ elsif from_protected_branch_to_empty
+ "Changed target branch to all branches"
+ elsif recently_added_branch.present?
+ "Changed target branch to #{recently_added_branch.first} branch"
+ end
+ end
end
end
diff --git a/ee/spec/services/approval_rules/update_service_spec.rb b/ee/spec/services/approval_rules/update_service_spec.rb
index 862b7f4841311..a17fd2b3e885e 100644
--- a/ee/spec/services/approval_rules/update_service_spec.rb
+++ b/ee/spec/services/approval_rules/update_service_spec.rb
@@ -311,6 +311,140 @@
"Removed User Batman from approval group on Gotham rule"
)
end
+
+ it 'audits the name change of a approval rule' do
+ described_class.new(approval_rule, user, name: 'Avenger').execute
+
+ expect(AuditEvent.last).to have_attributes(
+ author: user,
+ entity: project,
+ target_id: approval_rule.id,
+ target_type: approval_rule.class.name,
+ target_details: approval_rule.name,
+ details: include(custom_message: "Changed name from Gotham to Avenger")
+ )
+ end
+
+ context 'when changing target branch' do
+ before do
+ stub_licensed_features(multiple_approval_rules: true)
+ end
+
+ let(:protected_branch) { create(:protected_branch, project: target) }
+
+ context 'when new target branch is applied' do
+ subject(:execute) do
+ described_class.new(
+ approval_rule,
+ user,
+ protected_branch_ids: [protected_branch.id]
+ ).execute
+ end
+
+ it 'calls auditor with correct args' do
+ expect(::Gitlab::Audit::Auditor).to receive(:audit).ordered.and_call_original
+
+ expect(::Gitlab::Audit::Auditor).to receive(:audit).with(
+ hash_including(
+ author: user,
+ name: 'update_approval_rules',
+ scope: project,
+ target: approval_rule,
+ message: "Changed target branch to #{protected_branch.name} branch"
+ )
+ ).ordered
+
+ execute
+ end
+ end
+
+ context 'when target branch is changed to all branches' do
+ before do
+ approval_rule.update!(protected_branches: [protected_branch])
+ end
+
+ subject(:execute) do
+ described_class.new(
+ approval_rule,
+ user,
+ protected_branch_ids: []
+ ).execute
+ end
+
+ it 'calls auditor with correct args' do
+ expect(::Gitlab::Audit::Auditor).to receive(:audit).ordered.and_call_original
+
+ expect(::Gitlab::Audit::Auditor).to receive(:audit).with(
+ hash_including(
+ author: user,
+ name: 'update_approval_rules',
+ scope: project,
+ target: approval_rule,
+ message: "Changed target branch to all branches"
+ )
+ ).ordered
+
+ execute
+ end
+ end
+
+ context 'when target branch is changed to all protected branches' do
+ subject(:execute) do
+ described_class.new(
+ approval_rule,
+ user,
+ applies_to_all_protected_branches: true
+ ).execute
+ end
+
+ it 'calls auditor with correct args' do
+ expect(::Gitlab::Audit::Auditor).to receive(:audit).ordered.and_call_original
+
+ expect(::Gitlab::Audit::Auditor).to receive(:audit).with(
+ hash_including(
+ author: user,
+ name: 'update_approval_rules',
+ scope: project,
+ target: approval_rule,
+ message: "Changed target branch to all protected branches"
+ )
+ ).ordered
+
+ execute
+ end
+ end
+
+ context 'when target branch is changed from all protected branches to a protected branch' do
+ before do
+ approval_rule.update!(applies_to_all_protected_branches: true)
+ end
+
+ subject(:execute) do
+ described_class.new(
+ approval_rule,
+ user,
+ protected_branch_ids: [protected_branch.id],
+ applies_to_all_protected_branches: false
+ ).execute
+ end
+
+ it 'calls auditor with correct args' do
+ expect(::Gitlab::Audit::Auditor).to receive(:audit).ordered.and_call_original
+
+ expect(::Gitlab::Audit::Auditor).to receive(:audit).with(
+ hash_including(
+ author: user,
+ name: 'update_approval_rules',
+ scope: project,
+ target: approval_rule,
+ message: "Changed target branch to #{protected_branch.name} branch"
+ )
+ ).ordered
+
+ execute
+ end
+ end
+ end
end
context 'when rule update operation fails', :request_store do
--
GitLab