Remove controller-level "bypass" check for SP MFA requirement

app/controllers/concerns/two_factor_authenticatable_methods.rb

@@ -90,17 +90,6 @@ def check_already_authenticated
     redirect_to after_sign_in_path_for(current_user)
   end
 
-  def check_sp_required_mfa_bypass(auth_method:)
-    return unless service_provider_mfa_policy.user_needs_sp_auth_method_verification?
-    return if service_provider_mfa_policy.phishing_resistant_required? &&
-              TwoFactorAuthenticatable::AuthMethod.phishing_resistant?(auth_method)
-    if service_provider_mfa_policy.piv_cac_required? &&
-       auth_method == TwoFactorAuthenticatable::AuthMethod::PIV_CAC
-      return
-    end
-    prompt_to_verify_sp_required_mfa
-  end
-
   def reset_attempt_count_if_user_no_longer_locked_out
     return unless current_user.no_longer_locked_out?
 

app/controllers/two_factor_authentication/backup_code_verification_controller.rb

@@ -6,7 +6,6 @@ class BackupCodeVerificationController < ApplicationController
     include NewDeviceConcern
 
     prepend_before_action :authenticate_user
-    before_action :check_sp_required_mfa
 
     def show
       analytics.multi_factor_auth_enter_backup_code_visit(context: context)
@@ -80,9 +79,5 @@ def backup_code_params
     def handle_valid_backup_code
       redirect_to after_sign_in_path_for(current_user)
     end
-
-    def check_sp_required_mfa
-      check_sp_required_mfa_bypass(auth_method: 'backup_code')
-    end
   end
 end

app/controllers/two_factor_authentication/otp_verification_controller.rb

@@ -6,7 +6,6 @@ class OtpVerificationController < ApplicationController
     include MfaSetupConcern
     include NewDeviceConcern
 
-    before_action :check_sp_required_mfa
     before_action :confirm_multiple_factors_enabled
     before_action :redirect_if_blank_phone, only: [:show]
     before_action :confirm_voice_capability, only: [:show]
@@ -197,10 +196,6 @@ def confirmation_for_add_phone?
       UserSessionContext.confirmation_context?(context) && user_fully_authenticated?
     end
 
-    def check_sp_required_mfa
-      check_sp_required_mfa_bypass(auth_method: params[:otp_delivery_preference])
-    end
-
     def assign_phone
       if updating_existing_number?
         phone_changed

app/controllers/two_factor_authentication/totp_verification_controller.rb

@@ -5,7 +5,6 @@ class TotpVerificationController < ApplicationController
     include TwoFactorAuthenticatable
     include NewDeviceConcern
 
-    before_action :check_sp_required_mfa
     before_action :confirm_totp_enabled
 
     def show
@@ -57,9 +56,5 @@ def authenticator_view_data
         two_factor_authentication_method: 'authenticator',
       }.merge(generic_data)
     end
-
-    def check_sp_required_mfa
-      check_sp_required_mfa_bypass(auth_method: 'authenticator')
-    end
   end
 end

app/controllers/two_factor_authentication/webauthn_verification_controller.rb

@@ -6,7 +6,6 @@ class WebauthnVerificationController < ApplicationController
     include TwoFactorAuthenticatable
     include NewDeviceConcern
 
-    before_action :check_sp_required_mfa
     before_action :confirm_webauthn_enabled, only: :show
 
     def show
@@ -124,10 +123,6 @@ def form
       )
     end
 
-    def check_sp_required_mfa
-      check_sp_required_mfa_bypass(auth_method: 'webauthn')
-    end
-
     def platform_authenticator_param?
       params[:platform].to_s == 'true'
     end

spec/features/openid_connect/phishing_resistant_required_spec.rb

@@ -60,36 +60,42 @@
       context 'with piv cac configured' do
         let(:user) { create(:user, :fully_registered, :with_piv_or_cac) }
 
-        it 'sends user to authenticate with piv cac' do
+        it 'sends user to authenticate with piv cac and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_requesting_aal3(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_piv_cac_url)
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:piv_cac)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
       context 'with webauthn configured' do
         let(:user) { create(:user, :fully_registered, :with_webauthn) }
 
-        it 'sends user to authenticate with webauthn' do
+        it 'sends user to authenticate with webauthn and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_requesting_aal3(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_webauthn_url)
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:webauthn)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
       context 'with webauthn platform configured' do
         let(:user) { create(:user, :fully_registered, :with_webauthn_platform) }
 
-        it 'sends user to authenticate with webauthn platform' do
+        it 'sends user to authenticate with webauthn platform and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_requesting_aal3(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_webauthn_url(platform: true))
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:webauthn_platform)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
@@ -132,36 +138,42 @@
       context 'with piv cac configured' do
         let(:user) { create(:user, :fully_registered, :with_piv_or_cac) }
 
-        it 'sends user to authenticate with piv cac' do
+        it 'sends user to authenticate with piv cac and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_requesting_phishing_resistant(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_piv_cac_url)
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:piv_cac)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
       context 'with webauthn configured' do
         let(:user) { create(:user, :fully_registered, :with_webauthn) }
 
-        it 'sends user to authenticate with webauthn' do
+        it 'sends user to authenticate with webauthn and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_requesting_phishing_resistant(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_webauthn_url)
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:webauthn)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
       context 'with webauthn platform configured' do
         let(:user) { create(:user, :fully_registered, :with_webauthn_platform) }
 
-        it 'sends user to authenticate with webauthn platform' do
+        it 'sends user to authenticate with webauthn platform and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_requesting_phishing_resistant(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_webauthn_url(platform: true))
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:webauthn_platform)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
@@ -204,36 +216,42 @@
       context 'with piv cac configured' do
         let(:user) { create(:user, :fully_registered, :with_piv_or_cac) }
 
-        it 'sends user to authenticate with piv cac' do
+        it 'sends user to authenticate with piv cac and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_piv_cac_url)
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:piv_cac)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
       context 'with webauthn configured' do
         let(:user) { create(:user, :fully_registered, :with_webauthn) }
 
-        it 'sends user to authenticate with webauthn' do
+        it 'sends user to authenticate with webauthn and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_webauthn_url)
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:webauthn)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
       context 'with webauthn platform configured' do
         let(:user) { create(:user, :fully_registered, :with_webauthn_platform) }
 
-        it 'sends user to authenticate with webauthn platform' do
+        it 'sends user to authenticate with webauthn platform and removes weaker options' do
           sign_in_before_2fa(user)
 
           visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account')
-          visit login_two_factor_path(otp_delivery_preference: 'sms')
           expect(current_url).to eq(login_two_factor_webauthn_url(platform: true))
+          click_on t('two_factor_authentication.login_options_link_text')
+          expect(has_2fa_option?(:webauthn_platform)).to eq(true)
+          expect(has_2fa_option?(:sms)).to eq(false)
         end
       end
 
@@ -262,4 +280,11 @@
       end
     end
   end
+
+  def has_2fa_option?(auth_method)
+    page.find("label[for='two_factor_options_form_selection_#{auth_method}']")
+    true
+  rescue Capybara::ElementNotFound
+    false
+  end
 end