diff --git a/app/models/phone_configuration.rb b/app/models/phone_configuration.rb index 15311c0ffef..be629ce82e0 100644 --- a/app/models/phone_configuration.rb +++ b/app/models/phone_configuration.rb @@ -22,11 +22,13 @@ def selection_presenters capabilities = PhoneNumberCapabilities.new(phone, phone_confirmed: !!confirmed_at?) if capabilities.supports_sms? - options << TwoFactorAuthentication::SmsSelectionPresenter.new(user:, configuration: self) + options << TwoFactorAuthentication::SignInPhoneSelectionPresenter. + new(user:, configuration: self, method: :sms) end if capabilities.supports_voice? - options << TwoFactorAuthentication::VoiceSelectionPresenter.new(user:, configuration: self) + options << TwoFactorAuthentication::SignInPhoneSelectionPresenter. + new(user:, configuration: self, method: :voice) end options diff --git a/app/presenters/two_factor_authentication/selection_presenter.rb b/app/presenters/two_factor_authentication/selection_presenter.rb index 3a52380e482..3926937cd01 100644 --- a/app/presenters/two_factor_authentication/selection_presenter.rb +++ b/app/presenters/two_factor_authentication/selection_presenter.rb @@ -68,27 +68,11 @@ def disabled? private def login_label(type) - case type - when 'sms' - t('two_factor_authentication.login_options.sms') - when 'voice' - t('two_factor_authentication.login_options.voice') - else - raise "Unsupported login method: #{type}" - end + raise "Unsupported login method: #{type}" end def setup_label(type) - case type - when 'phone' - t('two_factor_authentication.two_factor_choice_options.phone') - when 'sms' - t('two_factor_authentication.two_factor_choice_options.sms') - when 'voice' - t('two_factor_authentication.two_factor_choice_options.voice') - else - raise "Unsupported setup method: #{type}" - end + raise "Unsupported setup method: #{type}" end def login_info(type) diff --git a/app/presenters/two_factor_authentication/phone_selection_presenter.rb b/app/presenters/two_factor_authentication/set_up_phone_selection_presenter.rb similarity index 61% rename from app/presenters/two_factor_authentication/phone_selection_presenter.rb rename to app/presenters/two_factor_authentication/set_up_phone_selection_presenter.rb index 398fb43d38c..b57bf159a8d 100644 --- a/app/presenters/two_factor_authentication/phone_selection_presenter.rb +++ b/app/presenters/two_factor_authentication/set_up_phone_selection_presenter.rb @@ -1,15 +1,11 @@ module TwoFactorAuthentication - class PhoneSelectionPresenter < SelectionPresenter + class SetUpPhoneSelectionPresenter < SetUpSelectionPresenter def method :phone end - def type - if MfaContext.new(configuration&.user).phone_configurations.many? - "#{super}_#{configuration.id}" - else - super - end + def label + t('two_factor_authentication.two_factor_choice_options.phone') end def info diff --git a/app/presenters/two_factor_authentication/sign_in_phone_selection_presenter.rb b/app/presenters/two_factor_authentication/sign_in_phone_selection_presenter.rb new file mode 100644 index 00000000000..207b9a82e71 --- /dev/null +++ b/app/presenters/two_factor_authentication/sign_in_phone_selection_presenter.rb @@ -0,0 +1,55 @@ +module TwoFactorAuthentication + class SignInPhoneSelectionPresenter < SignInSelectionPresenter + attr_reader :configuration, :user, :method + + def initialize(user:, configuration:, method:) + @user = user + @configuration = configuration + @method = method + end + + def type + if MfaContext.new(configuration&.user).phone_configurations.many? + "#{super}_#{configuration.id}" + else + super + end + end + + def label + if method.to_s == 'sms' + t('two_factor_authentication.login_options.sms') + elsif method.to_s == 'voice' + t('two_factor_authentication.login_options.voice') + end + end + + def info + case method + when :sms + t( + 'two_factor_authentication.login_options.sms_info_html', + phone: configuration.masked_phone, + ) + when :voice + t( + 'two_factor_authentication.login_options.voice_info_html', + phone: configuration.masked_phone, + ) + else + t('two_factor_authentication.two_factor_choice_options.phone_info') + end + end + + def disabled? + case method + when :sms + OutageStatus.new.vendor_outage?(:sms) + when :voice + OutageStatus.new.vendor_outage?(:voice) + else + OutageStatus.new.all_phone_vendor_outage? + end + end + end +end diff --git a/app/presenters/two_factor_authentication/sms_selection_presenter.rb b/app/presenters/two_factor_authentication/sms_selection_presenter.rb deleted file mode 100644 index 3226ae6adc3..00000000000 --- a/app/presenters/two_factor_authentication/sms_selection_presenter.rb +++ /dev/null @@ -1,22 +0,0 @@ -module TwoFactorAuthentication - class SmsSelectionPresenter < PhoneSelectionPresenter - def method - :sms - end - - def info - if configuration.present? - t( - 'two_factor_authentication.login_options.sms_info_html', - phone: configuration.masked_phone, - ) - else - super - end - end - - def disabled? - OutageStatus.new.vendor_outage?(:sms) - end - end -end diff --git a/app/presenters/two_factor_authentication/voice_selection_presenter.rb b/app/presenters/two_factor_authentication/voice_selection_presenter.rb deleted file mode 100644 index 369878c5e5d..00000000000 --- a/app/presenters/two_factor_authentication/voice_selection_presenter.rb +++ /dev/null @@ -1,22 +0,0 @@ -module TwoFactorAuthentication - class VoiceSelectionPresenter < PhoneSelectionPresenter - def method - :voice - end - - def info - if configuration.present? - t( - 'two_factor_authentication.login_options.voice_info_html', - phone: configuration.masked_phone, - ) - else - super - end - end - - def disabled? - OutageStatus.new.vendor_outage?(:voice) - end - end -end diff --git a/app/presenters/two_factor_options_presenter.rb b/app/presenters/two_factor_options_presenter.rb index 7466c3c8f3d..56a4567a328 100644 --- a/app/presenters/two_factor_options_presenter.rb +++ b/app/presenters/two_factor_options_presenter.rb @@ -34,7 +34,7 @@ def all_user_selected_options [ TwoFactorAuthentication::SetUpWebauthnPlatformSelectionPresenter.new(user: user), TwoFactorAuthentication::SetUpAuthAppSelectionPresenter.new(user: user), - TwoFactorAuthentication::PhoneSelectionPresenter.new(user: user), + TwoFactorAuthentication::SetUpPhoneSelectionPresenter.new(user: user), TwoFactorAuthentication::SetUpBackupCodeSelectionPresenter.new(user: user), TwoFactorAuthentication::SetUpWebauthnSelectionPresenter.new(user: user), TwoFactorAuthentication::SetUpPivCacSelectionPresenter.new(user: user), @@ -116,7 +116,7 @@ def phone_options if piv_cac_required? || phishing_resistant_only? || IdentityConfig.store.hide_phone_mfa_signup return [] else - [TwoFactorAuthentication::PhoneSelectionPresenter.new(user: user)] + [TwoFactorAuthentication::SetUpPhoneSelectionPresenter.new(user: user)] end end diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 13a73ae2513..bac52964f9d 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -163,8 +163,6 @@ en: (toll) phone numbers. piv_cac: Government employee ID piv_cac_info: PIV/CAC cards for government and military employees. Desktop only. - sms: Text message / SMS - voice: Phone call webauthn: Security key webauthn_info: A physical device, often shaped like a USB drive, that you plug in to your device. diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 2f079bd6027..c2a30013de9 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -173,8 +173,6 @@ es: piv_cac: Identificación de empleado gubernamental piv_cac_info: Credenciales PIV/CAC para empleados gubernamentales y del ejército. Únicamente versión de escritorio. - sms: Mensaje de texto / SMS - voice: Llamada telefónica webauthn: Clave de seguridad webauthn_info: Un dispositivo físico que suele tener la forma de una unidad USB y se conecta a su dispositivo. diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index ce17a3ce3a3..52dee824d9f 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -182,8 +182,6 @@ fr: piv_cac: Carte d’identification des employés du gouvernement piv_cac_info: Cartes PIV/CAC pour les fonctionnaires et les militaires. Bureau uniquement. - sms: SMS - voice: Appel téléphonique webauthn: Clef de sécurité webauthn_info: Un appareil physique, souvent en forme de clé USB, que vous branchez sur votre appareil. diff --git a/spec/presenters/two_factor_authentication/phone_selection_presenter_spec.rb b/spec/presenters/two_factor_authentication/set_up_phone_selection_presenter_spec.rb similarity index 82% rename from spec/presenters/two_factor_authentication/phone_selection_presenter_spec.rb rename to spec/presenters/two_factor_authentication/set_up_phone_selection_presenter_spec.rb index b22167d42f3..8c9e9137218 100644 --- a/spec/presenters/two_factor_authentication/phone_selection_presenter_spec.rb +++ b/spec/presenters/two_factor_authentication/set_up_phone_selection_presenter_spec.rb @@ -1,15 +1,13 @@ require 'rails_helper' -RSpec.describe TwoFactorAuthentication::PhoneSelectionPresenter do +RSpec.describe TwoFactorAuthentication::SetUpPhoneSelectionPresenter do let(:user_without_mfa) { create(:user) } let(:user_with_mfa) { create(:user, :with_phone) } - let(:presenter_with_mfa) { described_class.new(configuration: phone, user: user_with_mfa) } - let(:presenter_without_mfa) { described_class.new(configuration: phone, user: user_without_mfa) } + let(:presenter_with_mfa) { described_class.new(user: user_with_mfa) } + let(:presenter_without_mfa) { described_class.new(user: user_without_mfa) } describe '#info' do context 'when a user does not have a phone configuration (first time)' do - let(:phone) { nil } - it 'includes a note about choosing voice or sms' do expect(presenter_without_mfa.info). to include(t('two_factor_authentication.two_factor_choice_options.phone_info')) @@ -28,8 +26,6 @@ end describe '#disabled?' do - let(:phone) { build(:phone_configuration, phone: '+1 888 867-5309') } - it { expect(presenter_without_mfa.disabled?).to eq(false) } context 'all phone vendor outage' do @@ -42,7 +38,6 @@ end describe '#mfa_configuration' do - let(:phone) { nil } it 'returns an empty string when user has not configured this authenticator' do expect(presenter_without_mfa.mfa_configuration_description).to eq('') end diff --git a/spec/presenters/two_factor_authentication/sign_in_phone_selection_presenter_spec.rb b/spec/presenters/two_factor_authentication/sign_in_phone_selection_presenter_spec.rb new file mode 100644 index 00000000000..360ab52daf5 --- /dev/null +++ b/spec/presenters/two_factor_authentication/sign_in_phone_selection_presenter_spec.rb @@ -0,0 +1,92 @@ +require 'rails_helper' + +RSpec.describe TwoFactorAuthentication::SignInPhoneSelectionPresenter do + let(:user) { create(:user, :with_phone) } + let(:configuration) { create(:phone_configuration, user: user) } + + let(:presenter) do + described_class.new(user: user, configuration: configuration, method: 'phone') + end + + describe '#type' do + it 'returns phone appended with configuration id' do + expect(presenter.type).to eq "phone_#{configuration.id}" + end + end + + describe '#info' do + it 'raises with missing translation' do + expect(presenter.info).to eq( + t('two_factor_authentication.two_factor_choice_options.phone_info'), + ) + end + context 'with sms method' do + let(:presenter) do + described_class.new(user: user, configuration: configuration, method: :sms) + end + it 'returns the correct translation for sms' do + expect(presenter.info).to eq( + t( + 'two_factor_authentication.login_options.sms_info_html', + phone: configuration.masked_phone, + ), + ) + end + end + context 'with voice method' do + let(:presenter) do + described_class.new(user: user, configuration: configuration, method: :voice) + end + it 'returns the correct translation for voice' do + expect(presenter.info).to eq( + t( + 'two_factor_authentication.login_options.voice_info_html', + phone: configuration.masked_phone, + ), + ) + end + end + end + + describe '#disabled?' do + let(:user_without_mfa) { create(:user) } + let(:phone) { build(:phone_configuration, phone: '+1 888 867-5309') } + let(:presenter_without_mfa) do + described_class.new(configuration: phone, user: user_without_mfa, method: 'phone') + end + it { expect(presenter_without_mfa.disabled?).to eq(false) } + + context 'all phone vendor outage' do + before do + allow_any_instance_of(OutageStatus).to receive(:all_phone_vendor_outage?).and_return(true) + end + + it { expect(presenter_without_mfa.disabled?).to eq(true) } + end + + context 'voice vendor outage' do + let(:presenter_without_mfa) do + described_class.new(configuration: phone, user: user, method: method) + end + let(:method) { :voice } + before do + allow_any_instance_of(OutageStatus).to receive(:vendor_outage?).with(:voice). + and_return(true) + end + + it { expect(presenter_without_mfa.disabled?).to eq(true) } + end + + context 'sms vendor outage' do + let(:presenter_without_mfa) do + described_class.new(configuration: phone, user: user, method: method) + end + let(:method) { :sms } + before do + allow_any_instance_of(OutageStatus).to receive(:vendor_outage?).with(:sms).and_return(true) + end + + it { expect(presenter_without_mfa.disabled?).to eq(true) } + end + end +end diff --git a/spec/presenters/two_factor_authentication/sms_selection_presenter_spec.rb b/spec/presenters/two_factor_authentication/sms_selection_presenter_spec.rb deleted file mode 100644 index 54064ce07d3..00000000000 --- a/spec/presenters/two_factor_authentication/sms_selection_presenter_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'rails_helper' - -RSpec.describe TwoFactorAuthentication::SmsSelectionPresenter do - let(:subject) { described_class.new(configuration: phone, user: user) } - let(:user) { build(:user) } - - describe '#type' do - context 'when a user has only one phone configuration' do - let(:user) { create(:user, :with_phone) } - let(:phone) { MfaContext.new(user).phone_configurations.first } - - it 'returns sms' do - expect(subject.type).to eq 'sms' - end - end - - context 'when a user has more than one phone configuration' do - let(:user) { create(:user, :with_phone) } - let(:phone) do - record = create(:phone_configuration, user: user) - user.reload - record - end - - it 'returns sms:id' do - expect(subject.type).to eq "sms_#{phone.id}" - end - end - end - - describe '#info' do - context 'when a user has a phone configuration' do - let(:phone) { build(:phone_configuration, phone: '+1 888 867-5309') } - it 'includes the masked the number' do - expect(subject.info).to include('(***) ***-5309') - end - end - end - - describe '#disabled?' do - let(:phone) { build(:phone_configuration, phone: '+1 888 867-5309') } - it { expect(subject.disabled?).to eq(false) } - - context 'sms vendor outage' do - before do - allow_any_instance_of(OutageStatus).to receive(:vendor_outage?).with(:sms).and_return(true) - end - - it { expect(subject.disabled?).to eq(true) } - end - end -end diff --git a/spec/presenters/two_factor_authentication/voice_selection_presenter_spec.rb b/spec/presenters/two_factor_authentication/voice_selection_presenter_spec.rb deleted file mode 100644 index edb1d87d6d3..00000000000 --- a/spec/presenters/two_factor_authentication/voice_selection_presenter_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'rails_helper' - -RSpec.describe TwoFactorAuthentication::VoiceSelectionPresenter do - let(:subject) { described_class.new(configuration: phone, user: user) } - let(:user) { build(:user) } - describe '#type' do - context 'when a user has only one phone configuration' do - let(:user) { create(:user, :with_phone) } - let(:phone) { MfaContext.new(user).phone_configurations.first } - - it 'returns voice' do - expect(subject.type).to eq 'voice' - end - end - - context 'when a user has more than one phone configuration' do - let(:user) { create(:user, :with_phone) } - let(:phone) do - record = create(:phone_configuration, user: user) - user.reload - record - end - - it 'returns voice:id' do - expect(subject.type).to eq "voice_#{phone.id}" - end - end - end - - describe '#info' do - context 'when a user has a phone configuration' do - let(:phone) { build(:phone_configuration, phone: '+1 888 867-5309') } - it 'includes the masked the number' do - expect(subject.info).to include('(***) ***-5309') - end - end - end - - describe '#disabled?' do - let(:phone) { build(:phone_configuration, phone: '+1 888 867-5309') } - it { expect(subject.disabled?).to eq(false) } - - context 'voice vendor outage' do - before do - allow_any_instance_of(OutageStatus).to receive(:vendor_outage?).with(:voice). - and_return(true) - end - - it { expect(subject.disabled?).to eq(true) } - end - end -end diff --git a/spec/presenters/two_factor_login_options_presenter_spec.rb b/spec/presenters/two_factor_login_options_presenter_spec.rb index d1ff9ad4dfc..1386d500dd9 100644 --- a/spec/presenters/two_factor_login_options_presenter_spec.rb +++ b/spec/presenters/two_factor_login_options_presenter_spec.rb @@ -82,8 +82,8 @@ it 'returns classes for mfas associated with account' do expect(options_classes).to eq( [ - TwoFactorAuthentication::SmsSelectionPresenter, - TwoFactorAuthentication::VoiceSelectionPresenter, + TwoFactorAuthentication::SignInPhoneSelectionPresenter, + TwoFactorAuthentication::SignInPhoneSelectionPresenter, TwoFactorAuthentication::SignInWebauthnSelectionPresenter, TwoFactorAuthentication::SignInBackupCodeSelectionPresenter, TwoFactorAuthentication::SignInPivCacSelectionPresenter, @@ -114,8 +114,8 @@ it 'returns all mfas associated with account' do expect(options_classes).to eq( [ - TwoFactorAuthentication::SmsSelectionPresenter, - TwoFactorAuthentication::VoiceSelectionPresenter, + TwoFactorAuthentication::SignInPhoneSelectionPresenter, + TwoFactorAuthentication::SignInPhoneSelectionPresenter, TwoFactorAuthentication::SignInWebauthnSelectionPresenter, TwoFactorAuthentication::SignInBackupCodeSelectionPresenter, TwoFactorAuthentication::SignInPivCacSelectionPresenter, @@ -145,8 +145,8 @@ it 'returns all mfas associated with account' do expect(options_classes).to eq( [ - TwoFactorAuthentication::SmsSelectionPresenter, - TwoFactorAuthentication::VoiceSelectionPresenter, + TwoFactorAuthentication::SignInPhoneSelectionPresenter, + TwoFactorAuthentication::SignInPhoneSelectionPresenter, TwoFactorAuthentication::SignInWebauthnSelectionPresenter, TwoFactorAuthentication::SignInBackupCodeSelectionPresenter, TwoFactorAuthentication::SignInPivCacSelectionPresenter, diff --git a/spec/presenters/two_factor_options_presenter_spec.rb b/spec/presenters/two_factor_options_presenter_spec.rb index d644a20b6e4..97233969597 100644 --- a/spec/presenters/two_factor_options_presenter_spec.rb +++ b/spec/presenters/two_factor_options_presenter_spec.rb @@ -27,7 +27,7 @@ expect(presenter.options.map(&:class)).to eq [ TwoFactorAuthentication::SetUpWebauthnPlatformSelectionPresenter, TwoFactorAuthentication::SetUpAuthAppSelectionPresenter, - TwoFactorAuthentication::PhoneSelectionPresenter, + TwoFactorAuthentication::SetUpPhoneSelectionPresenter, TwoFactorAuthentication::SetUpBackupCodeSelectionPresenter, TwoFactorAuthentication::SetUpWebauthnSelectionPresenter, TwoFactorAuthentication::SetUpPivCacSelectionPresenter, @@ -76,7 +76,7 @@ expect(presenter.options.map(&:class)).to eq [ TwoFactorAuthentication::SetUpWebauthnPlatformSelectionPresenter, TwoFactorAuthentication::SetUpAuthAppSelectionPresenter, - TwoFactorAuthentication::PhoneSelectionPresenter, + TwoFactorAuthentication::SetUpPhoneSelectionPresenter, TwoFactorAuthentication::SetUpBackupCodeSelectionPresenter, TwoFactorAuthentication::SetUpWebauthnSelectionPresenter, TwoFactorAuthentication::SetUpPivCacSelectionPresenter,