LG-10050: Prevent MFA setup validation error from persisting

app/controllers/users/mfa_selection_controller.rb

@@ -22,12 +22,9 @@ def update
       if result.success?
         process_valid_form
       else
-        flash[:error] = t('errors.two_factor_auth_setup.must_select_additional_option')
-        redirect_back(fallback_location: second_mfa_setup_path, allow_other_host: false)
+        flash[:error] = result.first_error_message
+        redirect_to second_mfa_setup_path
       end
-    rescue ActionController::ParameterMissing
-      flash[:error] = t('errors.two_factor_auth_setup.must_select_option')
-      redirect_back(fallback_location: two_factor_options_path, allow_other_host: false)
     end
 
     # @api private
@@ -67,6 +64,8 @@ def process_valid_form
 
     def two_factor_options_form_params
       params.require(:two_factor_options_form).permit(:selection, selection: [])
+    rescue ActionController::ParameterMissing
+      ActionController::Parameters.new(selection: [])
     end
   end
 end

app/controllers/users/two_factor_authentication_setup_controller.rb

@@ -25,13 +25,10 @@ def create
       if result.success?
         process_valid_form
       else
-        flash[:error] = t('errors.two_factor_auth_setup.must_select_option')
+        flash.now[:error] = result.first_error_message
         @presenter = two_factor_options_presenter
         render :index
       end
-    rescue ActionController::ParameterMissing
-      flash[:error] = t('errors.two_factor_auth_setup.must_select_option')
-      redirect_back(fallback_location: authentication_methods_setup_path, allow_other_host: false)
     end
 
     # @api private
@@ -71,6 +68,8 @@ def confirm_user_needs_2fa_setup
 
     def two_factor_options_form_params
       params.require(:two_factor_options_form).permit(:selection, selection: [])
+    rescue ActionController::ParameterMissing
+      ActionController::Parameters.new(selection: [])
     end
   end
 end

app/forms/two_factor_options_form.rb

@@ -1,13 +1,14 @@
 class TwoFactorOptionsForm
   include ActiveModel::Model
+  include ActionView::Helpers::TranslationHelper
 
   attr_accessor :selection, :user, :phishing_resistant_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_mfa_or_in_required_flow?
+  validate :validate_selection_present
 
   def initialize(user:, phishing_resistant_required:, piv_cac_required:)
     self.user = user
@@ -16,7 +17,7 @@ def initialize(user:, phishing_resistant_required:, piv_cac_required:)
   end
 
   def submit(params)
-    self.selection = Array(params[:selection]).filter(&:present?)
+    self.selection = params[:selection]
 
     success = valid?
     update_otp_delivery_preference_for_user if success && user_needs_updating?
@@ -25,6 +26,11 @@ def submit(params)
 
   private
 
+  def validate_selection_present
+    return if !has_no_mfa_or_in_required_flow? || selection.present?
+    errors.add(:selection, missing_selection_error_message, type: :missing_selection)
+  end
+
   def mfa_user
     @mfa_user ||= MfaContext.new(user)
   end
@@ -66,7 +72,16 @@ def platform_auth_only_option?
   end
 
   def has_no_mfa_or_in_required_flow?
-    has_no_configured_mfa? || in_phishing_resistant_or_piv_cac_required_flow? ||
+    has_no_configured_mfa? ||
+      in_phishing_resistant_or_piv_cac_required_flow? ||
       platform_auth_only_option?
   end
+
+  def missing_selection_error_message
+    if has_no_configured_mfa? || in_phishing_resistant_or_piv_cac_required_flow?
+      t('errors.two_factor_auth_setup.must_select_option')
+    elsif platform_auth_only_option?
+      t('errors.two_factor_auth_setup.must_select_additional_option')
+    end
+  end
 end

app/views/users/mfa_selection/index.html.erb

@@ -11,7 +11,6 @@
   <div class="margin-bottom-4">
     <fieldset class="margin-0 padding-0 border-0">
       <legend class="margin-bottom-2 usa-sr-only"><%= @presenter.intro %></legend>
