Merge branch 'feature/audit-events-failed-login' into 'master'

Audit failed login events

See merge request !2587

app/controllers/omniauth_callbacks_controller.rb

 class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   include AuthenticatesWithTwoFactor
   include Devise::Controllers::Rememberable
+  prepend EE::OmniauthCallbacksController
 
   protect_from_forgery except: [:kerberos, :saml, :cas3]
 
@@ -34,14 +35,13 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
       if @user.two_factor_enabled?
         prompt_for_two_factor(@user)
       else
-        log_audit_event(@user, with: :ldap)
+        log_audit_event(@user, with: oauth['provider'])
         # The counter only gets incremented in `sign_in_and_redirect`
         show_ldap_sync_flash if @user.sign_in_count == 0
         sign_in_and_redirect(@user)
       end
     else
-      flash[:alert] = "Access denied for your LDAP account."
-      redirect_to new_user_session_path
+      fail_ldap_login
     end
   end
 
@@ -136,9 +136,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
         sign_in_and_redirect(@user)
       end
     else
-      error_message = @user.errors.full_messages.to_sentence
-
-      return redirect_to omniauth_error_path(oauth['provider'], error: error_message)
+      fail_login
     end
   end
 
@@ -159,6 +157,18 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     @oauth ||= request.env['omniauth.auth']
   end
 
+  def fail_login
+    error_message = @user.errors.full_messages.to_sentence
+
+    return redirect_to omniauth_error_path(oauth['provider'], error: error_message)
+  end
+
+  def fail_ldap_login
+    flash[:alert] = 'Access denied for your LDAP account.'
+
+    redirect_to new_user_session_path
+  end
+
   def log_audit_event(user, options = {})
     AuditEventService.new(user, user, options)
       .for_authentication.security_event

app/controllers/sessions_controller.rb

@@ -2,6 +2,7 @@ class SessionsController < Devise::SessionsController
   include AuthenticatesWithTwoFactor
   include Devise::Controllers::Rememberable
   include Recaptcha::ClientHelper
+  prepend EE::SessionsController
 
   skip_before_action :check_two_factor_requirement, only: [:destroy]
 

changelogs/unreleased-ee/feature-audit-events-failed-login.yml

+---
+title: Audit failed login events
+merge_request: 2587
+author:

doc/administration/audit_events.md

@@ -25,6 +25,7 @@ in your GitLab instance, and who gave them that permission level.
 |--------------------------------|--------------------------------------------------------------------------------------------------|
 | User added to group or project | Notes the author of the change, target user                                                      |
 | User permission changed        | Notes the author of the change, original permission and new permission, target user              |
+| User login failed              | Notes the target username and IP address                                                         |
 
 ## Audit events in project
 

ee/app/controllers/ee/omniauth_callbacks_controller.rb

+module EE
+  module OmniauthCallbacksController
+    protected
+
+    def fail_login
+      log_failed_login(@user.username, oauth['provider'])
+
+      error_message = @user.errors.full_messages.to_sentence
+
+      return redirect_to omniauth_error_path(oauth['provider'], error: error_message)
+    end
+
+    def fail_ldap_login
+      log_failed_login(@user.username, oauth['provider'])
+
+      flash[:alert] = 'Access denied for your LDAP account.'
+
+      redirect_to new_user_session_path
+    end
+
+    private
+
+    def log_failed_login(author, provider)
+      ::AuditEventService.new(author,
+                            nil,
+                            ip_address: request.remote_ip,
+                            with: provider)
+          .for_failed_login.unauth_security_event
+    end
+  end
+end

ee/app/controllers/ee/sessions_controller.rb

+module EE
+  module SessionsController
+    extend ActiveSupport::Concern
+
+    prepended do
+      after_action :log_failed_login, only: :new, if: :failed_login?
+    end
+
+    private
+
+    def log_failed_login
+      ::AuditEventService.new(request.filtered_parameters['user']['login'], nil, ip_address: request.remote_ip)
+          .for_failed_login.unauth_security_event
+    end
+
+    def failed_login?
+      env['warden.options'] && env['warden.options'][:action] == 'unauthenticated'
+    end
+  end
+end

