diff --git a/app/services/audit_event_service.rb b/app/services/audit_event_service.rb index 97debccfb18b080f44008b268514f2de489b30af..26244a8bcc54cfd3c00a2c35cf27dd2199593375 100644 --- a/app/services/audit_event_service.rb +++ b/app/services/audit_event_service.rb @@ -121,12 +121,15 @@ def authentication_event? def log_security_event_to_database return if Gitlab::Database.read_only? - event = AuditEvent.new(base_payload.merge(details: @details)) + event = build_event save_or_track event - event end + def build_event + AuditEvent.new(base_payload.merge(details: @details)) + end + def stream_event_to_external_destinations(_event) # Defined in EE end diff --git a/ee/app/models/ee/audit_event.rb b/ee/app/models/ee/audit_event.rb index b5fca879bd453f2bec39bbbec3a9280333f731c6..0574657c96cb6197172a9283c6298d975ffa114c 100644 --- a/ee/app/models/ee/audit_event.rb +++ b/ee/app/models/ee/audit_event.rb @@ -4,6 +4,7 @@ module EE module AuditEvent extend ActiveSupport::Concern extend ::Gitlab::Utils::Override + include ::Gitlab::Utils::StrongMemoize TRUNCATED_FIELDS = { entity_path: 5_500, @@ -16,8 +17,34 @@ module AuditEvent before_validation :truncate_fields end + attr_writer :entity + attr_accessor :root_group_entity_id + def entity - lazy_entity + strong_memoize(:entity) { lazy_entity } + end + + def root_group_entity + strong_memoize(:root_group_entity) do + next ::Group.find_by(id: root_group_entity_id) if root_group_entity_id.present? + next if entity.nil? + + root_group_entity = + case entity_type + when 'Group' + # Sub group events should be sent to the root ancestor's streaming destinations + entity.root_ancestor + when 'Project' + # Project events should be sent to the root ancestor's streaming destinations + # Projects without a group root ancestor should be ignored. + entity.group&.root_ancestor + else + nil + end + + self.root_group_entity_id = root_group_entity&.id + root_group_entity + end end def entity_path @@ -57,7 +84,7 @@ def lazy_entity def stream_to_external_destinations(use_json: false, event_name: 'audit_operation') return unless can_stream_to_external_destination? - perform_params = use_json ? [event_name, nil, self.to_json] : [event_name, id, nil] + perform_params = use_json ? [event_name, nil, streaming_json] : [event_name, id, nil] AuditEvents::AuditEventStreamingWorker.perform_async(*perform_params) end @@ -70,26 +97,13 @@ def entity_is_group_or_project? def can_stream_to_external_destination? return false if entity.nil? - group = group_entity + group = root_group_entity return false if group.nil? return false unless group.licensed_feature_available?(:external_audit_events) group.external_audit_event_destinations.exists? end - def group_entity - case entity_type - when 'Group' - entity - when 'Project' - # Project events should be sent to the root ancestor's streaming destinations - # Projects without a group root ancestor should be ignored. - entity.group&.root_ancestor - else - nil - end - end - def truncate_fields TRUNCATED_FIELDS.each do |name, limit| original = self[name] || self.details[name] @@ -98,5 +112,9 @@ def truncate_fields self[name] = self.details[name] = String(original).truncate(limit) end end + + def streaming_json + ::Gitlab::Json.generate(self, methods: [:root_group_entity_id]) + end end end diff --git a/ee/app/services/ee/audit_event_service.rb b/ee/app/services/ee/audit_event_service.rb index 9eba2f2537df2f8987ff9ed8f691c18d2433719e..21c18ca3430a0295b9da5f54fc20f050bd137872 100644 --- a/ee/app/services/ee/audit_event_service.rb +++ b/ee/app/services/ee/audit_event_service.rb @@ -239,7 +239,7 @@ def stream_event_to_external_destinations(event) return if event.is_a?(AuthenticationEvent) if event.entity_is_group_or_project? - event.run_after_commit_or_now { event.stream_to_external_destinations } + event.run_after_commit_or_now { event.stream_to_external_destinations(use_json: !!event.entity&.destroyed?) } end end @@ -250,6 +250,13 @@ def base_payload end end + override :build_event + def build_event + event = super + event.entity = @entity + event + end + def for_custom_model(model:, target_details:, target_id:) action = @details[:action] model_class = model.camelize diff --git a/ee/app/workers/audit_events/audit_event_streaming_worker.rb b/ee/app/workers/audit_events/audit_event_streaming_worker.rb index 6de6adae41705c8afccde05d89c322e06b0f516f..8c5670c6fd2101b0dcb9fe96c6d69f510c0cfe35 100644 --- a/ee/app/workers/audit_events/audit_event_streaming_worker.rb +++ b/ee/app/workers/audit_events/audit_event_streaming_worker.rb @@ -20,7 +20,7 @@ def perform(audit_operation, audit_event_id, audit_event_json = nil) audit_event = audit_event(audit_event_id, audit_event_json) return if audit_event.nil? - group = group_entity(audit_event) + group = audit_event.root_group_entity return if group.nil? # Do nothing if the event can't be resolved to a single group. return unless group.licensed_feature_available?(:external_audit_events) @@ -60,21 +60,5 @@ def parse_audit_event_json(audit_event_json) audit_event.id = audit_event.created_at.to_i if audit_event.id.blank? audit_event end - - def group_entity(audit_event) - entity = audit_event.entity - return if entity.nil? - - case audit_event.entity_type - when 'Group' - entity - when 'Project' - # Project events should be sent to the root ancestor's streaming destinations - # Projects without a group root ancestor should be ignored. - entity.group&.root_ancestor - else - nil - end - end end end diff --git a/ee/spec/models/ee/audit_event_spec.rb b/ee/spec/models/ee/audit_event_spec.rb index b7ad79632757d827b5eb704c1daa4a8003b6aebd..9284864e6ad3813657ce186c97a9673e3d37cb65 100644 --- a/ee/spec/models/ee/audit_event_spec.rb +++ b/ee/spec/models/ee/audit_event_spec.rb @@ -252,6 +252,47 @@ end end + describe '#root_group_entity' do + let_it_be(:root_group) { create(:group) } + let_it_be(:group) { create(:group, parent: root_group) } + let_it_be(:project) { create(:project, group: group) } + + context 'when root_group_entity_id is set' do + subject(:event) { described_class.new(root_group_entity_id: root_group.id) } + + it "return root_group_entity through root_group_entity_id" do + expect(event.root_group_entity).to eq(root_group) + end + end + + context "when entity is nil" do + subject(:event) { described_class.new(entity: nil) } + + it "return nil" do + expect(event.root_group_entity).to eq(nil) + end + end + + context "when entity is a group" do + subject(:event) { described_class.new(entity_id: group.id, entity_type: "Group") } + + it "return root_group and set root_group_entity_id" do + expect(event.entity).to eq(group) + expect(event.root_group_entity).to eq(root_group) + expect(event.root_group_entity_id).to eq(root_group.id) + end + end + + context "when entity is a project" do + subject(:event) { described_class.new(entity_id: project.id, entity_type: "Project") } + + it "return root_group and set root_group_entity_id" do + expect(event.root_group_entity).to eq(root_group) + expect(event.root_group_entity_id).to eq(root_group.id) + end + end + end + describe '#ip_address' do context 'when ip_address exists in both details hash and ip_address column' do subject(:event) do diff --git a/ee/spec/services/ee/groups/deploy_tokens/create_service_spec.rb b/ee/spec/services/ee/groups/deploy_tokens/create_service_spec.rb index 14bd470509989543b8cc18562962251ea4178b07..690b24bfb2f78e14aeadf7c480995e4d881dddd6 100644 --- a/ee/spec/services/ee/groups/deploy_tokens/create_service_spec.rb +++ b/ee/spec/services/ee/groups/deploy_tokens/create_service_spec.rb @@ -22,6 +22,19 @@ expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) end + + context 'when group is a sub-group' do + let_it_be(:parent_group) { create :group } + let_it_be(:group) { create :group, parent: parent_group } + + subject { described_class.new(group, user, deploy_token_params).execute } + + before do + group.add_owner(user) + end + + include_examples 'sends streaming audit event' + end end context 'when the deploy token is invalid' do diff --git a/ee/spec/services/ee/groups/deploy_tokens/destroy_service_spec.rb b/ee/spec/services/ee/groups/deploy_tokens/destroy_service_spec.rb index 7badf6fd2bf4c03d507ed01c2e5f553d6b15cd33..09d04fe3ff44b55f18341e536f3794c1a0e530d6 100644 --- a/ee/spec/services/ee/groups/deploy_tokens/destroy_service_spec.rb +++ b/ee/spec/services/ee/groups/deploy_tokens/destroy_service_spec.rb @@ -21,5 +21,20 @@ expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) end + + context 'when group is a sub-group' do + let_it_be(:parent_group) { create :group } + let_it_be(:group) { create :group, parent: parent_group } + let_it_be(:deploy_token) { create(:deploy_token, :group, groups: [group]) } + let_it_be(:deploy_token_params) { { token_id: deploy_token.id } } + + subject { described_class.new(group, user, deploy_token_params).execute } + + before do + group.add_owner(user) + end + + include_examples 'sends streaming audit event' + end end end diff --git a/ee/spec/services/ee/groups/deploy_tokens/revoke_service_spec.rb b/ee/spec/services/ee/groups/deploy_tokens/revoke_service_spec.rb index 60cd7d7b81ff7675c9411017c524575354b1dc52..87981df0e457da6fd32896d13ffb30d57f4201bc 100644 --- a/ee/spec/services/ee/groups/deploy_tokens/revoke_service_spec.rb +++ b/ee/spec/services/ee/groups/deploy_tokens/revoke_service_spec.rb @@ -21,5 +21,20 @@ expect(AuditEvent.last.details[:custom_message]).to eq(expected_message) end + + context 'when group is a sub-group' do + let_it_be(:parent_group) { create :group } + let_it_be(:group) { create :group, parent: parent_group } + let_it_be(:deploy_token) { create(:deploy_token, :group, groups: [group]) } + let_it_be(:deploy_token_params) { { id: deploy_token.id } } + + subject { described_class.new(group, user, deploy_token_params).execute } + + before do + group.add_owner(user) + end + + include_examples 'sends streaming audit event' + end end end diff --git a/ee/spec/services/ee/members/create_service_spec.rb b/ee/spec/services/ee/members/create_service_spec.rb index 3af02e558ce7a9c5d7bca62b37c69ad2cc25e165..89ef84da98eb4a71c352c07b0cce45cf89217b58 100644 --- a/ee/spec/services/ee/members/create_service_spec.rb +++ b/ee/spec/services/ee/members/create_service_spec.rb @@ -159,4 +159,17 @@ end end end + + context 'streaming audit event' do + let(:group) { root_ancestor } + let(:params) do + { + user_id: project_users.first.id, + access_level: Gitlab::Access::GUEST, + invite_source: '_invite_source_' + } + end + + include_examples 'sends streaming audit event' + end end diff --git a/ee/spec/services/ee/members/destroy_service_spec.rb b/ee/spec/services/ee/members/destroy_service_spec.rb index 60923ce7c03c38a29e17fd9fd04bd01c36c9ec34..2094bcedd1f3c101ad9676f919d9c30bab3a7ad1 100644 --- a/ee/spec/services/ee/members/destroy_service_spec.rb +++ b/ee/spec/services/ee/members/destroy_service_spec.rb @@ -58,6 +58,12 @@ end end + context 'streaming audit event' do + subject { described_class.new(current_user).execute(member, skip_authorization: true) } + + include_examples 'sends streaming audit event' + end + context 'group deletion schedule' do context 'when member user has a scheduled deletion for the group' do let!(:group_deletion_schedule) { create(:group_deletion_schedule, group: group, user_id: member_user.id, marked_for_deletion_on: 2.days.ago) } diff --git a/ee/spec/services/groups/create_service_spec.rb b/ee/spec/services/groups/create_service_spec.rb index 47a012c373a98b6290a583ca6a561c2e67f9582c..989229486647c33d319926da3b764c8f03fbda49 100644 --- a/ee/spec/services/groups/create_service_spec.rb +++ b/ee/spec/services/groups/create_service_spec.rb @@ -36,6 +36,18 @@ end end + context 'when created group is a sub-group' do + let(:group) { create :group } + + subject(:execute) { create_group(user, group_params.merge(parent_id: group.id)) } + + before do + group.add_owner(user) + end + + include_examples 'sends streaming audit event' + end + context 'repository_size_limit assignment as Bytes' do let(:admin_user) { create(:user, admin: true) } diff --git a/ee/spec/services/groups/destroy_service_spec.rb b/ee/spec/services/groups/destroy_service_spec.rb index b3b214094bc8dfeac11cfaa0c9c4c3f1457f6bcf..384777102e6cbbaa151fe5b7ead8f0040ec8a428 100644 --- a/ee/spec/services/groups/destroy_service_spec.rb +++ b/ee/spec/services/groups/destroy_service_spec.rb @@ -33,6 +33,28 @@ end end + context 'streaming audit event for sub group' do + let(:parent_group) { create :group } + let(:group) { create :group, parent: parent_group } + + subject { described_class.new(group, user, {}).execute } + + before do + parent_group.add_owner(user) + stub_licensed_features(external_audit_events: true) + parent_group.external_audit_event_destinations.create!(destination_url: 'http://example.com') + end + + it 'sends the audit streaming event with json format' do + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( + 'audit_operation', + nil, + a_string_including("group_entity_id\":#{parent_group.id}")) + + subject + end + end + context 'dependency_proxy_blobs' do let_it_be(:blob) { create(:dependency_proxy_blob) } let_it_be(:group) { blob.group } diff --git a/ee/spec/services/groups/update_service_spec.rb b/ee/spec/services/groups/update_service_spec.rb index ba7d2c97480beb6605086296ab2fb1ef2ce7ecec..4ddf094b11b6f602e70613ac3c7f1c343e10dc5a 100644 --- a/ee/spec/services/groups/update_service_spec.rb +++ b/ee/spec/services/groups/update_service_spec.rb @@ -75,6 +75,19 @@ end end + context 'sub group' do + let(:parent_group) { create :group } + let(:group) { create :group, parent: parent_group } + + subject { update_group(group, user, { name: 'new_sub_group_name' }) } + + before do + parent_group.add_owner(user) + end + + include_examples 'sends streaming audit event' + end + describe 'changing file_template_project_id' do let(:group) { create(:group) } let(:valid_project) { create(:project, namespace: group) } diff --git a/ee/spec/services/projects/destroy_service_spec.rb b/ee/spec/services/projects/destroy_service_spec.rb index 5aac45a24b44469e21323fb4c0c4ed016a8b07de..c0ed7626b96d29e0e6081cf32068e28373b21e8e 100644 --- a/ee/spec/services/projects/destroy_service_spec.rb +++ b/ee/spec/services/projects/destroy_service_spec.rb @@ -86,6 +86,28 @@ end end + context 'streaming audit event' do + let(:group) { create :group } + let(:project) { create :project, namespace: group } + + subject { described_class.new(project, user, {}).execute } + + before do + group.add_owner(user) + stub_licensed_features(external_audit_events: true) + group.external_audit_event_destinations.create!(destination_url: 'http://example.com') + end + + it 'sends the audit streaming event with json format' do + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).with( + 'audit_operation', + nil, + a_string_including("root_group_entity_id\":#{group.id}")) + + subject + end + end + context 'system hooks exception' do before do allow_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).and_raise('something went wrong') diff --git a/ee/spec/support/shared_examples/audit/sends_streaming_audit_event_shared_examples.rb b/ee/spec/support/shared_examples/audit/sends_streaming_audit_event_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..2b5adf580a2ee032140738f7a12e57021b32cb6c --- /dev/null +++ b/ee/spec/support/shared_examples/audit/sends_streaming_audit_event_shared_examples.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'sends streaming audit event' do + before do + stub_licensed_features(external_audit_events: true) + group.root_ancestor.external_audit_event_destinations.create!(destination_url: 'http://example.com') + end + + it 'sends the audit streaming event' do + expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).once + + subject + end +end diff --git a/ee/spec/workers/audit_events/audit_event_streaming_worker_spec.rb b/ee/spec/workers/audit_events/audit_event_streaming_worker_spec.rb index 8f262b633d5ee0658aee8beb4423c9fe9263626a..6e1831a0dc1d7172dce929afc3720266c61dc639 100644 --- a/ee/spec/workers/audit_events/audit_event_streaming_worker_spec.rb +++ b/ee/spec/workers/audit_events/audit_event_streaming_worker_spec.rb @@ -224,6 +224,20 @@ end it_behaves_like 'no HTTP calls are made' + + context 'when root_group_entity_id is passed in audit event json' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + let(:event) { create(:audit_event, :project_event, target_project: project) } + + before do + event.root_group_entity_id = group.id + end + + subject { worker.perform('audit_operation', nil, event.to_json(methods: [:root_group_entity_id])) } + + include_context 'audit event stream' + end end end end