Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ gem 'identity-doc-auth', github: '18F/identity-doc-auth', tag: 'v0.3.3'
gem 'identity-hostdata', github: '18F/identity-hostdata', tag: 'v0.4.3'
require File.join(__dir__, 'lib', 'lambda_jobs', 'git_ref.rb')
gem 'identity-idp-functions', github: '18F/identity-idp-functions', ref: LambdaJobs::GIT_REF
gem 'identity-telephony', github: '18f/identity-telephony', tag: 'v0.1.8'
gem 'identity-telephony', github: '18f/identity-telephony', tag: 'v0.1.11'
gem 'identity_validations', github: '18F/identity-validations', branch: 'main'
gem 'json-jwt', '>= 1.11.0'
gem 'jwt'
Expand Down
8 changes: 4 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ GIT

GIT
remote: https://github.com/18f/identity-telephony.git
revision: 3dd8a3e91c58565aa4188ac1ec7622c2665d04ae
tag: v0.1.8
revision: 8e56cce97f706679709a0007fd2904723f1f3cbc
tag: v0.1.11
specs:
identity-telephony (0.1.8)
identity-telephony (0.1.11)
aws-sdk-pinpoint
aws-sdk-pinpointsmsvoice
i18n
Expand Down Expand Up @@ -175,7 +175,7 @@ GEM
aws-sdk-lambda (1.57.0)
aws-sdk-core (~> 3, >= 3.109.0)
aws-sigv4 (~> 1.1)
aws-sdk-pinpoint (1.47.0)
aws-sdk-pinpoint (1.48.0)
aws-sdk-core (~> 3, >= 3.109.0)
aws-sigv4 (~> 1.1)
aws-sdk-pinpointsmsvoice (1.21.0)
Expand Down
23 changes: 23 additions & 0 deletions app/forms/new_phone_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ class NewPhoneForm

validates :otp_delivery_preference, inclusion: { in: %w[voice sms] }

validate :validate_not_voip

attr_accessor :phone, :international_code, :otp_delivery_preference,
:otp_make_default_number

Expand Down Expand Up @@ -64,9 +66,30 @@ def ingest_phone_number(params)
def extra_analytics_attributes
{
otp_delivery_preference: otp_delivery_preference,
phone_type: @phone_info&.type,
carrier: @phone_info&.carrier,
country_code: parsed_phone.country,
area_code: parsed_phone.area_code,
}
end

def validate_not_voip
return if phone.blank? || !FeatureManagement.voip_block?

@phone_info = Telephony.phone_info(phone)

if @phone_info.type == :voip &&
!FeatureManagement.voip_allowed_phones.include?(parsed_phone.e164)
errors.add(:phone, I18n.t('errors.messages.voip_check_error'))
elsif @phone_info.error
errors.add(:phone, I18n.t('errors.messages.voip_phone'))
end
end

def parsed_phone
@parsed_phone ||= Phonelib.parse(phone)
end

def ingest_submitted_params(params)
ingest_phone_number(params)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,16 @@ def info
phone: masked_number(configuration.phone),
)
else
t("two_factor_authentication.#{option_mode}.phone_info_html")
voip_note = if FeatureManagement.voip_block?
t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip')
end

safe_join([t("two_factor_authentication.#{option_mode}.phone_info_html"), *voip_note], ' ')
end
end

def security_level
I18n.t('two_factor_authentication.two_factor_choice_options.less_secure_label')
t('two_factor_authentication.two_factor_choice_options.less_secure_label')
end

private
Expand Down
3 changes: 3 additions & 0 deletions app/views/users/phone_setup/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
</p>
<p class="mt-tiny mb1">
<%= t('two_factor_authentication.phone_fee_disclosure') %>
<% if FeatureManagement.voip_block? %>
<%= t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip') %>
<% end %>
</p>

