Merge branch '9470-audit-failed-ldap-logins' into 'master'

Audit failed login from OAuth provider

See merge request gitlab-org/gitlab!38473

app/controllers/omniauth_callbacks_controller.rb

@@ -27,6 +27,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
       user = User.by_login(params[:username])
 
       user&.increment_failed_attempts!
+      log_failed_login(params[:username], failed_strategy.name)
     end
 
     super
@@ -90,6 +91,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
 
   private
 
+  def log_failed_login(user, provider)
+    # overridden in EE
+  end
+
   def after_omniauth_failure_path_for(scope)
     if Feature.enabled?(:user_mode_in_session)
       return new_admin_session_path if current_user_mode.admin_mode_requested?
@@ -198,6 +203,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   end
 
   def fail_login(user)
+    log_failed_login(user.username, oauth['provider'])
+
     error_message = user.errors.full_messages.to_sentence
 
     redirect_to omniauth_error_path(oauth['provider'], error: error_message)

ee/app/controllers/ee/omniauth_callbacks_controller.rb

@@ -14,23 +14,16 @@ module EE
       handle_omniauth
     end
 
-    protected
-
-    override :fail_login
-    def fail_login(user)
-      log_failed_login(user.username, oauth['provider'])
-
-      super
-    end
-
     private
 
+    override :log_failed_login
     def log_failed_login(author, provider)
-      ::AuditEventService.new(author,
+      ::AuditEventService.new(
+        author,
         nil,
         ip_address: request.remote_ip,
-                            with: provider)
-          .for_failed_login.unauth_security_event
+        with: provider
+      ).for_failed_login.unauth_security_event
     end
   end
 end

ee/changelogs/unreleased/9470-audit-failed-ldap-logins.yml

+---
+title: Audit failed login from OAuth provider
+merge_request: 38473
+author: banovp
+type: fixed

ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe OmniauthCallbacksController, type: :controller do
+  include LoginHelpers
+
+  let_it_be(:extern_uid) { 'my-uid' }
+  let_it_be(:provider) { :ldap }
+  let_it_be(:user) { create(:omniauth_user, extern_uid: extern_uid, provider: provider) }
+
+  before do
+    mock_auth_hash(provider.to_s, extern_uid, user.email)
+    stub_omniauth_provider(provider, context: request)
+  end
+
+  context 'when sign in fails' do
+    before do
+      subject.response = ActionDispatch::Response.new
+
+      allow(subject).to receive(:params)
+        .and_return(ActionController::Parameters.new(username: user.username))
+
+      stub_omniauth_failure(
+        OmniAuth::Strategies::LDAP.new(nil),
+        'invalid_credentials',
+        OmniAuth::Strategies::LDAP::InvalidCredentialsError.new('Invalid credentials for ldap')
+      )
+    end
+
+    it 'audits provider failed login when licensed' do
+      stub_licensed_features(extended_audit_events: true)
+      expect { subject.failure }.to change { AuditEvent.count }.by(1)
+    end
+
+    it 'does not audit provider failed login when unlicensed' do
+      stub_licensed_features(extended_audit_events: false)
+      expect { subject.failure }.not_to change { AuditEvent.count }
+    end
+  end
+end