Audit failed 2-factor authentication

First failed factor authentication (i.e using password) is audited but
not the second failed factor authentication. This change audits the
later. The ability to view the second authentication attempts allows
better monitoring of malicious attacks when the user's password has been
compromised.

app/controllers/concerns/authenticates_with_two_factor.rb

@@ -89,10 +89,7 @@ module AuthenticatesWithTwoFactor
       user.save!
       sign_in(user, message: :two_factor_authenticated, event: :authentication)
     else
-      user.increment_failed_attempts!
-      Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=OTP")
-      flash.now[:alert] = _('Invalid two-factor code.')
-      prompt_for_two_factor(user)
+      handle_two_factor_failure(user, 'OTP', _('Invalid two-factor code.'))
     end
   end
 
@@ -101,7 +98,7 @@ module AuthenticatesWithTwoFactor
     if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge])
       handle_two_factor_success(user)
     else
-      handle_two_factor_failure(user, 'U2F')
+      handle_two_factor_failure(user, 'U2F', _('Authentication via U2F device failed.'))
     end
   end
 
@@ -109,7 +106,7 @@ module AuthenticatesWithTwoFactor
     if Webauthn::AuthenticateService.new(user, user_params[:device_response], session[:challenge]).execute
       handle_two_factor_success(user)
     else
-      handle_two_factor_failure(user, 'WebAuthn')
+      handle_two_factor_failure(user, 'WebAuthn', _('Authentication via WebAuthn device failed.'))
     end
   end
 
@@ -152,13 +149,19 @@ module AuthenticatesWithTwoFactor
     sign_in(user, message: :two_factor_authenticated, event: :authentication)
   end
 
-  def handle_two_factor_failure(user, method)
+  def handle_two_factor_failure(user, method, message)
     user.increment_failed_attempts!
+    log_failed_two_factor(user, method, request.remote_ip)
+
     Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=#{method}")
-    flash.now[:alert] = _('Authentication via %{method} device failed.') % { method: method }
+    flash.now[:alert] = message
     prompt_for_two_factor(user)
   end
 
+  def log_failed_two_factor(user, method, ip_address)
+    # overridden in EE
+  end
+
   def handle_changed_user(user)
     clear_two_factor_attempt!
 
@@ -173,3 +176,5 @@ module AuthenticatesWithTwoFactor
     Digest::SHA256.hexdigest(user.encrypted_password) != session[:user_password_hash]
   end
 end
+
+AuthenticatesWithTwoFactor.prepend_if_ee('EE::AuthenticatesWithTwoFactor')

app/controllers/concerns/authenticates_with_two_factor_for_admin_mode.rb

@@ -52,11 +52,7 @@ module AuthenticatesWithTwoFactorForAdminMode
       # The admin user has successfully passed 2fa, enable admin mode ignoring password
       enable_admin_mode
     else
-      user.increment_failed_attempts!
-      Gitlab::AppLogger.info("Failed Admin Mode Login: user=#{user.username} ip=#{request.remote_ip} method=OTP")
-      flash.now[:alert] = _('Invalid two-factor code.')
-
-      admin_mode_prompt_for_two_factor(user)
+      admin_handle_two_factor_failure(user, 'OTP', _('Invalid two-factor code.'))
     end
   end
 
@@ -64,7 +60,7 @@ module AuthenticatesWithTwoFactorForAdminMode
     if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge])
       admin_handle_two_factor_success
     else
-      admin_handle_two_factor_failure(user, 'U2F')
+      admin_handle_two_factor_failure(user, 'U2F', _('Authentication via U2F device failed.'))
     end
   end
 
@@ -72,7 +68,7 @@ module AuthenticatesWithTwoFactorForAdminMode
     if Webauthn::AuthenticateService.new(user, user_params[:device_response], session[:challenge]).execute
       admin_handle_two_factor_success
     else
