changelog: Bug Fixes, Authentication, AAl3 Users cannot skip MFA selection for pivcac (LG-7020)

app/controllers/users/mfa_selection_controller.rb

@@ -9,7 +9,7 @@ class MfaSelectionController < ApplicationController
     before_action :multiple_factors_enabled?
 
     def index
-      @two_factor_options_form = TwoFactorOptionsForm.new(current_user)
+      two_factor_options_form
       @after_setup_path = after_mfa_setup_path
       @presenter = two_factor_options_presenter
       analytics.user_registration_2fa_additional_setup_visit
@@ -37,8 +37,7 @@ def update
     private
 
     def submit_form
-      @two_factor_options_form = TwoFactorOptionsForm.new(current_user)
-      @two_factor_options_form.submit(two_factor_options_form_params)
+      two_factor_options_form.submit(two_factor_options_form_params)
     end
 
     def two_factor_options_presenter
@@ -50,6 +49,14 @@ def two_factor_options_presenter
       )
     end
 
+    def two_factor_options_form
+      @two_factor_options_form ||= TwoFactorOptionsForm.new(
+        user: current_user,
+        aal3_required: service_provider_mfa_policy.aal3_required?,
+        piv_cac_required: service_provider_mfa_policy.piv_cac_required?,
+      )
+    end
+
     def process_valid_form
       user_session[:mfa_selections] = @two_factor_options_form.selection
 

app/controllers/users/two_factor_authentication_setup_controller.rb

@@ -8,7 +8,7 @@ class TwoFactorAuthenticationSetupController < ApplicationController
     before_action :confirm_user_needs_2fa_setup
 
     def index
-      @two_factor_options_form = TwoFactorOptionsForm.new(current_user)
+      two_factor_options_form
       @presenter = two_factor_options_presenter
       analytics.user_registration_2fa_setup_visit
     end
@@ -24,6 +24,7 @@ def create
         flash[:phone_error] = t('errors.two_factor_auth_setup.must_select_additional_option')
         redirect_to authentication_methods_setup_path(anchor: 'select_phone')
       else
+        flash[:error] = t('errors.two_factor_auth_setup.must_select_option')
         @presenter = two_factor_options_presenter
         render :index
       end
@@ -35,8 +36,7 @@ def create
     private
 
     def submit_form
-      @two_factor_options_form = TwoFactorOptionsForm.new(current_user)
-      @two_factor_options_form.submit(two_factor_options_form_params)
+      two_factor_options_form.submit(two_factor_options_form_params)
     end
 
     def two_factor_options_presenter
@@ -48,6 +48,14 @@ def two_factor_options_presenter
       )
     end
 
+    def two_factor_options_form
+      @two_factor_options_form ||= TwoFactorOptionsForm.new(
+        user: current_user,
+        aal3_required: service_provider_mfa_policy.aal3_required?,
+        piv_cac_required: service_provider_mfa_policy.piv_cac_required?,
+      )
+    end
+
     def process_valid_form
       user_session[:mfa_selections] = @two_factor_options_form.selection
       redirect_to confirmation_path(user_session[:mfa_selections].first)

app/forms/two_factor_options_form.rb

@@ -1,18 +1,19 @@
 class TwoFactorOptionsForm
   include ActiveModel::Model
 
-  attr_reader :selection
-  attr_reader :configuration_id
+  attr_accessor :selection, :user, :aal3_required, :piv_cac_required
 
   validates :selection, inclusion: { in: %w[phone sms voice auth_app piv_cac
                                             webauthn webauthn_platform
                                             backup_code] }
 
-  validates :selection, length: { minimum: 1 }, if: :has_no_configured_mfa?
+  validates :selection, length: { minimum: 1 }, if: :has_no_mfa_or_in_required_flow?
   validates :selection, length: { minimum: 2, message: 'phone' }, if: :phone_validations?
 
-  def initialize(user)
+  def initialize(user:, aal3_required:, piv_cac_required:)
     self.user = user
+    self.aal3_required = aal3_required
+    self.piv_cac_required = piv_cac_required
   end
 
   def submit(params)
@@ -25,9 +26,6 @@ def submit(params)
 
   private
 
-  attr_accessor :user
-  attr_writer :selection
-
   def mfa_user
     @mfa_user ||= MfaContext.new(user)
   end
@@ -39,6 +37,10 @@ def extra_analytics_attributes
     }
   end
 
