Commit 8d087540 authored by Tan Le's avatar Tan Le

Gracefully truncate entity_path in audit_events

Invalid `entity_path` should not cause validation errors as it obstructs
main user actions. Meanwhile, we'd like to ensure the data integrity on
`audit_events` table by truncating the value to the agreed length.
parent 966a6aa7
......@@ -5,14 +5,15 @@ module EE
extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
TEXT_LIMIT = {
TRUNCATED_FIELDS = {
entity_path: 5_500,
target_details: 5_500
}.freeze
prepended do
scope :by_entity, -> (entity_type, entity_id) { by_entity_type(entity_type).by_entity_id(entity_id) }
before_validation :truncate_target_details
before_validation :truncate_fields
end
def entity
......@@ -43,8 +44,13 @@ module EE
private
def truncate_target_details
self.target_details = self.details[:target_details] = target_details&.truncate(TEXT_LIMIT[:target_details])
def truncate_fields
TRUNCATED_FIELDS.each do |name, limit|
original = self[name] || self.details[name]
next unless original
self[name] = self.details[name] = String(original).truncate(limit)
end
end
end
end
......@@ -18,7 +18,7 @@ RSpec.describe AuditEvent, type: :model do
let_it_be(:details) do
{ author_name: 'Kungfu Panda', entity_path: 'gitlab-org/gitlab', target_details: 'Project X' }
end
let_it_be(:event) { create(:project_audit_event, details: details, target_details: nil) }
let_it_be(:event) { create(:project_audit_event, details: details, entity_path: nil, target_details: nil) }
it 'sets author_name' do
expect(event[:author_name]).to eq('Kungfu Panda')
......@@ -33,30 +33,66 @@ RSpec.describe AuditEvent, type: :model do
end
end
describe '#truncate_target_details' do
where(:database_column, :details_value, :expected_value) do
text_limit = described_class::TEXT_LIMIT[:target_details]
long_value = 'a' * (text_limit + 1)
truncated_long_value = long_value.truncate(text_limit)
short_value = 'a' * text_limit
[
[nil, nil, nil],
[long_value, nil, truncated_long_value],
[short_value, nil, short_value],
[nil, long_value, truncated_long_value],
[nil, short_value, short_value],
[long_value, 'something', truncated_long_value]
]
end
with_them do
let(:audit_event) { create(:audit_event, target_details: database_column, details: { target_details: details_value }) }
it 'expects both values to be the same and correct' do
expect(audit_event.target_details).to eq(expected_value)
expect(audit_event.details[:target_details]).to eq(expected_value)
context 'truncate_fields' do
shared_examples 'a truncated field' do
context 'when values are provided' do
using RSpec::Parameterized::TableSyntax
where(:database_column, :details_value, :expected_value) do
:long | nil | :truncated
:short | nil | :short
nil | :long | :truncated
nil | :short | :short
:long | :short | :truncated
end
with_them do
let(:values) do
{
long: 'a' * (field_limit + 1),
short: 'a' * field_limit,
truncated: 'a' * (field_limit - 3) + '...'
}
end
let(:audit_event) do
create(:audit_event,
field_name => values[database_column],
details: { field_name => values[details_value] }
)
end
it 'sets both values to be the same', :aggregate_failures do
expect(audit_event.send(field_name)).to eq(values[expected_value])
expect(audit_event.details[field_name]).to eq(values[expected_value])
end
end
end
context 'when values are not provided' do
let(:audit_event) do
create(:audit_event, field_name => nil, details: {})
end
it 'does not set', :aggregate_failures do
expect(audit_event.send(field_name)).to be_nil
expect(audit_event.details).not_to have_key(field_name)
end
end
end
context 'entity_path' do
let(:field_name) { :entity_path }
let(:field_limit) { 5_500 }
it_behaves_like 'a truncated field'
end
context 'target_details' do
let(:field_name) { :target_details }
let(:field_limit) { 5_500 }
it_behaves_like 'a truncated field'
end
end
end
......
......@@ -10,7 +10,6 @@ RSpec.describe EE::AuditEvents::CustomAuditEventService do
let(:entity) { create(:project) }
let(:entity_type) { 'Project' }
let(:custom_message) { 'Custom Event' }
let(:target_details) { nil }
let(:service) { described_class.new(user, entity, ip_address, custom_message) }
end
end
......
......@@ -7,7 +7,6 @@ RSpec.describe EE::AuditEvents::ImpersonationAuditEventService do
let(:ip_address) { '127.0.0.1' }
let(:message) { 'Impersonation Started' }
let(:logger) { instance_double(Gitlab::AuditJsonLogger) }
let(:target_details) { nil }
let(:service) { described_class.new(impersonator, ip_address, message) }
describe '#security_event' do
......@@ -30,8 +29,7 @@ RSpec.describe EE::AuditEvents::ImpersonationAuditEventService do
expect(security_event.details).to eq(custom_message: message,
ip_address: ip_address,
action: :custom,
target_details: target_details)
action: :custom)
expect(security_event.author_id).to eq(impersonator.id)
expect(security_event.entity_id).to eq(impersonator.id)
expect(security_event.entity_type).to eq('User')
......
......@@ -10,7 +10,6 @@ RSpec.describe EE::AuditEvents::RepositoryDownloadStartedAuditEventService do
let(:entity) { create(:project) }
let(:entity_type) { 'Project' }
let(:custom_message) { 'Repository Download Started' }
let(:target_details) { nil }
let(:service) { described_class.new(user, entity, ip_address) }
end
end
......
......@@ -62,7 +62,6 @@ RSpec.shared_examples 'logs the custom audit event' do
expect(security_event.details).to eq(custom_message: custom_message,
ip_address: ip_address,
target_details: target_details,
action: :custom)
expect(security_event.author_id).to eq(user.id)
expect(security_event.entity_id).to eq(entity.id)
......
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