diff --git a/app/assets/stylesheets/components/_validated-checkbox.scss b/app/assets/stylesheets/components/_validated-checkbox.scss new file mode 100644 index 00000000000..7bb8efb9024 --- /dev/null +++ b/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; + } +} diff --git a/app/assets/stylesheets/components/all.scss b/app/assets/stylesheets/components/all.scss index 1a0947e5b58..435850e3922 100644 --- a/app/assets/stylesheets/components/all.scss +++ b/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'; diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index 9543960b3cd..3731705cdfc 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/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) diff --git a/app/forms/two_factor_options_form.rb b/app/forms/two_factor_options_form.rb index e25258d8676..addd67d4ee6 100644 --- a/app/forms/two_factor_options_form.rb +++ b/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 diff --git a/app/javascript/packs/mfa_selection_component.ts b/app/javascript/packs/mfa_selection_component.ts new file mode 100644 index 00000000000..3f25909de9d --- /dev/null +++ b/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; + }); +}); diff --git a/app/presenters/two_factor_authentication/phone_selection_presenter.rb b/app/presenters/two_factor_authentication/phone_selection_presenter.rb index 102540443e1..dc5f67c9292 100644 --- a/app/presenters/two_factor_authentication/phone_selection_presenter.rb +++ b/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 diff --git a/app/presenters/two_factor_authentication/selection_presenter.rb b/app/presenters/two_factor_authentication/selection_presenter.rb index 137f9ccf734..e03675b1445 100644 --- a/app/presenters/two_factor_authentication/selection_presenter.rb +++ b/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' diff --git a/app/views/users/two_factor_authentication_setup/_mfa_selection_component.html.erb b/app/views/users/two_factor_authentication_setup/_mfa_selection_component.html.erb new file mode 100644 index 00000000000..ebbb0934146 --- /dev/null +++ b/app/views/users/two_factor_authentication_setup/_mfa_selection_component.html.erb @@ -0,0 +1,31 @@ +
+ <%= 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') %> +
<%= option.label %> + + <%= option.info %> + +
+ <% end %> + <% if option.type == "phone" && flash[:phone_error] %> + + <% end %> +
+<%= javascript_packs_tag_once('mfa_selection_component') %> \ No newline at end of file diff --git a/app/views/users/two_factor_authentication_setup/index.html.erb b/app/views/users/two_factor_authentication_setup/index.html.erb index b5c060a52be..5b418f66bb4 100644 --- a/app/views/users/two_factor_authentication_setup/index.html.erb +++ b/app/views/users/two_factor_authentication_setup/index.html.erb @@ -32,25 +32,8 @@ <% @presenter.options.each do |option| %>
" 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') %> -
<%= option.label %> - - <%= option.info %> - -
- <% end %> + <%= render partial: 'mfa_selection_component', + locals: { form: f, option: option } %> <% else %> <%= radio_button_tag( 'two_factor_options_form[selection]', diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 049bee7e87a..78b14ac0fcb 100644 --- a/config/locales/errors/en.yml +++ b/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}. diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index 0674e94c139..0d6f56777f5 100644 --- a/config/locales/errors/es.yml +++ b/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}. diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index a173700b69a..3384668a7f6 100644 --- a/config/locales/errors/fr.yml +++ b/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}. diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 1281a2f41d4..d57527b7d9f 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/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. You + need to select another method in addition to this one. phone_info_no_voip: Do not use web-based (VOIP) phone services or premium rate (toll) phone numbers. piv_cac: Government Employee ID diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 1b732f24b5a..8568b851fe2 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/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. Tienes que elegir otro método además + de este. 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 diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 0426c5618aa..0509626e2a5 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/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. Vous devez sélectionner une autre méthode en plus + de celle-ci. 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 diff --git a/db/schema.rb b/db/schema.rb index cb721d721a2..ac3ccd694cd 100644 --- a/db/schema.rb +++ b/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 diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index 96c11d000d8..e09791c8c39 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/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,13 +91,14 @@ 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: { @@ -105,8 +106,15 @@ 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 diff --git a/spec/features/two_factor_authentication/multiple_mfa_sign_up_spec.rb b/spec/features/two_factor_authentication/multiple_mfa_sign_up_spec.rb index 921d5f03681..82b1af21c26 100644 --- a/spec/features/two_factor_authentication/multiple_mfa_sign_up_spec.rb +++ b/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 diff --git a/spec/forms/two_factor_options_form_spec.rb b/spec/forms/two_factor_options_form_spec.rb index bdc95b81d5c..b15ed8d6286 100644 --- a/spec/forms/two_factor_options_form_spec.rb +++ b/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 diff --git a/spec/presenters/two_factor_authentication/phone_selection_presenter_spec.rb b/spec/presenters/two_factor_authentication/phone_selection_presenter_spec.rb index b1232fd892e..a673c34bac2 100644 --- a/spec/presenters/two_factor_authentication/phone_selection_presenter_spec.rb +++ b/spec/presenters/two_factor_authentication/phone_selection_presenter_spec.rb @@ -6,6 +6,9 @@ describe '#info' do context 'when a user does not have a phone configuration (first time)' do let(:phone) { nil } + before do + allow(IdentityConfig.store).to receive(:select_multiple_mfa_options).and_return(false) + end it 'includes a note about choosing voice or sms' do expect(presenter.info). @@ -20,11 +23,6 @@ before do allow(IdentityConfig.store).to receive(:voip_block).and_return(true) end - - it 'tells people to not use voip numbers' do - expect(presenter.info). - to include(t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip')) - end end end end diff --git a/spec/views/users/two_factor_authentication_setup/_mfa_selection_component.html.erb_spec.rb b/spec/views/users/two_factor_authentication_setup/_mfa_selection_component.html.erb_spec.rb new file mode 100644 index 00000000000..1de633efa3a --- /dev/null +++ b/spec/views/users/two_factor_authentication_setup/_mfa_selection_component.html.erb_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +describe 'users/two_factor_authentication_setup/_mfa_selection_component.html.erb' do + include SimpleForm::ActionViewExtensions::FormHelper + include Devise::Test::ControllerHelpers + + let(:lookup_context) { ActionView::LookupContext.new(ActionController::Base.view_paths) } + let(:view_context) { ActionView::Base.new(lookup_context, {}, controller) } + let(:form_object) { User.new } + let(:presenter) { TwoFactorOptionsPresenter.new(user_agent: nil) } + let(:form_builder) do + SimpleForm::FormBuilder.new(form_object.model_name.param_key, form_object, view_context, {}) + end + + subject(:rendered) do + render partial: 'mfa_selection_component', locals: { + form: form_builder, + option: presenter.options[4], + } + end + + it 'renders an lg-validated-field tag' do + expect(rendered).to have_css('.mfa-selection') + end + + context 'before selecting options' do + it 'does not display any errors' do + expect(rendered).to_not have_css('.checkbox__invalid') + end + end +end