+  def in_aal3_or_piv_cac_required_flow?
+    aal3_required || piv_cac_required
+  end
+
   def user_needs_updating?
     (%w[voice sms] & selection).present? &&
       !selection.include?(user.otp_delivery_preference)
@@ -58,6 +60,10 @@ def has_no_configured_mfa?
     mfa_user.enabled_mfa_methods_count == 0
   end
 
+  def has_no_mfa_or_in_required_flow?
+    has_no_configured_mfa? || in_aal3_or_piv_cac_required_flow?
+  end
+
   def kantara_2fa_phone_restricted?
     IdentityConfig.store.kantara_2fa_phone_restricted
   end

spec/controllers/users/mfa_selection_controller_spec.rb

@@ -36,8 +36,9 @@
       params = ActionController::Parameters.new(voice_params)
       response = FormResponse.new(success: true, errors: {}, extra: { selection: ['voice'] })
 
+      form_params = { user: user, aal3_required: false, piv_cac_required: nil }
       form = instance_double(TwoFactorOptionsForm)
-      allow(TwoFactorOptionsForm).to receive(:new).with(user).and_return(form)
+      allow(TwoFactorOptionsForm).to receive(:new).with(form_params).and_return(form)
       expect(form).to receive(:submit).
         with(params.require(:two_factor_options_form).permit(:selection)).
         and_return(response)

spec/controllers/users/two_factor_authentication_setup_controller_spec.rb

@@ -67,8 +67,9 @@
       params = ActionController::Parameters.new(voice_params)
       response = FormResponse.new(success: true, errors: {}, extra: { selection: ['voice'] })
 
+      form_params = { user: user, aal3_required: false, piv_cac_required: nil }
       form = instance_double(TwoFactorOptionsForm)
-      allow(TwoFactorOptionsForm).to receive(:new).with(user).and_return(form)
+      allow(TwoFactorOptionsForm).to receive(:new).with(form_params).and_return(form)
       expect(form).to receive(:submit).
         with(params.require(:two_factor_options_form).permit(:selection)).
         and_return(response)

spec/features/openid_connect/aal3_required_spec.rb

@@ -47,6 +47,21 @@
         expect(page).to have_content(t('two_factor_authentication.two_factor_aal3_choice'))
         expect(page).to have_xpath("//img[@alt='important alert icon']")
       end
+
+      it 'throws an error if user doesnt select AAL3 auth' do
+        user = user_with_2fa
+
+        visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account')
+        sign_in_live_with_2fa(user)
+
+        expect(current_url).to eq(authentication_methods_setup_url)
+        expect(page).to have_content(t('two_factor_authentication.two_factor_aal3_choice'))
+        expect(page).to have_xpath("//img[@alt='important alert icon']")
+
+        click_continue
+
+        expect(page).to have_content(t('errors.two_factor_auth_setup.must_select_option'))
+      end
     end
 
     context 'user has aal3 auth configured' do

spec/forms/two_factor_options_form_spec.rb

@@ -2,7 +2,15 @@
 
 describe TwoFactorOptionsForm do
   let(:user) { build(:user) }
-  subject { described_class.new(user) }
+  let(:aal3_required) { false }
+  let(:piv_cac_required) { false }
+  subject {
+    described_class.new(
+      user: user,
+      aal3_required: aal3_required,
+      piv_cac_required: piv_cac_required,
+    )
+  }
 
   describe '#submit' do
     let(:submit_phone) { subject.submit(selection: 'phone') }
@@ -120,6 +128,37 @@
       end
     end
 
+    context 'when a user wants to is required to add piv_cac on sign in' do
+      let(:user) { build(:user, :with_authentication_app) }
+      let(:enabled_mfa_methods_count) { 1 }
+      let(:mfa_selection) { ['phone'] }
+      let(:aal3_required) { true }
+      let(:piv_cac_required) { false }
+
+      before do
+        allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return(true)
+      end
+
+      context 'when user is didnt select an mfa' do
+        let(:mfa_selection) { nil }
+
+        it 'does not submits the form' do
+          submission = subject.submit(selection: mfa_selection)
+          expect(submission.success?).to be_falsey
+        end
+      end
+
+      context 'when user selects an mfa' do
+        it 'submits the form' do
+          submission = subject.submit(selection: mfa_selection)
+          expect(submission.success?).to be_truthy
+        end
+      end
+    end
+
+    context 'when user doesnt select mfa selection with existing account' do
+    end
+
     context 'when the feature flag toggle for 2FA phone restriction is off' do
       before do
         allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return(true)