Merge branch '16826-audit-failed-2fa' into 'master'

Audit failed 2-factor login attempt

See merge request gitlab-org/gitlab!41641

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

@@ -3691,7 +3691,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"