Commit 816032b2 authored by Tan Le's avatar Tan Le

Resolve IP address from RequestContext for audit

Callers of AuditEventService does not always have access to IP
address (e.g. Service classes). In addition, acquiring this piece of
information should not be the responsibility of the callers.

IP address can be retrieved from RequestContext and injected directly on
AuditEventService. This approach helps keeping the interface to
AuditEventService small and be more user-friendly.
parent 894d255e
...@@ -16,7 +16,7 @@ class AuditEventService ...@@ -16,7 +16,7 @@ class AuditEventService
@author = build_author(author) @author = build_author(author)
@entity = entity @entity = entity
@details = details @details = details
@ip_address = (@details[:ip_address].presence || @author.current_sign_in_ip) @ip_address = resolve_ip_address(@details, @author)
end end
# Builds the @details attribute for authentication # Builds the @details attribute for authentication
...@@ -64,6 +64,12 @@ class AuditEventService ...@@ -64,6 +64,12 @@ class AuditEventService
end end
end end
def resolve_ip_address(details, author)
details[:ip_address].presence ||
Gitlab::RequestContext.instance.client_ip ||
author.current_sign_in_ip
end
def base_payload def base_payload
{ {
author_id: @author.id, author_id: @author.id,
......
---
title: Resolve request IP address on audit event
merge_request: 46114
author:
type: changed
...@@ -3,17 +3,13 @@ ...@@ -3,17 +3,13 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe AuditEventService do RSpec.describe AuditEventService do
let(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:user) { create(:user, :with_sign_ins) } let_it_be(:user) { create(:user, :with_sign_ins) }
let(:project_member) { create(:project_member, user: user) } let_it_be(:project_member) { create(:project_member, user: user) }
let(:service) { described_class.new(user, project, { action: :destroy }) } let(:service) { described_class.new(user, project, { action: :destroy }) }
let(:logger) { instance_double(Gitlab::AuditJsonLogger) } let(:logger) { instance_double(Gitlab::AuditJsonLogger) }
describe '#security_event' do describe '#security_event' do
before do
stub_licensed_features(admin_audit_log: false)
end
it 'creates an event and logs to a file' do it 'creates an event and logs to a file' do
expect(service).to receive(:file_logger).and_return(logger) expect(service).to receive(:file_logger).and_return(logger)
expect(logger).to receive(:info).with(author_id: user.id, expect(logger).to receive(:info).with(author_id: user.id,
...@@ -78,6 +74,31 @@ RSpec.describe AuditEventService do ...@@ -78,6 +74,31 @@ RSpec.describe AuditEventService do
audit_service.for_authentication.security_event audit_service.for_authentication.security_event
end end
context 'with IP address', :request_store do
using RSpec::Parameterized::TableSyntax
where(:from_caller, :from_context, :from_author_sign_in, :output) do
'192.168.0.1' | '192.168.0.2' | '192.168.0.3' | '192.168.0.1'
nil | '192.168.0.2' | '192.168.0.3' | '192.168.0.2'
nil | nil | '192.168.0.3' | '192.168.0.3'
end
with_them do
let(:user) { create(:user, current_sign_in_ip: from_author_sign_in) }
let(:audit_service) { described_class.new(user, user, with: 'standard', ip_address: from_caller) }
before do
allow(Gitlab::RequestContext.instance).to receive(:client_ip).and_return(from_context)
end
specify do
expect(AuthenticationEvent).to receive(:new).with(hash_including(ip_address: output))
audit_service.for_authentication.security_event
end
end
end
end 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