diff --git a/ee/lib/audit/changes.rb b/ee/lib/audit/changes.rb index fc6ff2937e9a0ba98d17bed5e1b1997fa7d653cb..640f11b1a66e432e4f2fa972a0037ad52715d07e 100644 --- a/ee/lib/audit/changes.rb +++ b/ee/lib/audit/changes.rb @@ -46,7 +46,13 @@ def not_recently_created? end def changed?(column) - model.previous_changes.has_key?(column) + return false unless model.previous_changes.has_key?(column) + + from = model.previous_changes[column].first + to = model.previous_changes[column].second + return false if from.blank? && to.blank? + + true end def changes(column) diff --git a/ee/lib/ee/audit/project_changes_auditor.rb b/ee/lib/ee/audit/project_changes_auditor.rb index 39f31a254b5ab77caca7e86f29a90e9b1989d91b..d05d991d7ffd64c51b712edb64ceb7838cccf51e 100644 --- a/ee/lib/ee/audit/project_changes_auditor.rb +++ b/ee/lib/ee/audit/project_changes_auditor.rb @@ -17,12 +17,36 @@ def execute audit_changes(:disable_overriding_approvers_per_merge_request, as: 'prevent users from modifying MR approval rules in merge requests', model: model) audit_changes(:require_password_to_approve, as: 'require user password for approvals', model: model) + audit_changes(:resolve_outdated_diff_discussions, as: 'resolve_outdated_diff_discussions', model: model) + audit_changes(:printing_merge_request_link_enabled, as: 'printing_merge_request_link_enabled', model: model) + audit_changes(:remove_source_branch_after_merge, as: 'remove_source_branch_after_merge', model: model) + audit_changes(:only_allow_merge_if_pipeline_succeeds, as: 'only_allow_merge_if_pipeline_succeeds', model: model) + audit_changes(:only_allow_merge_if_all_discussions_are_resolved, as: 'only_allow_merge_if_all_discussions_are_resolved', model: model) + audit_changes(:suggestion_commit_message, as: 'suggestion_commit_message', model: model) + + audit_merge_method audit_project_feature_changes audit_compliance_framework_changes + audit_project_setting_changes + audit_project_ci_cd_setting_changes end private + def audit_merge_method + return unless model.previous_changes.has_key?(:merge_requests_ff_only_enabled) || + model.previous_changes.has_key?(:merge_requests_rebase_enabled) + + merge_method_message = _("Changed merge method to %{merge_method}") % { merge_method: model.human_merge_method } + audit_context = { + author: @current_user, + scope: model, + target: model, + message: merge_method_message + } + ::Gitlab::Audit::Auditor.audit(audit_context) + end + def audit_project_feature_changes ::EE::Audit::ProjectFeatureChangesAuditor.new(@current_user, model.project_feature, model).execute end @@ -31,6 +55,14 @@ def audit_compliance_framework_changes ::EE::Audit::ComplianceFrameworkChangesAuditor.new(@current_user, model.compliance_framework_setting, model).execute end + def audit_project_setting_changes + ::EE::Audit::ProjectSettingChangesAuditor.new(@current_user, model.project_setting, model).execute + end + + def audit_project_ci_cd_setting_changes + ::EE::Audit::ProjectCiCdSettingChangesAuditor.new(@current_user, model.ci_cd_settings, model).execute + end + def attributes_from_auditable_model(column) case column when :name diff --git a/ee/lib/ee/audit/project_ci_cd_setting_changes_auditor.rb b/ee/lib/ee/audit/project_ci_cd_setting_changes_auditor.rb new file mode 100644 index 0000000000000000000000000000000000000000..e8a2d864f69f31bd907ab96bcd8f59a1063c75d9 --- /dev/null +++ b/ee/lib/ee/audit/project_ci_cd_setting_changes_auditor.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +module EE + module Audit + class ProjectCiCdSettingChangesAuditor < BaseChangesAuditor + def initialize(current_user, ci_cd_settings, project) + @project = project + + super(current_user, ci_cd_settings) + end + + def execute + return if model.blank? + + audit_changes(:merge_pipelines_enabled, as: 'merge_pipelines_enabled', entity: @project, model: model) + audit_changes(:merge_trains_enabled, as: 'merge_trains_enabled', entity: @project, model: model) + end + + def attributes_from_auditable_model(column) + { + from: model.previous_changes[column].first, + to: model.previous_changes[column].last, + target_details: @project.full_path + } + end + end + end +end diff --git a/ee/lib/ee/audit/project_setting_changes_auditor.rb b/ee/lib/ee/audit/project_setting_changes_auditor.rb new file mode 100644 index 0000000000000000000000000000000000000000..65a06595ba22e7f7a6a3f8e0ed0526ee74aa3716 --- /dev/null +++ b/ee/lib/ee/audit/project_setting_changes_auditor.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +module EE + module Audit + class ProjectSettingChangesAuditor < BaseChangesAuditor + def initialize(current_user, project_setting, project) + @project = project + + super(current_user, project_setting) + end + + def execute + return if model.blank? + + audit_changes(:squash_option, as: 'squash_option', entity: @project, model: model) + audit_changes(:allow_merge_on_skipped_pipeline, as: 'allow_merge_on_skipped_pipeline', entity: @project, + model: model) + end + + def attributes_from_auditable_model(column) + { + from: model.previous_changes[column].first, + to: model.previous_changes[column].last, + target_details: @project.full_path + } + end + end + end +end diff --git a/ee/spec/lib/ee/audit/project_changes_auditor_spec.rb b/ee/spec/lib/ee/audit/project_changes_auditor_spec.rb index a0bbd54e02dcbbd21afd955e2288ba086cf237b9..32b37188d672f977bdd2e1a6023b4f571cbe106b 100644 --- a/ee/spec/lib/ee/audit/project_changes_auditor_spec.rb +++ b/ee/spec/lib/ee/audit/project_changes_auditor_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe EE::Audit::ProjectChangesAuditor do + using RSpec::Parameterized::TableSyntax describe '.audit_changes' do let_it_be(:user) { create(:user) } @@ -146,6 +147,100 @@ ) end end + + context 'when auditable boolean column is changed' do + columns = %w[resolve_outdated_diff_discussions printing_merge_request_link_enabled + remove_source_branch_after_merge only_allow_merge_if_pipeline_succeeds + only_allow_merge_if_all_discussions_are_resolved] + columns.each do |column| + where(:prev_value, :new_value) do + true | false + false | true + end + + before do + project.update_attribute(column, prev_value) + end + + with_them do + it 'creates an audit event' do + project.update_attribute(column, new_value) + + expect { foo_instance.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + change: column, + from: prev_value, + to: new_value + }) + end + end + end + end + + it 'creates an event when suggestion_commit_message change' do + previous_value = project.suggestion_commit_message + new_value = "I'm a suggested commit message" + project.update!(suggestion_commit_message: new_value) + + expect { foo_instance.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + change: 'suggestion_commit_message', + from: previous_value, + to: new_value + }) + end + + it 'does not create an event when suggestion_commit_message change from nil to empty string' do + project.update!(suggestion_commit_message: "") + + expect { foo_instance.execute }.not_to change { AuditEvent.count } + end + + context 'when merge method is changed from Merge' do + where(:ff, :rebase, :method) do + true | true | 'Fast-forward' + true | false | 'Fast-forward' + false | true | 'Rebase merge' + end + + before do + project.update!(merge_requests_ff_only_enabled: false, merge_requests_rebase_enabled: false) + end + + with_them do + it 'creates an audit event' do + project.update!(merge_requests_ff_only_enabled: ff, merge_requests_rebase_enabled: rebase) + + expect { foo_instance.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + custom_message: "Changed merge method to #{method}" + }) + end + end + end + + context 'when merge method is changed to Merge' do + where(:ff, :rebase) do + true | true + true | false + false | true + end + + with_them do + before do + project.update!(merge_requests_ff_only_enabled: ff, merge_requests_rebase_enabled: rebase) + end + + it 'creates an Merge method audit event' do + project.update!(merge_requests_ff_only_enabled: false, merge_requests_rebase_enabled: false) + + expect { foo_instance.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + custom_message: "Changed merge method to Merge" + }) + end + end + end end end end diff --git a/ee/spec/lib/ee/audit/project_ci_cd_setting_changes_auditor_spec.rb b/ee/spec/lib/ee/audit/project_ci_cd_setting_changes_auditor_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..70a3c5f12998d7358d3b405e6500a7970e72e126 --- /dev/null +++ b/ee/spec/lib/ee/audit/project_ci_cd_setting_changes_auditor_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe EE::Audit::ProjectCiCdSettingChangesAuditor do + using RSpec::Parameterized::TableSyntax + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:ci_cd_settings) { project.ci_cd_settings } + let_it_be(:project_ci_cd_setting_changes_auditor) { described_class.new(user, ci_cd_settings, project) } + + before do + stub_licensed_features(extended_audit_events: true) + end + + context 'when auditable boolean column is changed' do + columns = %w[merge_trains_enabled merge_pipelines_enabled] + columns.each do |column| + context 'when column changes from boolean' do + where(:prev_value, :new_value) do + true | false + false | true + end + + before do + project.ci_cd_settings.update_attribute(column, prev_value) + end + + with_them do + it 'creates an audit event' do + project.ci_cd_settings.update_attribute(column, new_value) + + expect { project_ci_cd_setting_changes_auditor.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + change: column, + from: prev_value, + to: new_value + }) + end + end + end + + context 'when column changes to false from nil' do + before do + project.ci_cd_settings.update_attribute(column, nil) + end + + it 'does not create an audit event' do + project.ci_cd_settings.update_attribute(column, false) + + expect { project_ci_cd_setting_changes_auditor.execute }.not_to change { AuditEvent.count } + end + end + end + end + end +end diff --git a/ee/spec/lib/ee/audit/project_setting_changes_auditor_spec.rb b/ee/spec/lib/ee/audit/project_setting_changes_auditor_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..fe6af59faf22aff02a7a339cb3d3c1109dcb9dd2 --- /dev/null +++ b/ee/spec/lib/ee/audit/project_setting_changes_auditor_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe EE::Audit::ProjectSettingChangesAuditor do + using RSpec::Parameterized::TableSyntax + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:project_setting_changes_auditor) { described_class.new(user, project.project_setting, project) } + + before do + stub_licensed_features(extended_audit_events: true) + end + + context 'when project setting is updated' do + options = ProjectSetting.squash_options.keys + options.each do |prev_value| + options.each do |new_value| + context 'when squash option is changed' do + before do + project.project_setting.update!(squash_option: prev_value) + end + + next if new_value == prev_value + + it 'creates an audit event' do + project.project_setting.update!(squash_option: new_value) + + expect { project_setting_changes_auditor.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + change: 'squash_option', + from: prev_value, + to: new_value + }) + end + end + end + end + + context 'when allow_merge_on_skipped_pipeline is changed' do + where(:prev_value, :new_value) do + true | false + false | true + end + + before do + project.project_setting.update!(allow_merge_on_skipped_pipeline: prev_value) + end + + with_them do + it 'creates an audit event' do + project.project_setting.update!(allow_merge_on_skipped_pipeline: new_value) + + expect { project_setting_changes_auditor.execute }.to change { AuditEvent.count }.by(1) + expect(AuditEvent.last.details).to include({ + change: 'allow_merge_on_skipped_pipeline', + from: prev_value, + to: new_value + }) + end + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3c1ada932471ed85d57a5981f4296c0e93441260..c130d99a3128d616ac86bf4a1196757562c28c42 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7026,6 +7026,9 @@ msgstr "" msgid "Changed assignee(s)." msgstr "" +msgid "Changed merge method to %{merge_method}" +msgstr "" + msgid "Changed reviewer(s)." msgstr ""