ee/app/services/ee/audit_event_service.rb

@@ -68,6 +68,20 @@ module EE
       self
     end
 
+    def for_failed_login
+      ip = @details[:ip_address]
+      auth = @details[:with] || 'STANDARD'
+
+      @details = {
+        failed_login: auth.upcase,
+        author_name: @author,
+        target_details: @author,
+        ip_address: ip
+      }
+
+      self
+    end
+
     def security_event
       if admin_audit_log_enabled?
         add_security_event_admin_details!
@@ -92,5 +106,18 @@ module EE
     def admin_audit_log_enabled?
       License.feature_available?(:admin_audit_log)
     end
+
+    def unauth_security_event
+      return unless audit_events_enabled?
+
+      @details.delete(:ip_address) unless admin_audit_log_enabled?
+
+      SecurityEvent.create(
+        author_id: -1,
+        entity_id: -1,
+        entity_type: 'User',
+        details: @details
+      )
+    end
   end
 end

lib/audit/details.rb

 module Audit
   class Details
-    CRUD_ACTIONS = %i[add remove change].freeze
+    ACTIONS = %i[add remove failed_login change].freeze
     
     def self.humanize(*args)
       new(*args).humanize
@@ -14,21 +14,23 @@ module Audit
       if @details[:with]
         "Signed in with #{@details[:with].upcase} authentication"
       else
-        crud_action_text
+        action_text
       end
     end
     
     private
     
-    def crud_action_text
-      action = @details.slice(*CRUD_ACTIONS)
+    def action_text
+      action = @details.slice(*ACTIONS)
       value = @details.values.first.tr('_', ' ')
-      
+
       case action.keys.first
       when :add
-        "Added #{value}#{@details[:as] ? " as #{@details[:as]}" : ""}"
+        "Added #{value}#{@details[:as] ? " as #{@details[:as]}" : ''}"
       when :remove
         "Removed #{value}"
+      when :failed_login
+        "Failed to login with #{Gitlab::OAuth::Provider.label_for(value).upcase} authentication"
       else
         "Changed #{value} from #{@details[:from]} to #{@details[:to]}"
       end

spec/ee/spec/services/ee/audit_event_service_spec.rb

+require 'spec_helper'
+
+describe AuditEventService do
+  describe '#for_failed_login' do
+    let(:author_name) { 'testuser' }
+    let(:ip_address) { '127.0.0.1' }
+    let(:service) { described_class.new(author_name, nil, ip_address: ip_address) }
+    let(:event) { service.for_failed_login.unauth_security_event }
+
+    it 'has the right type' do
+      expect(event.entity_type).to eq('User')
+    end
+
+    it 'has the right author' do
+      expect(event.details[:author_name]).to eq(author_name)
+    end
+
+    it 'has the right IP address' do
+      allow(service).to receive(:admin_audit_log_enabled?).and_return(true)
+
+      expect(event.details[:ip_address]).to eq(ip_address)
+    end
+
+    it 'has the right auth method for OAUTH' do
+      oauth_service = described_class.new(author_name, nil, ip_address: ip_address, with: 'ldap')
+      event = oauth_service.for_failed_login.unauth_security_event
+
+      expect(event.details[:failed_login]).to eq('LDAP')
+    end
+  end
+end

spec/features/login_spec.rb

@@ -152,6 +152,14 @@ feature 'Login' do
         enter_code(user.current_otp)
         expect(current_path).to eq root_path
       end
+
+      it 'creates a security event after failed OAuth login' do
+        stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config])
+        user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')
+        gitlab_sign_in_via('saml', user, 'wrong-uid')
+
+        expect(SecurityEvent.where(entity_id: -1).count).to eq(1)
+      end
     end
   end
 
@@ -173,6 +181,8 @@ feature 'Login' do
 
       gitlab_sign_in(user)
       expect(page).to have_content('Invalid Login or password.')
+
+      expect(SecurityEvent.where(entity_id: -1).count).to eq(1)
     end
   end