-      <%= hidden_field_tag 'two_factor_options_form[selection][]', nil %>
       <% @presenter.options.each do |option| %>
         <%= render(option) do %>
           <%= render partial: 'partials/multi_factor_authentication/mfa_selection',

app/views/users/two_factor_authentication_setup/index.html.erb

@@ -23,7 +23,6 @@
       <legend class="margin-bottom-2 usa-sr-only">
         <%= t('two_factor_authentication.form_legend') %>
       </legend>
-      <%= hidden_field_tag 'two_factor_options_form[selection][]', nil %>
       <% @presenter.options.each do |option| %>
         <%= render(option) do %>
           <%= render partial: 'partials/multi_factor_authentication/mfa_selection',

spec/controllers/users/mfa_selection_controller_spec.rb

@@ -27,7 +27,7 @@
 
       voice_params = {
         two_factor_options_form: {
-          selection: 'voice',
+          selection: ['voice'],
         },
       }
 
@@ -51,7 +51,7 @@
 
         patch :update, params: {
           two_factor_options_form: {
-            selection: 'phone',
+            selection: ['phone'],
           },
         }
 
@@ -91,7 +91,7 @@
 
         patch :update, params: {
           two_factor_options_form: {
-            selection: 'auth_app',
+            selection: ['auth_app'],
           },
         }
 
@@ -105,7 +105,7 @@
 
         patch :update, params: {
           two_factor_options_form: {
-            selection: 'webauthn',
+            selection: ['webauthn'],
           },
         }
 
@@ -119,7 +119,7 @@
 
         patch :update, params: {
           two_factor_options_form: {
-            selection: 'webauthn_platform',
+            selection: ['webauthn_platform'],
           },
         }
 
@@ -133,7 +133,7 @@
 
         patch :update, params: {
           two_factor_options_form: {
-            selection: 'piv_cac',
+            selection: ['piv_cac'],
           },
         }
 
@@ -149,15 +149,15 @@
       end
 
       context 'with no active MFA' do
+        let(:user) { build(:user) }
+
         it 'redirects to the index page with a flash error' do
           patch :update, params: {
             two_factor_options_form: {},
           }
 
-          expect(response).to redirect_to two_factor_options_path
-          expect(flash[:error]).to eq(
-            t('errors.two_factor_auth_setup.must_select_option'),
-          )
+          expect(response).to redirect_to second_mfa_setup_path
+          expect(flash[:error]).to eq(t('errors.two_factor_auth_setup.must_select_option'))
         end
       end
 
@@ -166,9 +166,7 @@
           create(:phone_configuration, user: user)
 
           patch :update, params: {
-            two_factor_options_form: {
-              selection: [''],
-            },
+            two_factor_options_form: {},
           }
 
           expect(response).to redirect_to account_path
@@ -182,7 +180,7 @@
 
         patch :update, params: {
           two_factor_options_form: {
-            selection: 'foo',
+            selection: ['foo'],
           },
         }
 

spec/controllers/users/two_factor_authentication_setup_controller_spec.rb

@@ -61,7 +61,7 @@
 
       voice_params = {
         two_factor_options_form: {
-          selection: 'voice',
+          selection: ['voice'],
         },
       }
 
@@ -149,7 +149,7 @@
 
         patch :create, params: {
           two_factor_options_form: {
-            selection: 'auth_app',
+            selection: ['auth_app'],
           },
         }
 
@@ -163,7 +163,7 @@
 
         patch :create, params: {
           two_factor_options_form: {
-            selection: 'webauthn',
+            selection: ['webauthn'],
           },
         }
 
@@ -177,7 +177,7 @@
 
         patch :create, params: {
           two_factor_options_form: {
-            selection: 'webauthn_platform',
+            selection: ['webauthn_platform'],
           },
         }
 
@@ -191,7 +191,7 @@
 
         patch :create, params: {
           two_factor_options_form: {
-            selection: 'piv_cac',
+            selection: ['piv_cac'],
           },
         }
 
@@ -205,7 +205,7 @@
 
         patch :create, params: {
           two_factor_options_form: {
-            selection: 'foo',
+            selection: ['foo'],
           },
         }