-      admin_handle_two_factor_failure(user, 'WebAuthn')
+      admin_handle_two_factor_failure(user, 'WebAuthn', _('Authentication via WebAuthn device failed.'))
     end
   end
 
@@ -100,11 +96,12 @@ module AuthenticatesWithTwoFactorForAdminMode
     enable_admin_mode
   end
 
-  def admin_handle_two_factor_failure(user, method)
+  def admin_handle_two_factor_failure(user, method, message)
     user.increment_failed_attempts!
-    Gitlab::AppLogger.info("Failed Admin Mode Login: user=#{user.username} ip=#{request.remote_ip} method=#{method}")
-    flash.now[:alert] = _('Authentication via %{method} device failed.') % { method: method }
+    log_failed_two_factor(user, method, request.remote_ip)
 
+    Gitlab::AppLogger.info("Failed Admin Mode Login: user=#{user.username} ip=#{request.remote_ip} method=#{method}")
+    flash.now[:alert] = message
     admin_mode_prompt_for_two_factor(user)
   end
 end

doc/administration/audit_events.md

@@ -126,6 +126,7 @@ recorded:
 - User was added ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/251) in GitLab 12.8)
 - User was blocked via Admin Area ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/251) in GitLab 12.8)
 - User was blocked via API ([introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25872) in GitLab 12.9)
+- Failed second-factor authentication attempt ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/16826) in GitLab 13.5)
 
 It's possible to filter particular actions by choosing an audit data type from
 the filter dropdown box. You can further filter by specific group, project, or user
@@ -172,6 +173,7 @@ the steps bellow.
 
    ```ruby
    Feature.enable(:repository_push_audit_event)
+   ```
 
 ## Export to CSV **(PREMIUM ONLY)**
 

ee/app/controllers/concerns/ee/authenticates_with_two_factor.rb

+# frozen_string_literal: true
+
+module EE
+  module AuthenticatesWithTwoFactor
+    extend ::Gitlab::Utils::Override
+
+    override :log_failed_two_factor
+    def log_failed_two_factor(user, method, ip_address)
+      ::AuditEventService.new(
+        user,
+        user,
+        ip_address: ip_address,
+        with: method
+      ).for_failed_login.unauth_security_event
+    end
+  end
+end

ee/changelogs/unreleased/16826-audit-failed-2fa.yml

+---
+title: Audit failed 2-factor login attempt
+merge_request: 41641
+author:
+type: added

ee/spec/controllers/ee/admin/sessions_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Admin::SessionsController, :do_not_mock_admin_mode do
+  include_context 'custom session'
+
+  describe '#create' do
+    context 'when using two-factor authentication' do
+      def authenticate_2fa(user_params, otp_user_id: user.id)
+        post(:create, params: { user: user_params }, session: { otp_user_id: otp_user_id })
+      end
+
+      before do
+        sign_in(user)
+        controller.current_user_mode.request_admin_mode!
+      end
+
+      context 'when OTP authentication fails' do
+        it_behaves_like 'an auditable failed authentication' do
+          let(:user) { create(:admin, :two_factor) }
+          let(:operation) { authenticate_2fa(otp_attempt: 'invalid', otp_user_id: user.id) }
+          let(:method) { 'OTP' }
+        end
+      end
+
+      context 'when U2F authentication fails' do
+        before do
+          allow(U2fRegistration).to receive(:authenticate).and_return(false)
+        end
+
+        it_behaves_like 'an auditable failed authentication' do
+          let(:user) { create(:admin, :two_factor_via_u2f) }
+          let(:operation) { authenticate_2fa(device_response: 'invalid', otp_user_id: user.id) }
+          let(:method) { 'U2F' }
+        end
+      end
+
+      context 'when WebAuthn authentication fails' do
+        before do
+          stub_feature_flags(webauthn: true)
+          webauthn_authenticate_service = instance_spy(Webauthn::AuthenticateService, execute: false)
+          allow(Webauthn::AuthenticateService).to receive(:new).and_return(webauthn_authenticate_service)
+        end
+
+        it_behaves_like 'an auditable failed authentication' do
+          let(:user) { create(:admin, :two_factor_via_webauthn) }
+          let(:operation) { authenticate_2fa(device_response: 'invalid', otp_user_id: user.id) }
+          let(:method) { 'WebAuthn' }
+        end
+      end
+    end
+  end
+end