<%= validated_form_for(@new_phone_form,
Expand Down
3 changes: 3 additions & 0 deletions app/views/users/phones/add.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
</p>
<p class="mt-tiny mb1">
<%= t('two_factor_authentication.phone_fee_disclosure') %>
<% if FeatureManagement.voip_block? %>
<%= t('two_factor_authentication.two_factor_choice_options.phone_info_no_voip') %>
<% end %>
</p>

<%= validated_form_for(@new_phone_form,
Expand Down
2 changes: 2 additions & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ usps_ipp_password: ''
usps_ipp_root_url: ''
usps_ipp_sponsor_id: ''
usps_ipp_username: ''
voip_block: 'false'
voip_allowed_phones: '[]'

development:
aal_authn_context_enabled: 'true'
Expand Down
3 changes: 3 additions & 0 deletions config/locales/errors/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ en:
unauthorized_service_provider: Unauthorized Service Provider
usps_otp_expired: Your confirmation code has expired. Please request another
letter for a new code.
voip_check_error: There was an error checking your phone, please try again
voip_phone: This number is a web-based (VOIP) phone service. Please select a
different number and try again
weak_password: Your password is not strong enough. %{feedback}
piv_cac_setup:
unique_name: That name is already taken. Please choose a different name.
Expand Down
4 changes: 4 additions & 0 deletions config/locales/errors/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ es:
unauthorized_service_provider: Proveedor de servicio no autorizado
usps_otp_expired: Tu código de confirmación ha caducado. Vuelve a solicitar
otra carta para recibir un nuevo código.
voip_check_error: Se ha producido un error al comprobar su teléfono, por favor,
inténtelo de nuevo
voip_phone: Este número corresponde a un servicio de telefonía basado en la
web (VoIP). Por favor, seleccione un número diferente e inténtelo de nuevo.
weak_password: Su contraseña no es suficientemente segura. %{feedback}
piv_cac_setup:
unique_name: El nombre ya fue escogido. Por favor, elija un nombre diferente.
Expand Down
4 changes: 4 additions & 0 deletions config/locales/errors/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ fr:
unauthorized_service_provider: Fournisseur de service non autorisé
usps_otp_expired: Votre code de confirmation a expiré. Veuillez solliciter une
autre lettre afin d'obtenir un nouveau code.
voip_check_error: Il y a eu une erreur lors de la vérification de votre téléphone,
veuillez réessayer
voip_phone: Ce numéro est un service téléphonique basé sur le Web (Voix sur
IP). Veuillez sélectionner un autre numéro et réessayer
weak_password: Votre mot de passe n'est pas assez fort. %{feedback}
piv_cac_setup:
unique_name: Ce nom est déjà pris. Veuillez choisir un autre nom.
Expand Down
1 change: 1 addition & 0 deletions config/locales/two_factor_authentication/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ en:
more_secure_label: More Secure
phone: Phone
phone_info_html: Get security codes by text message (SMS) or phone call.
phone_info_no_voip: Please do not use web-based (VOIP) phone services.
piv_cac: Government employee ID
piv_cac_info_html: Insert your government or military PIV or CAC card and enter
your PIN.
Expand Down
1 change: 1 addition & 0 deletions config/locales/two_factor_authentication/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ es:
phone: Teléfono
phone_info_html: Obtenga códigos de seguridad por mensaje de texto (SMS) o llamada
telefónica.
phone_info_no_voip: No utilice servicios telefónicos basados en web (VOIP).
piv_cac: Identificación de empleado del gobierno
piv_cac_info_html: Inserte su tarjeta gubernamental o militar PIV o CAC e ingrese
su PIN.
Expand Down
2 changes: 2 additions & 0 deletions config/locales/two_factor_authentication/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ fr:
more_secure_label: Plus de protection
phone: Téléphone
phone_info_html: Obtenir les codes de sécurité par SMS ou appel téléphonique.
phone_info_no_voip: Veuillez ne pas utiliser les services téléphoniques basés
sur le Web (VOIP).
piv_cac: Numéro d'employé du gouvernement
piv_cac_info_html: Insérez votre carte PIV ou CAC du gouvernement ou militaire
et entrez votre code PIN.
Expand Down
15 changes: 15 additions & 0 deletions lib/feature_management.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,19 @@ def self.logo_upload_enabled?
def self.log_to_stdout?
!Rails.env.test? && AppConfig.env.log_to_stdout == 'true'
end

# Whether or not we should block VOIP phone numbers
def self.voip_block?
AppConfig.env.voip_block == 'true'
end

# Manual allowlist for VOIPs, should only include known VOIPs that we use for smoke tests
# @return [Set<String>] set of phone numbers normalized to e164
def self.voip_allowed_phones
@voip_allowed_phones ||= if (allowed_phones = AppConfig.env.voip_allowed_phones).present?
JSON.parse(allowed_phones).map { |p| Phonelib.parse(p).e164 }.to_set
else
Set.new
end
end
end
18 changes: 18 additions & 0 deletions spec/controllers/users/phone_setup_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
require 'rails_helper'

describe Users::PhoneSetupController do
before { allow(FeatureManagement).to receive(:voip_block?).and_return(true) }

describe 'GET index' do
context 'when signed out' do
it 'redirects to sign in page' do
Expand Down Expand Up @@ -43,6 +45,10 @@
],
},
otp_delivery_preference: 'sms',
area_code: nil,
carrier: 'Test Mobile Carrier',
country_code: nil,
phone_type: :mobile,
}

