From d0fdb9dc462cb237e905b296f71d13a0ad24d15f Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Wed, 29 Mar 2023 10:34:01 +0000 Subject: [PATCH 1/4] Audit ban action Add user ban audit event at the instance level Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116103 EE: true --- app/services/users/ban_service.rb | 2 + ee/app/services/ee/users/ban_service.rb | 29 +++++++++ ee/config/audit_events/types/ban_user.yml | 8 +++ ee/spec/services/ee/users/ban_service_spec.rb | 62 +++++++++++++++++++ 4 files changed, 101 insertions(+) create mode 100644 ee/app/services/ee/users/ban_service.rb create mode 100644 ee/config/audit_events/types/ban_user.yml create mode 100644 ee/spec/services/ee/users/ban_service_spec.rb diff --git a/app/services/users/ban_service.rb b/app/services/users/ban_service.rb index 959d4be37956c1..5ed31cdb778f84 100644 --- a/app/services/users/ban_service.rb +++ b/app/services/users/ban_service.rb @@ -17,3 +17,5 @@ def action end end end + +Users::BanService.prepend_mod_with('Users::BanService') diff --git a/ee/app/services/ee/users/ban_service.rb b/ee/app/services/ee/users/ban_service.rb new file mode 100644 index 00000000000000..0a2b7039fd7a23 --- /dev/null +++ b/ee/app/services/ee/users/ban_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module EE + module Users + module BanService + extend ::Gitlab::Utils::Override + + override :update_user + def update_user(user) + super && log_audit_event(user) + end + + private + + def log_audit_event(user) + audit_context = { + name: "ban_user", + author: current_user, + scope: user, + target: user, + target_details: user.full_path, + message: "Banned user" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end +end diff --git a/ee/config/audit_events/types/ban_user.yml b/ee/config/audit_events/types/ban_user.yml new file mode 100644 index 00000000000000..d8efb65c476635 --- /dev/null +++ b/ee/config/audit_events/types/ban_user.yml @@ -0,0 +1,8 @@ +name: ban_user +description: Event triggered on user ban action +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377620 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116103 +feature_category: "user_management" +milestone: "15.11" +saved_to_database: true +streamed: true \ No newline at end of file diff --git a/ee/spec/services/ee/users/ban_service_spec.rb b/ee/spec/services/ee/users/ban_service_spec.rb new file mode 100644 index 00000000000000..a78bf84bd21768 --- /dev/null +++ b/ee/spec/services/ee/users/ban_service_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::BanService, feature_category: :user_management do + let_it_be(:current_user) { create(:admin) } + + subject(:service) { described_class.new(current_user) } + + describe '#execute' do + let!(:user) { create(:user) } + + subject(:operation) { service.execute(user) } + + describe 'audit events' do + context 'when licensed', :enable_admin_mode do + before do + stub_licensed_features(admin_audit_log: true) + end + + context 'when user ban operation succeeds' do + it 'logs an audit event' do + expect { operation }.to change { AuditEvent.count }.by(1) + end + + it 'logs the audit event info' do + operation + expect(AuditEvent.last).to have_attributes( + details: hash_including(custom_message: 'Banned user') + ) + end + end + + context 'when user ban operation fails' do + let!(:user) { create(:user) } + + before do + allow(user).to receive(:ban).and_return(false) + end + + it 'does not log any audit event' do + expect { operation }.not_to change { AuditEvent.count } + end + end + end + + context 'when not licensed' do + before do + stub_licensed_features( + admin_audit_log: false, + audit_events: false, + extended_audit_events: false + ) + end + + it 'does not log any audit event' do + expect { operation }.not_to change { AuditEvent.count } + end + end + end + end +end -- GitLab From ae94bb8a3d55cbbe2dc50bcafa47c5b350c9820c Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Wed, 29 Mar 2023 11:58:07 +0000 Subject: [PATCH 2/4] Alter update user method --- ee/app/services/ee/users/ban_service.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/app/services/ee/users/ban_service.rb b/ee/app/services/ee/users/ban_service.rb index 0a2b7039fd7a23..d7f89cc44b7269 100644 --- a/ee/app/services/ee/users/ban_service.rb +++ b/ee/app/services/ee/users/ban_service.rb @@ -6,8 +6,10 @@ module BanService extend ::Gitlab::Utils::Override override :update_user - def update_user(user) - super && log_audit_event(user) + def update_user(user) + super.tap do |result| + log_audit_event(user) if result.present? + end end private -- GitLab From ceffe2a948570ff8d7a57f02deedeb5b2a76c1b2 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Wed, 29 Mar 2023 12:02:12 +0000 Subject: [PATCH 3/4] Fix rubocop failures --- ee/app/services/ee/users/ban_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/services/ee/users/ban_service.rb b/ee/app/services/ee/users/ban_service.rb index d7f89cc44b7269..e49351053eed66 100644 --- a/ee/app/services/ee/users/ban_service.rb +++ b/ee/app/services/ee/users/ban_service.rb @@ -6,7 +6,7 @@ module BanService extend ::Gitlab::Utils::Override override :update_user - def update_user(user) + def update_user(user) super.tap do |result| log_audit_event(user) if result.present? end -- GitLab From ec15dd68af70a94a3c41d907c9a41baec8a4505d Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Wed, 29 Mar 2023 18:13:09 +0000 Subject: [PATCH 4/4] Update target details --- ee/app/services/ee/users/ban_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/services/ee/users/ban_service.rb b/ee/app/services/ee/users/ban_service.rb index e49351053eed66..f5958fbd4800be 100644 --- a/ee/app/services/ee/users/ban_service.rb +++ b/ee/app/services/ee/users/ban_service.rb @@ -20,7 +20,7 @@ def log_audit_event(user) author: current_user, scope: user, target: user, - target_details: user.full_path, + target_details: user.username, message: "Banned user" } -- GitLab