diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index 3e2db5b07f6..b6d0481fcc1 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -98,10 +98,8 @@ def extra_analytics_attributes def validate_not_voip return if phone.blank? || !IdentityConfig.store.phone_service_check - return unless IdentityConfig.store.voip_block - if phone_info.type == :voip && - !FeatureManagement.voip_allowed_phones.include?(parsed_phone.e164) + if phone_info.type == :voip errors.add(:phone, I18n.t('errors.messages.voip_phone'), type: :voip_phone) elsif phone_info.error errors.add(:phone, I18n.t('errors.messages.voip_check_error'), type: :voip_check_error) diff --git a/app/views/users/phone_setup/index.html.erb b/app/views/users/phone_setup/index.html.erb index 75e28a1cab1..e98536a30a1 100644 --- a/app/views/users/phone_setup/index.html.erb +++ b/app/views/users/phone_setup/index.html.erb @@ -10,9 +10,7 @@

<%= t('two_factor_authentication.phone_fee_disclosure') %> - <% if IdentityConfig.store.voip_block %> - <%= t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip') %> - <% end %> + <%= t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip') %>

<%= simple_form_for( diff --git a/config/application.yml.default b/config/application.yml.default index 2ffdedae096..abef600480b 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -361,8 +361,6 @@ get_usps_proofing_results_job_reprocess_delay_minutes: 5 get_usps_proofing_results_job_request_delay_milliseconds: 1000 voice_otp_pause_time: '0.5s' voice_otp_speech_rate: 'slow' -voip_block: true -voip_allowed_phones: '[]' weekly_auth_funnel_report_config: '[]' development: diff --git a/lib/feature_management.rb b/lib/feature_management.rb index c8c07e8b8f3..9741cb7b37b 100644 --- a/lib/feature_management.rb +++ b/lib/feature_management.rb @@ -119,15 +119,6 @@ def self.recaptcha_enterprise? IdentityConfig.store.recaptcha_enterprise_project_id.present? end - # Manual allowlist for VOIPs, should only include known VOIPs that we use for smoke tests - # @return [Set] set of phone numbers normalized to e164 - def self.voip_allowed_phones - @voip_allowed_phones ||= begin - allowed_phones = IdentityConfig.store.voip_allowed_phones - allowed_phones.map { |p| Phonelib.parse(p).e164 }.to_set - end - end - # Whether we collect device profiling information as part of the proofing process. def self.proofing_device_profiling_collecting_enabled? case IdentityConfig.store.proofing_device_profiling diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 88aab8e842f..83644faf286 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -498,8 +498,6 @@ def self.build_store(config_map) config.add(:version_headers_enabled, type: :boolean) config.add(:voice_otp_pause_time) config.add(:voice_otp_speech_rate) - config.add(:voip_allowed_phones, type: :json) - config.add(:voip_block, type: :boolean) config.add(:weekly_auth_funnel_report_config, type: :json) @key_types = config.key_types diff --git a/spec/controllers/users/phone_setup_controller_spec.rb b/spec/controllers/users/phone_setup_controller_spec.rb index b898b40c7b5..868bd72fc47 100644 --- a/spec/controllers/users/phone_setup_controller_spec.rb +++ b/spec/controllers/users/phone_setup_controller_spec.rb @@ -4,7 +4,6 @@ let(:mfa_selections) { ['voice'] } before do allow(IdentityConfig.store).to receive(:phone_service_check).and_return(true) - allow(IdentityConfig.store).to receive(:voip_block).and_return(true) end describe 'GET index' do diff --git a/spec/features/phone/add_phone_spec.rb b/spec/features/phone/add_phone_spec.rb index b65c8017357..56b0ad11357 100644 --- a/spec/features/phone/add_phone_spec.rb +++ b/spec/features/phone/add_phone_spec.rb @@ -169,7 +169,6 @@ let(:telephony_gem_voip_number) { '+12255551000' } scenario 'adding a VOIP phone' do - allow(IdentityConfig.store).to receive(:voip_block).and_return(true) allow(IdentityConfig.store).to receive(:phone_service_check).and_return(true) user = create(:user, :fully_registered) diff --git a/spec/forms/new_phone_form_spec.rb b/spec/forms/new_phone_form_spec.rb index 5cc867b0282..909cc21078a 100644 --- a/spec/forms/new_phone_form_spec.rb +++ b/spec/forms/new_phone_form_spec.rb @@ -234,11 +234,9 @@ context 'voip numbers' do let(:telephony_gem_voip_number) { '+12255552000' } - let(:voip_block) { false } let(:phone_service_check) { true } before do - allow(IdentityConfig.store).to receive(:voip_block).and_return(voip_block) allow(IdentityConfig.store).to receive(:phone_service_check).and_return(phone_service_check) end @@ -246,71 +244,44 @@ form.submit(params.merge(phone: telephony_gem_voip_number)) end - context 'when voip numbers are blocked' do - let(:voip_block) { true } - - it 'is invalid' do - expect(result.success?).to eq(false) - expect(result.errors[:phone]).to eq([I18n.t('errors.messages.voip_phone')]) - end - - it 'logs the type and carrier' do - expect(result.extra).to include( - phone_type: :voip, - carrier: 'Test VOIP Carrier', - ) - end + it 'voip numbers are invalid' do + expect(result.success?).to eq(false) + expect(result.errors[:phone]).to eq([I18n.t('errors.messages.voip_phone')]) + end - context 'when the number is on the allowlist' do - before do - expect(FeatureManagement).to receive(:voip_allowed_phones). - and_return([telephony_gem_voip_number]) - end + it 'logs the type and carrier' do + expect(result.extra).to include( + phone_type: :voip, + carrier: 'Test VOIP Carrier', + ) + end - it 'is valid' do - expect(result.success?).to eq(true) - expect(result.errors).to be_blank - end + context 'when AWS rate limits info type checks' do + before do + expect(Telephony).to receive(:phone_info). + and_raise(Aws::Pinpoint::Errors::TooManyRequestsException.new(nil, 'error message')) end - context 'when AWS rate limits info type checks' do - before do - expect(Telephony).to receive(:phone_info). - and_raise(Aws::Pinpoint::Errors::TooManyRequestsException.new(nil, 'error message')) - end - - it 'logs a warning and fails open' do - expect(result.extra[:warn]).to include('AWS pinpoint phone info rate limit') + it 'logs a warning and fails open' do + expect(result.extra[:warn]).to include('AWS pinpoint phone info rate limit') - expect(result.success?).to eq(true) - expect(result.errors).to be_blank - end + expect(result.success?).to eq(true) + expect(result.errors).to be_blank end + end - context 'when voip checks are disabled' do - let(:phone_service_check) { false } - - it 'does not check the phone type' do - expect(Telephony).to_not receive(:phone_info) + context 'when voip checks are disabled' do + let(:phone_service_check) { false } - result - end + it 'does not check the phone type' do + expect(Telephony).to_not receive(:phone_info) - it 'allows voip numbers since it cannot check the type' do - expect(result.success?).to eq(true) - expect(result.errors).to be_blank - end + result end - end - - context 'when voip numbers are allowed' do - let(:voip_block) { false } - - it 'does a voip check but does not enforce it' do - expect(Telephony).to receive(:phone_info).and_call_original + it 'allows voip numbers since it cannot check the type' do expect(result.success?).to eq(true) - expect(result.to_h).to include(phone_type: :voip) + expect(result.errors).to be_blank end end diff --git a/spec/lib/feature_management_spec.rb b/spec/lib/feature_management_spec.rb index 3b329dd91f0..d6e64c21958 100644 --- a/spec/lib/feature_management_spec.rb +++ b/spec/lib/feature_management_spec.rb @@ -350,20 +350,6 @@ end end - describe '.voip_allowed_phones' do - before do - # clear memoization - FeatureManagement.instance_variable_set(:@voip_allowed_phones, nil) - end - - it 'normalizes phone numbers and put them in a set' do - voip_allowed_phones = ['18885551234', '+18888675309'] - - expect(IdentityConfig.store).to receive(:voip_allowed_phones).and_return(voip_allowed_phones) - expect(FeatureManagement.voip_allowed_phones).to eq(Set['+18885551234', '+18888675309']) - end - end - describe '#proofing_device_profiling_collecting_enabled?' do it 'returns false for disabled' do expect(IdentityConfig.store).to receive(:proofing_device_profiling).and_return(:disabled) diff --git a/spec/presenters/two_factor_authentication/set_up_phone_selection_presenter_spec.rb b/spec/presenters/two_factor_authentication/set_up_phone_selection_presenter_spec.rb index 8c9e9137218..7d0c1d6a4e4 100644 --- a/spec/presenters/two_factor_authentication/set_up_phone_selection_presenter_spec.rb +++ b/spec/presenters/two_factor_authentication/set_up_phone_selection_presenter_spec.rb @@ -16,12 +16,6 @@ it 'does not include a masked number' do expect(presenter_without_mfa.info).to_not include('***') end - - context 'when VOIP numbers are blocked' do - before do - allow(IdentityConfig.store).to receive(:voip_block).and_return(true) - end - end end end diff --git a/spec/views/phone_setup/index.html.erb_spec.rb b/spec/views/phone_setup/index.html.erb_spec.rb index 9fc2896333c..5c790d9d34b 100644 --- a/spec/views/phone_setup/index.html.erb_spec.rb +++ b/spec/views/phone_setup/index.html.erb_spec.rb @@ -28,28 +28,10 @@ end context 'voip numbers' do - before do - allow(IdentityConfig.store).to receive(:voip_block).and_return(voip_block) - end - - context 'when voip numbers are allowed' do - let(:voip_block) { false } - - it 'does not mention voip' do - expect(render).to_not have_content( - t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip'), - ) - end - end - - context 'when voip numbers are blocked' do - let(:voip_block) { true } - - it 'tells users to not use VOIP numbers' do - expect(render).to have_content( - t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip'), - ) - end + it 'tells users to not use VOIP numbers' do + expect(render).to have_content( + t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip'), + ) end end