From aa441ef1e568a9764063674bedc7a287117fabf5 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Thu, 6 Apr 2023 23:17:01 +0530 Subject: [PATCH 1/4] Refactoring audit events for groups --- .../audit_events/custom_audit_event_service.rb | 1 + ee/app/services/ee/groups/destroy_service.rb | 18 +++++++++++++----- .../groups/mark_for_deletion_service.rb | 15 +++++++++------ ee/app/services/groups/restore_service.rb | 15 +++++++++------ .../custom_audit_event_service_spec.rb | 1 + .../services/groups/destroy_service_spec.rb | 8 ++++++-- .../groups/mark_for_deletion_service_spec.rb | 4 ++++ .../services/groups/restore_service_spec.rb | 4 ++++ .../audit_event_logging_shared_examples.rb | 12 ++++++++++++ 9 files changed, 59 insertions(+), 19 deletions(-) diff --git a/ee/app/services/audit_events/custom_audit_event_service.rb b/ee/app/services/audit_events/custom_audit_event_service.rb index 22a4659d868d5b..04df10899190d3 100644 --- a/ee/app/services/audit_events/custom_audit_event_service.rb +++ b/ee/app/services/audit_events/custom_audit_event_service.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true module AuditEvents + # TODO: delete this file as it will not be used after project one gets merged class CustomAuditEventService < ::AuditEventService def initialize(author, entity, ip_address, custom_message) super(author, entity, { diff --git a/ee/app/services/ee/groups/destroy_service.rb b/ee/app/services/ee/groups/destroy_service.rb index c72cb0d73e2a37..44723836bd8060 100644 --- a/ee/app/services/ee/groups/destroy_service.rb +++ b/ee/app/services/ee/groups/destroy_service.rb @@ -51,11 +51,19 @@ def delete_dependency_proxy_blobs(group) end def log_audit_event - ::AuditEventService.new( - current_user, - group, - action: :destroy - ).for_group.security_event + audit_context = { + name: 'group_destroyed', + author: current_user, + scope: group.root_ancestor, + target: group, + message: 'Group destroyed', + target_details: group.full_path, + additional_details: { + remove: 'group' + } + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end end diff --git a/ee/app/services/groups/mark_for_deletion_service.rb b/ee/app/services/groups/mark_for_deletion_service.rb index a20ab7e46a132f..e55d86d31f94b0 100644 --- a/ee/app/services/groups/mark_for_deletion_service.rb +++ b/ee/app/services/groups/mark_for_deletion_service.rb @@ -31,12 +31,15 @@ def deletion_schedule_params end def log_audit_event - AuditEvents::CustomAuditEventService.new( - current_user, - group, - nil, - 'Group marked for deletion' - ).for_group.security_event + audit_context = { + name: 'group_deletion_marked', + author: current_user, + scope: group, + target: group, + message: 'Group marked for deletion' + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end end diff --git a/ee/app/services/groups/restore_service.rb b/ee/app/services/groups/restore_service.rb index 098e0d5d636294..19592a6b455e48 100644 --- a/ee/app/services/groups/restore_service.rb +++ b/ee/app/services/groups/restore_service.rb @@ -28,12 +28,15 @@ def remove_deletion_schedule end def log_audit_event - AuditEvents::CustomAuditEventService.new( - current_user, - group, - nil, - 'Group restored' - ).for_group.security_event + audit_context = { + name: 'group_restored', + author: current_user, + scope: group, + target: group, + message: 'Group restored' + } + + ::Gitlab::Audit::Auditor.audit(audit_context) end end end diff --git a/ee/spec/services/audit_events/custom_audit_event_service_spec.rb b/ee/spec/services/audit_events/custom_audit_event_service_spec.rb index da7c7ac31b3638..914ec92ecca8ee 100644 --- a/ee/spec/services/audit_events/custom_audit_event_service_spec.rb +++ b/ee/spec/services/audit_events/custom_audit_event_service_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +# TODO: delete this file as it will not be used after project one gets merged RSpec.describe AuditEvents::CustomAuditEventService do describe '#security_event' do diff --git a/ee/spec/services/groups/destroy_service_spec.rb b/ee/spec/services/groups/destroy_service_spec.rb index b17c308a0ba1f3..410869bd02f3d9 100644 --- a/ee/spec/services/groups/destroy_service_spec.rb +++ b/ee/spec/services/groups/destroy_service_spec.rb @@ -16,6 +16,8 @@ .to receive(:destroy).and_return(group) end + let_it_be(:event_type) { 'group_destroyed' } + let(:attributes) do { author_id: user.id, @@ -24,9 +26,11 @@ details: { remove: 'group', author_name: user.name, + author_class: user.class.name, target_id: group.id, target_type: 'Group', - target_details: group.full_path + target_details: group.full_path, + custom_message: 'Group destroyed' } } end @@ -47,7 +51,7 @@ it 'sends the audit streaming event with json format' do expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( - 'audit_operation', + 'group_destroyed', nil, a_string_including("group_entity_id\":#{parent_group.id}")) diff --git a/ee/spec/services/groups/mark_for_deletion_service_spec.rb b/ee/spec/services/groups/mark_for_deletion_service_spec.rb index 05e9d70fe99650..5ae875ef6c2db9 100644 --- a/ee/spec/services/groups/mark_for_deletion_service_spec.rb +++ b/ee/spec/services/groups/mark_for_deletion_service_spec.rb @@ -68,6 +68,10 @@ context 'audit events' do it 'logs audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + hash_including(name: 'group_deletion_marked') + ).and_call_original + expect { subject }.to change { AuditEvent.count }.by(1) end end diff --git a/ee/spec/services/groups/restore_service_spec.rb b/ee/spec/services/groups/restore_service_spec.rb index 6f5998028edd59..0b5886e186de61 100644 --- a/ee/spec/services/groups/restore_service_spec.rb +++ b/ee/spec/services/groups/restore_service_spec.rb @@ -66,6 +66,10 @@ context 'audit events' do it 'logs audit event' do + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + hash_including(name: 'group_restored') + ).and_call_original + expect { subject }.to change { AuditEvent.count }.by(1) end end diff --git a/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb b/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb index 8a66143d4cc7c9..e9947caaf91fac 100644 --- a/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb +++ b/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb @@ -16,6 +16,18 @@ expect(AuditEvent.last).to have_attributes(attributes) end + + it 'calls the audit method with the event type' do + audit_name = event_type rescue nil # rubocop:disable Style/RescueModifier + + if audit_name.present? + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + hash_including(name: audit_name) + ).and_call_original + + operation + end + end end it 'does not log audit event if operation fails' do -- GitLab From 2fbc5378d509d0379480a568a2a6c4acd66d540b Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Mon, 10 Apr 2023 19:07:46 +0530 Subject: [PATCH 2/4] Deleted the todos --- ee/app/services/audit_events/custom_audit_event_service.rb | 1 - ee/spec/services/audit_events/custom_audit_event_service_spec.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/ee/app/services/audit_events/custom_audit_event_service.rb b/ee/app/services/audit_events/custom_audit_event_service.rb index 04df10899190d3..22a4659d868d5b 100644 --- a/ee/app/services/audit_events/custom_audit_event_service.rb +++ b/ee/app/services/audit_events/custom_audit_event_service.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true module AuditEvents - # TODO: delete this file as it will not be used after project one gets merged class CustomAuditEventService < ::AuditEventService def initialize(author, entity, ip_address, custom_message) super(author, entity, { diff --git a/ee/spec/services/audit_events/custom_audit_event_service_spec.rb b/ee/spec/services/audit_events/custom_audit_event_service_spec.rb index 914ec92ecca8ee..da7c7ac31b3638 100644 --- a/ee/spec/services/audit_events/custom_audit_event_service_spec.rb +++ b/ee/spec/services/audit_events/custom_audit_event_service_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'spec_helper' -# TODO: delete this file as it will not be used after project one gets merged RSpec.describe AuditEvents::CustomAuditEventService do describe '#security_event' do -- GitLab From 11ed1e20cc99778d60eb5682d6f21c0e389b77e8 Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Mon, 10 Apr 2023 19:11:56 +0530 Subject: [PATCH 3/4] Added yml files for new group audit events --- ee/config/audit_events/types/group_deletion_marked.yml | 9 +++++++++ ee/config/audit_events/types/group_destroyed.yml | 9 +++++++++ ee/config/audit_events/types/group_restored.yml | 9 +++++++++ 3 files changed, 27 insertions(+) create mode 100644 ee/config/audit_events/types/group_deletion_marked.yml create mode 100644 ee/config/audit_events/types/group_destroyed.yml create mode 100644 ee/config/audit_events/types/group_restored.yml diff --git a/ee/config/audit_events/types/group_deletion_marked.yml b/ee/config/audit_events/types/group_deletion_marked.yml new file mode 100644 index 00000000000000..7a10210549f049 --- /dev/null +++ b/ee/config/audit_events/types/group_deletion_marked.yml @@ -0,0 +1,9 @@ +--- +name: group_deletion_marked +description: Event triggered when a group is marked for deletion. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374106 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116986 +feature_category: compliance_management +milestone: '15.11' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/group_destroyed.yml b/ee/config/audit_events/types/group_destroyed.yml new file mode 100644 index 00000000000000..0300fc6b890d22 --- /dev/null +++ b/ee/config/audit_events/types/group_destroyed.yml @@ -0,0 +1,9 @@ +--- +name: group_destroyed +description: Event triggered when a group is destroyed. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374106 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116986 +feature_category: compliance_management +milestone: '15.11' +saved_to_database: true +streamed: true diff --git a/ee/config/audit_events/types/group_restored.yml b/ee/config/audit_events/types/group_restored.yml new file mode 100644 index 00000000000000..2cece207ead59b --- /dev/null +++ b/ee/config/audit_events/types/group_restored.yml @@ -0,0 +1,9 @@ +--- +name: group_restored +description: Event triggered when a group is restored. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/374106 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116986 +feature_category: compliance_management +milestone: '15.11' +saved_to_database: true +streamed: true -- GitLab From 3c18d50cb43d45d8d6d625de0fef54e292e6482b Mon Sep 17 00:00:00 2001 From: Hitesh Raghuvanshi Date: Tue, 11 Apr 2023 12:58:52 +0530 Subject: [PATCH 4/4] Replaced rescue with defined check --- .../services/audit_event_logging_shared_examples.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb b/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb index e9947caaf91fac..69e2dda76dd3b9 100644 --- a/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb +++ b/ee/spec/support/shared_examples/services/audit_event_logging_shared_examples.rb @@ -18,11 +18,9 @@ end it 'calls the audit method with the event type' do - audit_name = event_type rescue nil # rubocop:disable Style/RescueModifier - - if audit_name.present? + if defined?(event_type) expect(::Gitlab::Audit::Auditor).to receive(:audit).with( - hash_including(name: audit_name) + hash_including(name: event_type) ).and_call_original operation -- GitLab