Merge branch 'dblessing-auth-events-logging' into 'master'

Log authentication events alongside audit events

See merge request gitlab-org/gitlab!42033

app/services/audit_event_service.rb

@@ -25,6 +25,8 @@ def initialize(author, entity, details = {})
   #
   # @return [AuditEventService]
   def for_authentication
+    mark_as_authentication_event!
+
     @details = {
       with: @details[:with],
       target_id: @author.id,
@@ -40,6 +42,7 @@ def for_authentication
   # @return [AuditEvent] persited if saves and non-persisted if fails
   def security_event
     log_security_event_to_file
+    log_authentication_event_to_database
     log_security_event_to_database
   end
 
@@ -50,6 +53,7 @@ def log_security_event_to_file
 
   private
 
+  attr_accessor :authentication_event
   attr_reader :ip_address
 
   def build_author(author)
@@ -70,6 +74,22 @@ def base_payload
     }
   end
 
+  def authentication_event_payload
+    {
+      # @author can be a User or various Gitlab::Audit authors.
+      # Only capture real users for successful authentication events.
+      user: author_if_user,
+      user_name: @author.name,
+      ip_address: ip_address,
+      result: AuthenticationEvent.results[:success],
+      provider: @details[:with]
+    }
+  end
+
+  def author_if_user
+    @author if @author.is_a?(User)
+  end
+
   def file_logger
     @file_logger ||= Gitlab::AuditJsonLogger.build
   end
@@ -78,11 +98,25 @@ def formatted_details
     @details.merge(@details.slice(:from, :to).transform_values(&:to_s))
   end
 
+  def mark_as_authentication_event!
+    self.authentication_event = true
+  end
+
+  def authentication_event?
+    authentication_event
+  end
+
   def log_security_event_to_database
     return if Gitlab::Database.read_only?
 
     AuditEvent.create(base_payload.merge(details: @details))
   end
+
+  def log_authentication_event_to_database
+    return unless Gitlab::Database.read_write? && authentication_event?
+
+    AuthenticationEvent.create(authentication_event_payload)
+  end
 end
 
 AuditEventService.prepend_if_ee('EE::AuditEventService')

changelogs/unreleased/dblessing-auth-events-logging.yml

+---
+title: Log authentication events alongside existing audit events
+merge_request: 42033
+author:
+type: added

spec/controllers/ldap/omniauth_callbacks_controller_spec.rb

@@ -11,6 +11,11 @@
     expect(request.env['warden']).to be_authenticated
   end
 
+  it 'creates an authentication event record' do
+    expect { post provider }.to change { AuthenticationEvent.count }.by(1)
+    expect(AuthenticationEvent.last.provider).to eq(provider.to_s)
+  end
+
   context 'with sign in prevented' do
     let(:ldap_settings) { ldap_setting_defaults.merge(prevent_ldap_sign_in: true) }
 

spec/controllers/omniauth_callbacks_controller_spec.rb

@@ -170,6 +170,11 @@ def stub_route_as(path)
           expect(request.env['warden']).to be_authenticated
         end
 
+        it 'creates an authentication event record' do
+          expect { post provider }.to change { AuthenticationEvent.count }.by(1)
+          expect(AuthenticationEvent.last.provider).to eq(provider.to_s)
+        end
+
         context 'when user has no linked provider' do
           let(:user) { create(:user) }
 

spec/controllers/sessions_controller_spec.rb

@@ -140,6 +140,11 @@
           expect(AuditEvent.last.details[:with]).to eq('standard')
         end
 
+        it 'creates an authentication event record' do
+          expect { post(:create, params: { user: user_params }) }.to change { AuthenticationEvent.count }.by(1)
+          expect(AuthenticationEvent.last.provider).to eq('standard')
+        end
+
         include_examples 'user login request with unique ip limit', 302 do
           def request
             post(:create, params: { user: user_params })
@@ -407,6 +412,11 @@ def authenticate_2fa(user_params, otp_user_id: user.id)
         expect { authenticate_2fa(login: user.username, otp_attempt: user.current_otp) }.to change { AuditEvent.count }.by(1)
         expect(AuditEvent.last.details[:with]).to eq("two-factor")
       end
+
+      it "creates an authentication event record" do
+        expect { authenticate_2fa(login: user.username, otp_attempt: user.current_otp) }.to change { AuthenticationEvent.count }.by(1)
+        expect(AuthenticationEvent.last.provider).to eq("two-factor")
+      end
     end
 
     context 'when using two-factor authentication via U2F device' do
@@ -448,6 +458,13 @@ def authenticate_2fa_u2f(user_params)
         expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { AuditEvent.count }.by(1)
         expect(AuditEvent.last.details[:with]).to eq("two-factor-via-u2f-device")
       end
+
+      it "creates an authentication event record" do
+        allow(U2fRegistration).to receive(:authenticate).and_return(true)
+
+        expect { authenticate_2fa_u2f(login: user.username, device_response: "{}") }.to change { AuthenticationEvent.count }.by(1)
+        expect(AuthenticationEvent.last.provider).to eq("two-factor-via-u2f-device")
+      end
     end
   end
 

spec/services/audit_event_service_spec.rb

@@ -52,6 +52,22 @@
       expect(details[:action]).to eq(:create)
       expect(details[:target_id]).to eq(1)
     end
+
+    context 'authentication event' do
+      let(:audit_service) { described_class.new(user, user, with: 'standard') }
+
+      it 'creates an authentication event' do
+        expect(AuthenticationEvent).to receive(:create).with(
+          user: user,
+          user_name: user.name,
+          ip_address: user.current_sign_in_ip,
+          result: AuthenticationEvent.results[:success],
+          provider: 'standard'
+        )
+
+        audit_service.for_authentication.security_event
+      end
+    end
   end
 
   describe '#log_security_event_to_file' do