ee/spec/controllers/ee/sessions_controller_spec.rb

@@ -88,5 +88,45 @@ RSpec.describe SessionsController, :geo do
         end
       end
     end
+
+    context 'when using two-factor authentication' do
+      def authenticate_2fa(user_params, otp_user_id: user.id)
+        post(:create, params: { user: user_params }, session: { otp_user_id: otp_user_id })
+      end
+
+      context 'when OTP authentication fails' do
+        it_behaves_like 'an auditable failed authentication' do
+          let(:user) { create(:user, :two_factor) }
+          let(:operation) { authenticate_2fa(otp_attempt: 'invalid', otp_user_id: user.id) }
+          let(:method) { 'OTP' }
+        end
+      end
+
+      context 'when U2F authentication fails' do
+        before do
+          allow(U2fRegistration).to receive(:authenticate).and_return(false)
+        end
+
+        it_behaves_like 'an auditable failed authentication' do
+          let(:user) { create(:user, :two_factor_via_u2f) }
+          let(:operation) { authenticate_2fa(device_response: 'invalid', otp_user_id: user.id) }
+          let(:method) { 'U2F' }
+        end
+      end
+
+      context 'when WebAuthn authentication fails' do
+        before do
+          stub_feature_flags(webauthn: true)
+          webauthn_authenticate_service = instance_spy(Webauthn::AuthenticateService, execute: false)
+          allow(Webauthn::AuthenticateService).to receive(:new).and_return(webauthn_authenticate_service)
+        end
+
+        it_behaves_like 'an auditable failed authentication' do
+          let(:user) { create(:user, :two_factor_via_webauthn) }
+          let(:operation) { authenticate_2fa(device_response: 'invalid', otp_user_id: user.id) }
+          let(:method) { 'WebAuthn' }
+        end
+      end
+    end
   end
 end

ee/spec/features/users/login_spec.rb

@@ -32,6 +32,16 @@ RSpec.describe 'Login' do
       .to change { AuditEvent.where(entity_id: -1).count }.from(0).to(1)
   end
 
+  it 'creates a security event for an invalid one-time code' do
+    user = create(:user, :two_factor)
+    gitlab_sign_in(user)
+
+    expect do
+      fill_in 'user_otp_attempt', with: 'invalid_code'
+      click_button 'Verify code'
+    end.to change { AuditEvent.count }.by(1)
+  end
+
   describe 'smartcard authentication' do
     before do
       allow(Gitlab.config.smartcard).to receive(:enabled).and_return(true)

ee/spec/support/shared_examples/controllers/sessions_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'an auditable failed authentication' do
+  it 'log an audit event', :aggregate_failures do
+    audit_event_service = instance_spy(AuditEventService)
+    allow(AuditEventService).to receive(:new).and_return(audit_event_service)
+
+    operation
+
+    expect(AuditEventService).to have_received(:new).with(user, user, ip_address: '0.0.0.0', with: method)
+    expect(audit_event_service).to have_received(:for_failed_login)
+    expect(audit_event_service).to have_received(:unauth_security_event)
+  end
+end

locale/gitlab.pot

@@ -3676,7 +3676,10 @@ msgstr ""
 msgid "Authentication method updated"
 msgstr ""
 
-msgid "Authentication via %{method} device failed."
+msgid "Authentication via U2F device failed."
+msgstr ""
+
+msgid "Authentication via WebAuthn device failed."
 msgstr ""
 
 msgid "Author"