From 703269750b8954fcc7013e78498001eba7942f02 Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Thu, 30 Mar 2023 06:43:19 +0000 Subject: [PATCH 1/3] Audit unban action Add user ban audit event at the instance level Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116221 EE: true --- app/services/users/unban_service.rb | 2 + doc/administration/audit_events.md | 3 + ee/app/services/ee/users/unban_service.rb | 31 ++++++++++ ee/config/audit_events/types/unban_user.yml | 8 +++ .../services/ee/users/unban_service_spec.rb | 62 +++++++++++++++++++ 5 files changed, 106 insertions(+) create mode 100644 ee/app/services/ee/users/unban_service.rb create mode 100644 ee/config/audit_events/types/unban_user.yml create mode 100644 ee/spec/services/ee/users/unban_service_spec.rb diff --git a/app/services/users/unban_service.rb b/app/services/users/unban_service.rb index 753a02fa752209..2019f7e82e16b3 100644 --- a/app/services/users/unban_service.rb +++ b/app/services/users/unban_service.rb @@ -17,3 +17,5 @@ def action end end end + +Users::UnbanService.prepend_mod_with('Users::UnbanService') diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index 97c1cbc7490228..f924d39354929e 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -356,6 +356,9 @@ The following user actions on a GitLab instance generate instance audit events: GitLab 15.1. - Enabled Admin Mode. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/362101) in GitLab 15.7. - All [group events](#group-events) and [project events](#project-events). +- User was unblocked using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/377620) in GitLab 15.11. +- User was banned using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/377620) in GitLab 15.11. +- User was unbanned using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/377620) in GitLab 15.11. Instance events can also be accessed using the [Instance Audit Events API](../api/audit_events.md#instance-audit-events). diff --git a/ee/app/services/ee/users/unban_service.rb b/ee/app/services/ee/users/unban_service.rb new file mode 100644 index 00000000000000..aac1bb61237ac9 --- /dev/null +++ b/ee/app/services/ee/users/unban_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module EE + module Users + module UnbanService + extend ::Gitlab::Utils::Override + + override :update_user + def update_user(user) + super.tap do |result| + log_audit_event(user) if result.present? + end + end + + private + + def log_audit_event(user) + audit_context = { + name: "unban_user", + author: current_user, + scope: user, + target: user, + target_details: user.username, + message: "Unbanned user" + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end +end diff --git a/ee/config/audit_events/types/unban_user.yml b/ee/config/audit_events/types/unban_user.yml new file mode 100644 index 00000000000000..417979463d0e2f --- /dev/null +++ b/ee/config/audit_events/types/unban_user.yml @@ -0,0 +1,8 @@ +name: unban_user +description: Event triggered on user unban action +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377620 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116221 +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/unban_service_spec.rb b/ee/spec/services/ee/users/unban_service_spec.rb new file mode 100644 index 00000000000000..0f0ff2fcad1fca --- /dev/null +++ b/ee/spec/services/ee/users/unban_service_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::UnbanService, 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, :banned) } + + 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 unban 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: 'Unbanned user') + ) + end + end + + context 'when user unban operation fails' do + let!(:user) { create(:user) } + + before do + allow(user).to receive(:unban).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 70595acd9057b3d17e5921d82226caa7c180899b Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Fri, 31 Mar 2023 14:40:53 +0000 Subject: [PATCH 2/3] Add ban and unban events --- doc/administration/audit_events.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index 9d51be5ebf4619..a600a4d7501321 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -357,6 +357,8 @@ The following user actions on a GitLab instance generate instance audit events: - Enabled Admin Mode. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/362101) in GitLab 15.7. - All [group events](#group-events) and [project events](#project-events). - User was unblocked using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115727) in GitLab 15.11. +- User was banned using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116103) in GitLab 15.11. +- User was unbanned using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116221) in GitLab 15.11. Instance events can also be accessed using the [Instance Audit Events API](../api/audit_events.md#instance-audit-events). -- GitLab From f284640baeda9afe410f4e31d8579d26ec90767c Mon Sep 17 00:00:00 2001 From: sameer shaik Date: Tue, 4 Apr 2023 06:35:22 +0000 Subject: [PATCH 3/3] Add user management service --- ee/app/services/ee/users/ban_service.rb | 23 ++------ .../ee/users/management_base_service.rb | 31 ++++++++++ ee/app/services/ee/users/unban_service.rb | 23 ++------ ee/spec/services/ee/users/ban_service_spec.rb | 59 ++++++------------- .../services/ee/users/unban_service_spec.rb | 59 ++++++------------- 5 files changed, 81 insertions(+), 114 deletions(-) create mode 100644 ee/app/services/ee/users/management_base_service.rb diff --git a/ee/app/services/ee/users/ban_service.rb b/ee/app/services/ee/users/ban_service.rb index f5958fbd4800be..41892d6ed497ea 100644 --- a/ee/app/services/ee/users/ban_service.rb +++ b/ee/app/services/ee/users/ban_service.rb @@ -4,27 +4,16 @@ module EE module Users module BanService extend ::Gitlab::Utils::Override - - override :update_user - def update_user(user) - super.tap do |result| - log_audit_event(user) if result.present? - end - end + include ManagementBaseService private - def log_audit_event(user) - audit_context = { - name: "ban_user", - author: current_user, - scope: user, - target: user, - target_details: user.username, - message: "Banned user" - } + def event_name + 'ban_user' + end - ::Gitlab::Audit::Auditor.audit(audit_context) + def event_message + 'Banned user' end end end diff --git a/ee/app/services/ee/users/management_base_service.rb b/ee/app/services/ee/users/management_base_service.rb new file mode 100644 index 00000000000000..4ff76c9893604d --- /dev/null +++ b/ee/app/services/ee/users/management_base_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module EE + module Users + module ManagementBaseService + extend ::Gitlab::Utils::Override + + override :update_user + def update_user(user) + super.tap do |result| + log_audit_event(user) if result.present? + end + end + + private + + def log_audit_event(user) + audit_context = { + name: event_name, + author: current_user, + scope: user, + target: user, + target_details: user.username, + message: event_message + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end + end +end diff --git a/ee/app/services/ee/users/unban_service.rb b/ee/app/services/ee/users/unban_service.rb index aac1bb61237ac9..34ec1b4b663593 100644 --- a/ee/app/services/ee/users/unban_service.rb +++ b/ee/app/services/ee/users/unban_service.rb @@ -4,27 +4,16 @@ module EE module Users module UnbanService extend ::Gitlab::Utils::Override - - override :update_user - def update_user(user) - super.tap do |result| - log_audit_event(user) if result.present? - end - end + include ManagementBaseService private - def log_audit_event(user) - audit_context = { - name: "unban_user", - author: current_user, - scope: user, - target: user, - target_details: user.username, - message: "Unbanned user" - } + def event_name + 'unban_user' + end - ::Gitlab::Audit::Auditor.audit(audit_context) + def event_message + 'Unbanned user' end end end diff --git a/ee/spec/services/ee/users/ban_service_spec.rb b/ee/spec/services/ee/users/ban_service_spec.rb index a78bf84bd21768..ff6b22227fd5d8 100644 --- a/ee/spec/services/ee/users/ban_service_spec.rb +++ b/ee/spec/services/ee/users/ban_service_spec.rb @@ -12,49 +12,28 @@ 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 'for audit events', :enable_admin_mode do + include_examples 'audit event logging' do + let(:operation) { service.execute(user) } - context 'when not licensed' do - before do - stub_licensed_features( - admin_audit_log: false, - audit_events: false, - extended_audit_events: false - ) + let(:fail_condition!) 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 } + let(:attributes) do + { + author_id: current_user.id, + entity_id: user.id, + entity_type: 'User', + details: { + author_class: 'User', + author_name: current_user.name, + custom_message: 'Banned user', + target_details: user.username, + target_id: user.id, + target_type: 'User' + } + } end end end diff --git a/ee/spec/services/ee/users/unban_service_spec.rb b/ee/spec/services/ee/users/unban_service_spec.rb index 0f0ff2fcad1fca..bd8a802a3cd538 100644 --- a/ee/spec/services/ee/users/unban_service_spec.rb +++ b/ee/spec/services/ee/users/unban_service_spec.rb @@ -12,49 +12,28 @@ 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 unban 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: 'Unbanned user') - ) - end - end - - context 'when user unban operation fails' do - let!(:user) { create(:user) } - - before do - allow(user).to receive(:unban).and_return(false) - end - - it 'does not log any audit event' do - expect { operation }.not_to change { AuditEvent.count } - end - end - end + context 'for audit events', :enable_admin_mode do + include_examples 'audit event logging' do + let(:operation) { service.execute(user) } - context 'when not licensed' do - before do - stub_licensed_features( - admin_audit_log: false, - audit_events: false, - extended_audit_events: false - ) + let(:fail_condition!) do + allow(user).to receive(:unban).and_return(false) end - it 'does not log any audit event' do - expect { operation }.not_to change { AuditEvent.count } + let(:attributes) do + { + author_id: current_user.id, + entity_id: user.id, + entity_type: 'User', + details: { + author_class: 'User', + author_name: current_user.name, + custom_message: 'Unbanned user', + target_details: user.username, + target_id: user.id, + target_type: 'User' + } + } end end end -- GitLab