Merge branch '34159-smarcards-as-2fa' into 'master'

Smartcard should be counted as 2fa

Closes #34159

See merge request gitlab-org/gitlab!29504

app/controllers/concerns/enforces_two_factor_authentication.rb

@@ -16,7 +16,7 @@ module EnforcesTwoFactorAuthentication
   end
 
   def check_two_factor_requirement
-    if two_factor_authentication_required? && current_user && !current_user.temp_oauth_email? && !current_user.two_factor_enabled? && !skip_two_factor?
+    if two_factor_authentication_required? && current_user_requires_two_factor?
       redirect_to profile_two_factor_auth_path
     end
   end
@@ -27,6 +27,10 @@ module EnforcesTwoFactorAuthentication
       current_user.try(:ultraauth_user?)
   end
 
+  def current_user_requires_two_factor?
+    current_user && !current_user.temp_oauth_email? && !current_user.two_factor_enabled? && !skip_two_factor?
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def two_factor_authentication_reason(global: -> {}, group: -> {})
     if two_factor_authentication_required?
@@ -61,3 +65,5 @@ module EnforcesTwoFactorAuthentication
     session[:skip_two_factor] && session[:skip_two_factor] > Time.current
   end
 end
+
+EnforcesTwoFactorAuthentication.prepend_if_ee('EE::EnforcesTwoFactorAuthentication')

ee/app/controllers/concerns/ee/enforces_two_factor_authentication.rb

+# frozen_string_literal: true
+
+module EE
+  module EnforcesTwoFactorAuthentication
+    extend ::Gitlab::Utils::Override
+
+    override :current_user_requires_two_factor?
+    def current_user_requires_two_factor?
+      super && !active_smartcard_session?
+    end
+
+    private
+
+    def active_smartcard_session?
+      return false unless ::Gitlab::Auth::Smartcard.enabled?
+
+      return false unless current_user.smartcard_identities.any?
+
+      ::Gitlab::Auth::Smartcard::Session.new.active?(current_user)
+    end
+  end
+end

ee/changelogs/unreleased/34159-smarcards-as-2fa.yml

+---
+title: Smartcard should be counted as 2fa
+merge_request: 29504
+author:
+type: fixed

ee/spec/features/users/login_spec.rb

@@ -77,6 +77,45 @@ describe 'Login' do
 
           expect(page).to have_selector('.nav-tabs a[href="#smartcard"]')
         end
+
+        describe 'with two-factor authentication required', :clean_gitlab_redis_shared_state do
+          let_it_be(:user) { create(:user) }
+          let_it_be(:smartcard_identity) { create(:smartcard_identity, user: user) }
+
+          before do
+            stub_application_setting(require_two_factor_authentication: true)
+          end
+
+          context 'with a smartcard session' do
+            let(:openssl_certificate_store) { instance_double(OpenSSL::X509::Store) }
+            let(:openssl_certificate) do
+              instance_double(OpenSSL::X509::Certificate, subject: smartcard_identity.subject, issuer: smartcard_identity.issuer)
+            end
+
+            it 'does not ask for Two-Factor Authentication' do
+              allow(Gitlab::Auth::Smartcard::Certificate).to receive(:store).and_return(openssl_certificate_store)
+              allow(OpenSSL::X509::Certificate).to receive(:new).and_return(openssl_certificate)
+              allow(openssl_certificate_store).to receive(:verify).and_return(true)
+
+              # Loging using smartcard
+              visit verify_certificate_smartcard_path(client_certificate: openssl_certificate)
+
+              visit profile_path
+
+              expect(page).not_to have_content('Two-Factor Authentication')
+            end
+          end
+
+          context 'without a smartcard session' do
+            it 'asks for Two-Factor Authentication' do
+              sign_in(user)
+
+              visit profile_path
+
+              expect(page).to have_content('Two-Factor Authentication')
+            end
+          end
+        end
       end
     end
   end