Commit ab04d08e authored by Max Woolf's avatar Max Woolf

Move event streaming in to after commit queue

EE: true
parent ed624def
# frozen_string_literal: true
class AuditEvent < ApplicationRecord
include AfterCommitQueue
include CreatedAtFilterable
include BulkInsertSafe
include EachBatch
......
......@@ -141,7 +141,7 @@ class AuditEventService
event.save! if should_save_database?(@save_type)
stream_event_to_external_destinations(event) if should_save_stream?(@save_type)
rescue StandardError => e
Gitlab::ErrorTracking.track_exception(e, audit_event_type: event.class.to_s)
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, audit_event_type: event.class.to_s)
end
end
......
......@@ -238,7 +238,9 @@ module EE
def stream_event_to_external_destinations(event)
return if event.is_a?(AuthenticationEvent)
event.stream_to_external_destinations if event.entity_is_group_or_project?
if event.entity_is_group_or_project?
event.run_after_commit_or_now { event.stream_to_external_destinations }
end
end
override :base_payload
......
......@@ -157,8 +157,7 @@ RSpec.describe AuditEventService, :request_store do
allow(user).to receive_messages(current_sign_in_ip: 'invalid IP')
expect(Gitlab::ErrorTracking).to(
receive(:track_exception)
.with(ActiveRecord::RecordInvalid, audit_event_type: 'AuditEvent').and_call_original
receive(:track_and_raise_for_dev_exception)
)
service.security_event
......@@ -365,8 +364,7 @@ RSpec.describe AuditEventService, :request_store do
end
describe '#for_user' do
let(:author_name) { 'Administrator' }
let(:current_user) { User.new(name: author_name) }
let(:current_user) { create(:user) }
let(:user) { create(:user) }
let(:custom_message) { 'Some strange event has occurred' }
let(:options) { { action: action, custom_message: custom_message } }
......@@ -384,7 +382,7 @@ RSpec.describe AuditEventService, :request_store do
it 'sets the details attribute' do
expect(service.instance_variable_get(:@details)).to eq(
remove: 'user',
author_name: author_name,
author_name: current_user.name,
target_id: user.id,
target_type: 'User',
target_details: user.username
......@@ -402,7 +400,7 @@ RSpec.describe AuditEventService, :request_store do
it 'sets the details attribute' do
expect(service.instance_variable_get(:@details)).to eq(
add: 'user',
author_name: author_name,
author_name: current_user.name,
target_id: user.id,
target_type: 'User',
target_details: user.full_path
......@@ -420,7 +418,7 @@ RSpec.describe AuditEventService, :request_store do
it 'sets the details attribute' do
expect(service.instance_variable_get(:@details)).to eq(
custom_message: custom_message,
author_name: author_name,
author_name: current_user.name,
target_id: user.id,
target_type: 'User',
target_details: user.full_path
......@@ -653,6 +651,7 @@ RSpec.describe AuditEventService, :request_store do
before do
stub_licensed_features(external_audit_events: true)
stub_feature_flags(ff_external_audit_events_namespace: group.root_ancestor)
end
subject(:event) { described_class.new(user, project, details, save_type).for_project.security_event }
......@@ -660,10 +659,27 @@ RSpec.describe AuditEventService, :request_store do
context 'with save_type of :database_and_stream' do
let(:save_type) { :database_and_stream }
it 'save to database and stream' do
expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).once
it 'saves to database' do
expect { event }.to change(AuditEvent, :count).by(1)
end
it 'streams the event' do
expect_next_instance_of(described_class) do |service|
expect(service).to receive(:stream_event_to_external_destinations).once
end
event
end
context 'when the event is created within a transaction' do
it 'does not raise an error about a job being enqueued from within a transaction' do
RSpec::Expectations.configuration.on_potential_false_positives = :nothing
ApplicationRecord.transaction do
expect { event }.not_to raise_error(Sidekiq::Worker::EnqueueFromTransactionError)
end
end
end
end
context 'with save_type of :database' do
......@@ -678,9 +694,16 @@ RSpec.describe AuditEventService, :request_store do
context 'with save_type of :stream' do
let(:save_type) { :stream }
it 'saves to database and stream' do
expect(AuditEvents::AuditEventStreamingWorker).to receive(:perform_async).once
expect { event }.to change(AuditEvent, :count).by(0)
it 'does not save to database' do
expect { event }.not_to change(AuditEvent, :count)
end
it 'streams the event' do
expect_next_instance_of(described_class) do |service|
expect(service).to receive(:stream_event_to_external_destinations).once
end
event
end
end
end
......
......@@ -60,17 +60,18 @@ RSpec.describe AuditEventService do
ip_address: user.current_sign_in_ip,
result: AuthenticationEvent.results[:success],
provider: 'standard'
)
).and_call_original
audit_service.for_authentication.security_event
end
it 'tracks exceptions when the event cannot be created' do
allow(user).to receive_messages(current_sign_in_ip: 'invalid IP')
allow_next_instance_of(AuditEvent) do |event|
allow(event).to receive(:valid?).and_return(false)
end
expect(Gitlab::ErrorTracking).to(
receive(:track_exception)
.with(ActiveRecord::RecordInvalid, audit_event_type: 'AuthenticationEvent').and_call_original
receive(:track_and_raise_for_dev_exception)
)
audit_service.for_authentication.security_event
......@@ -93,7 +94,7 @@ RSpec.describe AuditEventService do
end
specify do
expect(AuthenticationEvent).to receive(:new).with(hash_including(ip_address: output))
expect(AuthenticationEvent).to receive(:new).with(hash_including(ip_address: output)).and_call_original
audit_service.for_authentication.security_event
end
......
......@@ -7,13 +7,13 @@ RSpec.shared_examples 'user login operation with unique ip limit' do
end
it 'allows user authenticating from the same ip' do
expect { operation_from_ip('ip') }.not_to raise_error
expect { operation_from_ip('ip') }.not_to raise_error
expect { operation_from_ip('111.221.4.3') }.not_to raise_error
expect { operation_from_ip('111.221.4.3') }.not_to raise_error
end
it 'blocks user authenticating from two distinct ips' do
expect { operation_from_ip('ip') }.not_to raise_error
expect { operation_from_ip('ip2') }.to raise_error(Gitlab::Auth::TooManyIps)
expect { operation_from_ip('111.221.4.3') }.not_to raise_error
expect { operation_from_ip('1.2.2.3') }.to raise_error(Gitlab::Auth::TooManyIps)
end
end
end
......@@ -25,13 +25,13 @@ RSpec.shared_examples 'user login request with unique ip limit' do |success_stat
end
it 'allows user authenticating from the same ip' do
expect(request_from_ip('ip')).to have_gitlab_http_status(success_status)
expect(request_from_ip('ip')).to have_gitlab_http_status(success_status)
expect(request_from_ip('111.221.4.3')).to have_gitlab_http_status(success_status)
expect(request_from_ip('111.221.4.3')).to have_gitlab_http_status(success_status)
end
it 'blocks user authenticating from two distinct ips' do
expect(request_from_ip('ip')).to have_gitlab_http_status(success_status)
expect(request_from_ip('ip2')).to have_gitlab_http_status(:forbidden)
expect(request_from_ip('111.221.4.3')).to have_gitlab_http_status(success_status)
expect(request_from_ip('1.2.2.3')).to have_gitlab_http_status(:forbidden)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment