Lg 6167 second mfa for sms

app/assets/stylesheets/components/_validated-checkbox.scss

@@ -0,0 +1,7 @@
+.mfa-selection {
+  .usa-checkbox__input--tile:checked
+    + label.checkbox__invalid.usa-checkbox__label.usa-checkbox__label--illustrated {
+    border-color: color('secondary');
+    border-width: 2px;
+  }
+}

app/assets/stylesheets/components/all.scss

@@ -24,4 +24,5 @@
 @import 'spinner-dots';
 @import 'step-indicator';
 @import 'troubleshooting-options';
+@import 'validated-checkbox';
 @import 'i18n-dropdown';

app/controllers/users/two_factor_authentication_setup_controller.rb

@@ -6,7 +6,6 @@ class TwoFactorAuthenticationSetupController < ApplicationController
     before_action :authenticate_user
     before_action :confirm_user_authenticated_for_2fa_setup
     before_action :confirm_user_needs_2fa_setup
-    before_action :handle_empty_selection, only: :create
 
     def index
       @two_factor_options_form = TwoFactorOptionsForm.new(current_user)
@@ -20,10 +19,16 @@ def create
 
       if result.success?
         process_valid_form
+      elsif result.errors[:selection].include? 'phone'
+        flash[:phone_error] = t('errors.two_factor_auth_setup.must_select_additional_option')
+        redirect_to two_factor_options_path(anchor: 'select_phone')
       else
         @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: two_factor_options_path, allow_other_host: false)
     end
 
     private
@@ -47,13 +52,6 @@ def process_valid_form
       redirect_to confirmation_path(user_session[:selected_mfa_options].first)
     end
 
-    def handle_empty_selection
-      return if params[:two_factor_options_form].present?
-
-      flash[:error] = t('errors.two_factor_auth_setup.must_select_option')
-      redirect_back(fallback_location: two_factor_options_path, allow_other_host: false)
-    end
-
     def confirm_user_needs_2fa_setup
       return unless mfa_policy.two_factor_enabled?
       return if params.has_key?(:multiple_mfa_setup)

app/forms/two_factor_options_form.rb

@@ -7,6 +7,10 @@ class TwoFactorOptionsForm
   validates :selection, inclusion: { in: %w[phone sms voice auth_app piv_cac
                                             webauthn webauthn_platform
                                             backup_code] }
+  validates :selection, length: { minimum: 2, message: 'phone' }, if: [
+    :multiple_mfa_options_enabled?,
+    :phone_selected?,
+  ]
 
   def initialize(user)
     self.user = user
@@ -17,7 +21,6 @@ def submit(params)
 
     success = valid?
     update_otp_delivery_preference_for_user if success && user_needs_updating?
-
     FormResponse.new(success: success, errors: errors, extra: extra_analytics_attributes)
   end
 
@@ -42,4 +45,12 @@ def update_otp_delivery_preference_for_user
       selection.find { |element| %w[voice sms].include?(element) } }
     UpdateUser.new(user: user, attributes: user_attributes).call
   end
+
+  def multiple_mfa_options_enabled?
+    IdentityConfig.store.select_multiple_mfa_options
+  end
+
+  def phone_selected?
+    selection.include?('phone') || selection.include?('voice') || selection.include?('sms')
+  end
 end

app/javascript/packs/mfa_selection_component.ts

@@ -0,0 +1,17 @@
+function clearPhoneSelectionError() {
+  const error = document.getElementById('phone_error');
+  const invalid = document.querySelector('label.checkbox__invalid');
+  if (error) {
+    error.style.display = 'none';
+  }
+  if (invalid) {
+    invalid.classList.remove('checkbox__invalid');
+  }
+}
+
+document.addEventListener('DOMContentLoaded', () => {
+  const checkboxes = document.getElementsByName('two_factor_options_form[selection][]');
+  checkboxes.forEach((checkbox) => {
+    checkbox.onchange = clearPhoneSelectionError;
+  });
+});

app/presenters/two_factor_authentication/phone_selection_presenter.rb

@@ -13,14 +13,9 @@ def type
     end
 
     def info
-      voip_note = if IdentityConfig.store.voip_block
-        t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip')
-      end
-
-      safe_join(
-        [t('two_factor_authentication.two_factor_choice_options.phone_info'), *voip_note],
-        ' ',
-      )
+      IdentityConfig.store.select_multiple_mfa_options ?
+          t('two_factor_authentication.two_factor_choice_options.phone_info_html') :
+          t('two_factor_authentication.two_factor_choice_options.phone_info')
     end
 
     def security_level

app/presenters/two_factor_authentication/selection_presenter.rb

@@ -112,7 +112,9 @@ def setup_info(type)
       when 'backup_code'
         t('two_factor_authentication.two_factor_choice_options.backup_code_info')
       when 'phone'
-        t('two_factor_authentication.two_factor_choice_options.phone_info')
+        IdentityConfig.store.select_multiple_mfa_options ?
+          t('two_factor_authentication.two_factor_choice_options.phone_info_html') :
+          t('two_factor_authentication.two_factor_choice_options.phone_info')
       when 'piv_cac'
         t('two_factor_authentication.two_factor_choice_options.piv_cac_info')
       when 'sms'

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

@@ -0,0 +1,31 @@
+<div class="mfa-selection">
+  <%= check_box_tag(
+        'two_factor_options_form[selection][]',
+        option.type,
+        option.type == 'phone' && flash[:phone_error].present?,
+        disabled: option.disabled?,
+        class: 'usa-checkbox__input usa-checkbox__input--tile',
+        id: "two_factor_options_form_selection_#{option.type}",
+      ) %>
+  <%= label_tag(
+        "two_factor_options_form_selection_#{option.type}",
+        class: [
+          option.type == 'phone' && flash[:phone_error] && 'checkbox__invalid',
+          'usa-checkbox__label',
+          'usa-checkbox__label--illustrated',
+        ].select(&:present?).join(' '),
+      ) do %>
+    <%= image_tag(asset_url("mfa-options/#{option.type}.svg"), alt: "#{option.label} icon", class: 'usa-checkbox__image') %>
+    <div class="usa-checkbox__label--text"><%= option.label %>
+      <span class="usa-checkbox__label-description">
+        <%= option.info %>
+      </span>
+    </div>
+  <% end %>
+  <% if option.type == "phone" && flash[:phone_error] %>
+    <div id="phone_error" class="usa-error-message margin-bottom-1 margin-top-neg-1" role="alert">
+        <%= flash[:phone_error] %>
+    </div>
+  <% end %>
+</div>
+<%= javascript_packs_tag_once('mfa_selection_component') %>

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

@@ -32,25 +32,8 @@
       <% @presenter.options.each do |option| %>
         <div id="<%= "select_#{option.type}" %>" class="<%= option.html_class %>">
           <% if IdentityConfig.store.select_multiple_mfa_options %>
-            <%= check_box_tag(
-                  'two_factor_options_form[selection][]',
-                  option.type,
-                  false,
-                  disabled: option.disabled?,
-                  class: 'usa-checkbox__input usa-checkbox__input--tile',
-                  id: "two_factor_options_form_selection_#{option.type}",
-                ) %>
-            <%= label_tag(
-                  "two_factor_options_form_selection_#{option.type}",
-                  class: 'usa-checkbox__label usa-checkbox__label--illustrated',
-                ) do %>
-                    <%= image_tag(asset_url("mfa-options/#{option.type}.svg"), alt: "#{option.label} icon", class: 'usa-checkbox__image') %>
-                    <div class="usa-checkbox__label--text"><%= option.label %>
-                      <span class="usa-checkbox__label-description">
-                        <%= option.info %>
-                      </span>
-                    </div>
-                  <% end %>
+            <%= render partial: 'mfa_selection_component',
+                       locals: { form: f, option: option } %>
           <% else %>
             <%= radio_button_tag(
                   'two_factor_options_form[selection]',

config/locales/errors/en.yml

@@ -107,6 +107,7 @@ en:
     sign_in:
       bad_password_limit: You have exceeeded the maximum sign in attempts.
     two_factor_auth_setup:
+      must_select_additional_option: Select an additional authentication method.
       must_select_option: Select an authentication method.
     verify_personal_key:
       throttled: You tried too many times, please try again in %{timeout}.

config/locales/errors/es.yml

@@ -112,6 +112,7 @@ es:
     sign_in:
       bad_password_limit: Has superado el número máximo de intentos de inicio de sesión.
     two_factor_auth_setup:
+      must_select_additional_option: Seleccione un método de autenticación adicional.
       must_select_option: Seleccione un método de autenticación.
     verify_personal_key:
       throttled: Lo intentaste muchas veces, vuelve a intentarlo en %{timeout}.

config/locales/errors/fr.yml

@@ -121,6 +121,7 @@ fr:
     sign_in:
       bad_password_limit: Vous avez dépassé le nombre maximal de tentatives de connexion.
     two_factor_auth_setup:
+      must_select_additional_option: Sélectionnez une méthode d’authentification supplémentaire.
       must_select_option: Sélectionnez une méthode d’authentification.
     verify_personal_key:
       throttled: Vous avez essayé plusieurs fois, essayez à nouveau dans %{timeout}.

config/locales/two_factor_authentication/en.yml

@@ -133,7 +133,9 @@ en:
       less_secure_label: Less secure
       more_secure_label: More Secure
       phone: Text or Voice Message
-      phone_info: Receive a secure code by (SMS) text or phone call to your device.
+      phone_info: Receive a secure code by (SMS) text or phone call.
+      phone_info_html: Receive a secure code by (SMS) text or phone call. <strong>You
+        need to select another method in addition to this one.</strong>
       phone_info_no_voip: Do not use web-based (VOIP) phone services or premium rate
         (toll) phone numbers.
       piv_cac: Government Employee ID

config/locales/two_factor_authentication/es.yml

@@ -144,8 +144,11 @@ es:
       less_secure_label: Menos seguro
       more_secure_label: Más seguro
       phone: Mensaje de texto o de voz
-      phone_info: Reciba un código de seguridad a través de un mensaje de texto (SMS)
-        o una llamada telefónica a su dispositivo.
+      phone_info: Recibir un código seguro por medio de un mensaje de texto (SMS) o
+        una llamada telefónica.
+      phone_info_html: Recibir un código seguro por medio de un mensaje de texto (SMS)
+        o una llamada telefónica. <strong>Tienes que elegir otro método además
+        de este.</strong>
       phone_info_no_voip: Se prohíbe el uso de servicios telefónicos basados en la web
         (VOIP) o de números de teléfono de tarificación adicional (de pago).
       piv_cac: Identificación de empleado gubernamental

config/locales/two_factor_authentication/fr.yml

@@ -148,8 +148,10 @@ fr:
       less_secure_label: Moins sécurisé
       more_secure_label: Plus sécurisé
       phone: Message texte ou vocal
-      phone_info: Recevez un code sécurisé par message texte ou appel téléphonique sur
-        votre appareil.
+      phone_info: Recevoir un code de sécurité par texto (SMS) ou appel téléphonique.
+      phone_info_html: Recevoir un code de sécurité par texto (SMS) ou appel
+        téléphonique. <strong>Vous devez sélectionner une autre méthode en plus
+        de celle-ci.</strong>
       phone_info_no_voip: N’utilisez pas de services téléphoniques basés sur le Web (
         Voix sur IP ) ou de numéros de téléphone à tarif majoré ( péage ).
       piv_cac: Carte d’identification des employés du gouvernement

db/schema.rb

@@ -171,8 +171,8 @@
     t.string "state"
     t.boolean "aamva"
     t.datetime "verify_submit_at"
-    t.datetime "verify_phone_submit_at"
     t.integer "verify_phone_submit_count", default: 0
+    t.datetime "verify_phone_submit_at"
     t.datetime "document_capture_submit_at"
     t.index ["issuer"], name: "index_doc_auth_logs_on_issuer"
     t.index ["user_id"], name: "index_doc_auth_logs_on_user_id", unique: true

spec/controllers/users/two_factor_authentication_setup_controller_spec.rb

@@ -81,7 +81,7 @@
       stub_analytics
 
       result = {
-        selection: ['voice'],
+        selection: ['voice', 'auth_app'],
         success: true,
         errors: {},
       }
@@ -91,22 +91,30 @@
 
       patch :create, params: {
         two_factor_options_form: {
-          selection: 'voice',
+          selection: ['voice', 'auth_app'],
         },
       }
     end
 
-    context 'when the selection is phone' do
-      it 'redirects to phone setup page' do
+    context 'when the selection is only phone and multi mfa is enabled' do
+      before do
+        allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return(true)
         stub_sign_in_before_2fa
 
         patch :create, params: {
           two_factor_options_form: {
             selection: 'phone',
           },
         }
+      end
 
-        expect(response).to redirect_to phone_setup_url
+      it 'the redirect to the form page with an anchor' do
+        expect(response).to redirect_to(two_factor_options_path(anchor: 'select_phone'))
+      end
+      it 'contains a flash message' do
+        expect(flash[:phone_error]).to eq(
+          t('errors.two_factor_auth_setup.must_select_additional_option'),
+        )
       end
     end
 

spec/features/two_factor_authentication/multiple_mfa_sign_up_spec.rb

@@ -76,6 +76,34 @@
     end
   end
 
+  describe 'user attempts to submit with only the phone MFA method selected', js: true do
+    before do
+      sign_in_before_2fa
+      click_2fa_option('phone')
+      click_on t('forms.buttons.continue')
+    end
+
+    scenario 'redirects to the two_factor path with an error and phone option selected' do
+      expect(page).
+      to have_content(t('errors.two_factor_auth_setup.must_select_additional_option'))
+      expect(
+        URI.parse(current_url).path + '#' + URI.parse(current_url).fragment,
+      ).to eq two_factor_options_path(anchor: 'select_phone')
+    end
+
+    scenario 'clears the error when another mfa method is selected' do
+     click_2fa_option('backup_code')
+     expect(page).
+        to_not have_content(t('errors.two_factor_auth_setup.must_select_additional_option'))
+    end
+
+    scenario 'clears the error when phone mfa method is unselected' do
+     click_2fa_option('phone')
+     expect(page).
+       to_not have_content(t('errors.two_factor_auth_setup.must_select_additional_option'))
+    end
+  end
+
   def click_2fa_option(option)
     find("label[for='two_factor_options_form_selection_#{option}']").click
   end

spec/forms/two_factor_options_form_spec.rb

@@ -6,22 +6,33 @@
 
   describe '#submit' do
     it 'is successful if the selection is valid' do
-      %w[voice sms auth_app piv_cac webauthn webauthn_platform].each do |selection|
+      %w[auth_app piv_cac webauthn webauthn_platform].each do |selection|
         result = subject.submit(selection: selection)
 
         expect(result.success?).to eq true
       end
     end
 
+    it 'is unsuccessful if the selection is invalid for multi mfa' do
+      allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return(true)
+      %w[phone sms voice !!!!].each do |selection|
+        result = subject.submit(selection: selection)
+
+        expect(result.success?).to eq false
+      end
+    end
+
     it 'is unsuccessful if the selection is invalid' do
-      result = subject.submit(selection: '!!!!')
+      %w[!!!!].each do |selection|
+        result = subject.submit(selection: selection)
 
-      expect(result.success?).to eq false
-      expect(result.errors).to include :selection
+        expect(result.success?).to eq false
+        expect(result.errors).to include :selection
+      end
     end
 
     context "when the selection is different from the user's otp_delivery_preference" do
-      it "updates the user's otp_delivery_preference" do
+      it "updates the user's otp_delivery_preference if they have an alternate method selected" do
         user_updater = instance_double(UpdateUser)
         allow(UpdateUser).
           to receive(:new).
@@ -32,7 +43,7 @@
           and_return(user_updater)
         expect(user_updater).to receive(:call)
 
-        subject.submit(selection: 'voice')
+        subject.submit(selection: ['voice', 'backup_code'])
       end
     end