expect(@analytics).to receive(:track_event).
Expand All @@ -69,6 +75,10 @@
success: true,
errors: {},
otp_delivery_preference: 'voice',
area_code: '703',
carrier: 'Test Mobile Carrier',
country_code: 'US',
phone_type: :mobile,
}

expect(@analytics).to receive(:track_event).
Expand Down Expand Up @@ -103,6 +113,10 @@
success: true,
errors: {},
otp_delivery_preference: 'sms',
area_code: '703',
carrier: 'Test Mobile Carrier',
country_code: 'US',
phone_type: :mobile,
}

expect(@analytics).to receive(:track_event).
Expand Down Expand Up @@ -136,6 +150,10 @@
success: true,
errors: {},
otp_delivery_preference: 'sms',
area_code: '703',
carrier: 'Test Mobile Carrier',
country_code: 'US',
phone_type: :mobile,
}

expect(@analytics).to receive(:track_event).
Expand Down
14 changes: 14 additions & 0 deletions spec/features/phone/add_phone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@
expect(user.reload.phone_configurations.count).to eq(1)
end

let(:telephony_gem_voip_number) { '+12255551000' }

scenario 'adding a VOIP phone' do
allow(FeatureManagement).to receive(:voip_block?).and_return(true)

user = create(:user, :signed_up)

sign_in_and_2fa_user(user)
click_on "+ #{t('account.index.phone_add')}"
fill_in :new_phone_form_phone, with: telephony_gem_voip_number
click_continue
expect(page).to have_content(t('errors.messages.voip_phone'))
end

context 'when the user does not have a phone' do
scenario 'cancelling add phone otp confirmation redirect to account' do
user = create(:user, :with_authentication_app)
Expand Down
54 changes: 52 additions & 2 deletions spec/forms/new_phone_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
otp_delivery_preference: 'sms',
}
end
subject { NewPhoneForm.new(user) }
subject(:form) { NewPhoneForm.new(user) }

it_behaves_like 'a phone form'

Expand Down Expand Up @@ -45,7 +45,7 @@
it 'includes otp preference in the form response extra' do
result = subject.submit(params)

expect(result.extra).to eq(
expect(result.extra).to include(
otp_delivery_preference: params[:otp_delivery_preference],
)
end
Expand Down Expand Up @@ -150,5 +150,55 @@
expect(result.success?).to eq(true)
expect(result.errors).to be_empty
end

context 'voip numbers' do
let(:telephony_gem_voip_number) { '+12255552000' }

subject(:result) do
form.submit(params.merge(phone: telephony_gem_voip_number))
end

context 'when voip numbers are blocked' do
before do
expect(FeatureManagement).to receive(:voip_block?).and_return(true)
end

it 'is invalid' do
expect(result.success?).to eq(false)
expect(result.errors[:phone]).to eq([I18n.t('errors.messages.voip_check_error')])
end

it 'logs the type and carrier' do
expect(result.extra).to include(
phone_type: :voip,
carrier: 'Test VOIP Carrier',
)
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 'is valid' do
expect(result.success?).to eq(true)
expect(result.errors).to be_blank
end
end
end

context 'when voip numbers are allowed' do
before do
expect(FeatureManagement).to receive(:voip_block?).and_return(false)
end

it 'allows voip numbers, does not even make a voip check' do
expect(Telephony).to_not receive(:phone_info)

expect(result.success?).to eq(true)
end
end
end
end
end
26 changes: 26 additions & 0 deletions spec/lib/feature_management_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -434,4 +434,30 @@
end
end
end

describe '.voip_allowed_phones' do
before do
# clear memoization
FeatureManagement.instance_variable_set(:@voip_allowed_phones, nil)

expect(AppConfig.env).to receive(:voip_allowed_phones).and_return(voip_allowed_phones)
end

context 'with a nil or missing config' do
let(:voip_allowed_phones) { nil }

it 'is the empty set' do
expect(FeatureManagement.voip_allowed_phones).to eq(Set.new)
end
end

context 'when there are phone numbers in the list' do
let(:voip_allowed_phones) { '["18885551234", "+18888675309"]' }

it 'normalizes phone numbers and put them in a set' do
expect(FeatureManagement.voip_allowed_phones).
to eq(Set['+18885551234', '+18888675309'])
end
end
end
end
Loading