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
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
'app/radio-btn',
'app/print-personal-key',
'app/utils/ms-formatter',
'app/phone-internationalization',
],
}
}
61 changes: 61 additions & 0 deletions app/assets/javascripts/app/phone-internationalization.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { PhoneFormatter } from 'field-kit';

const I18n = window.LoginGov.I18n;
const phoneFormatter = new PhoneFormatter();

const getPhoneUnsupportedAreaCodeCountry = (areaCode) => {
const form = document.querySelector('#new_two_factor_setup_form');
const phoneUnsupportedAreaCodes = JSON.parse(form.dataset.unsupportedAreaCodes);
return phoneUnsupportedAreaCodes[areaCode];
};

const areaCodeFromUSPhone = (phone) => {
const digits = phoneFormatter.digitsWithoutCountryCode(phone);
if (digits.length >= 10) {
return digits.slice(0, 3);
}
return null;
};

const unsupportedPhoneOTPDeliveryWarningMessage = (phone) => {
const areaCode = areaCodeFromUSPhone(phone);
const country = getPhoneUnsupportedAreaCodeCountry(areaCode);
if (country) {
const messageTemplate = I18n.t('devise.two_factor_authentication.otp_delivery_preference.phone_unsupported');
return messageTemplate.replace('%{location}', country);
}
return null;
};

const updateOTPDeliveryMethods = () => {
const phoneInput = document.querySelector('#two_factor_setup_form_phone');
const phoneRadio = document.querySelector('#two_factor_setup_form_otp_delivery_preference_voice');
const smsRadio = document.querySelector('#two_factor_setup_form_otp_delivery_preference_sms');
const phoneLabel = phoneRadio.parentNode.parentNode;
const deliveryMethodHint = document.querySelector('#otp_delivery_preference_instruction');
const optPhoneLabelInfo = document.querySelector('#otp_phone_label_info');

const phone = phoneInput.value;

const warningMessage = unsupportedPhoneOTPDeliveryWarningMessage(phone);
if (warningMessage) {
phoneRadio.disabled = true;
phoneLabel.classList.add('btn-disabled');
smsRadio.click();
deliveryMethodHint.innerText = warningMessage;
optPhoneLabelInfo.innerText = I18n.t('devise.two_factor_authentication.otp_phone_label_info_modile_only');
} else {
phoneRadio.disabled = false;
phoneLabel.classList.remove('btn-disabled');
deliveryMethodHint.innerText = I18n.t('devise.two_factor_authentication.otp_delivery_preference.instruction');
optPhoneLabelInfo.innerText = I18n.t('devise.two_factor_authentication.otp_phone_label_info');
}
};

document.addEventListener('DOMContentLoaded', () => {
const input = document.querySelector('#two_factor_setup_form_phone');
if (input) {
input.addEventListener('keyup', updateOTPDeliveryMethods);
updateOTPDeliveryMethods();
}
});
1 change: 1 addition & 0 deletions app/assets/javascripts/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ import 'app/form-validation';
import 'app/form-field-format';
import 'app/idv-finance-helper';
import 'app/radio-btn';
import 'app/phone-internationalization';
import 'app/print-personal-key';
4 changes: 4 additions & 0 deletions app/assets/javascripts/misc/i18n-strings.js.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
window.LoginGov = window.LoginGov || {};

<% keys = [
'devise.two_factor_authentication.otp_delivery_preference.instruction',
'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported',
'devise.two_factor_authentication.otp_phone_label_info',
'devise.two_factor_authentication.otp_phone_label_info_modile_only',
'errors.messages.format_mismatch',
'errors.messages.missing_field',
'forms.passwords.show',
Expand Down
6 changes: 6 additions & 0 deletions app/assets/stylesheets/components/_btn.scss
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,9 @@
outline: none;
}
}

.btn-disabled {
background-color: $gray-light;
border-color: $gray;
color: $gray;
}
5 changes: 5 additions & 0 deletions app/assets/stylesheets/components/_form.scss
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ input::-webkit-inner-spin-button {
background-image: url(data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4NCjwhLS0gR2VuZXJhdG9yOiBBZG9iZSBJbGx1c3RyYXRvciAxNy4xLjAsIFNWRyBFeHBvcnQgUGx1Zy1JbiAuIFNWRyBWZXJzaW9uOiA2LjAwIEJ1aWxkIDApICAtLT4NCjwhRE9DVFlQRSBzdmcgUFVCTElDICItLy9XM0MvL0RURCBTVkcgMS4xLy9FTiIgImh0dHA6Ly93d3cudzMub3JnL0dyYXBoaWNzL1NWRy8xLjEvRFREL3N2ZzExLmR0ZCI+DQo8c3ZnIHZlcnNpb249IjEuMSIgaWQ9IkxheWVyXzEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHg9IjBweCIgeT0iMHB4Ig0KCSB2aWV3Qm94PSIwIDAgOCA4IiBlbmFibGUtYmFja2dyb3VuZD0ibmV3IDAgMCA4IDgiIHhtbDpzcGFjZT0icHJlc2VydmUiPg0KPHBhdGggZmlsbD0iI0ZGRkZGRiIgZD0iTTQsMUMyLjMsMSwxLDIuMywxLDRzMS4zLDMsMywzczMtMS4zLDMtM1M1LjcsMSw0LDF6Ii8+DQo8L3N2Zz4NCg==);
}

.radio input:disabled ~ .indicator {
background-color: $gray-light;
border-color: $gray;
}

.select-alt {
color: $white;
display: inline-block;
Expand Down
12 changes: 11 additions & 1 deletion app/controllers/concerns/phone_confirmation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,17 @@ def prompt_to_confirm_phone(phone:, context: 'confirmation')
user_session[:context] = context

redirect_to otp_send_path(
otp_delivery_selection_form: { otp_delivery_preference: current_user.otp_delivery_preference }
otp_delivery_selection_form: { otp_delivery_preference: otp_delivery_method(phone) }
)
end

private

def otp_delivery_method(phone)
if PhoneNumberCapabilities.new(phone).sms_only?
:sms
else
current_user.otp_delivery_preference
end
end
end
10 changes: 10 additions & 0 deletions app/controllers/concerns/two_factor_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ def phone_view_data
phone_number: display_phone_to_deliver_to,
code_value: direct_otp_code,
otp_delivery_preference: two_factor_authentication_method,
voice_otp_delivery_unsupported: voice_otp_delivery_unsupported?,
reenter_phone_number_path: reenter_phone_number_path,
unconfirmed_phone: unconfirmed_phone?,
totp_enabled: current_user.totp_enabled?,
Expand Down Expand Up @@ -265,6 +266,15 @@ def display_phone_to_deliver_to
end
end

def voice_otp_delivery_unsupported?
phone_number = if authentication_context?
current_user.phone
else
user_session[:unconfirmed_phone]
end
PhoneNumberCapabilities.new(phone_number).sms_only?
end

def decorated_user
current_user.decorate
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class TwoFactorAuthenticationSetupController < ApplicationController

def index
@two_factor_setup_form = TwoFactorSetupForm.new(current_user)
@unsupported_area_codes = PhoneNumberCapabilities::VOICE_UNSUPPORTED_US_AREA_CODES
analytics.track_event(Analytics::USER_REGISTRATION_PHONE_SETUP_VISIT)
end

Expand Down
1 change: 1 addition & 0 deletions app/forms/two_factor_setup_form.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class TwoFactorSetupForm
include ActiveModel::Model
include FormPhoneValidator
include OtpDeliveryPreferenceValidator

attr_accessor :phone

Expand Down
21 changes: 14 additions & 7 deletions app/presenters/two_factor_auth_code/phone_delivery_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def cancel_link
:phone_number,
:unconfirmed_phone,
:otp_delivery_preference,
:voice_otp_delivery_unsupported,
:confirmation_for_phone_change
)

Expand All @@ -42,7 +43,19 @@ def phone_number_tag
end

def otp_fallback_options
safe_join([phone_fallback_link, auth_app_fallback_link])
if totp_enabled
otp_fallback_options_with_totp
elsif !voice_otp_delivery_unsupported
safe_join([phone_fallback_link, '.'])
end
end

def otp_fallback_options_with_totp
if voice_otp_delivery_unsupported
safe_join([auth_app_fallback_tag, '.'])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Climate says this line is not tested.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, this presenter has basically no tests. I can stick some in there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@monfresh: Added tests for the parts I changed.

else
safe_join([phone_fallback_link, auth_app_fallback_link])
end
end

def update_phone_link
Expand All @@ -64,15 +77,9 @@ def phone_link_tag
end

def auth_app_fallback_link
return empty unless totp_enabled

t('links.phone_confirmation.auth_app_fallback_html', link: auth_app_fallback_tag)
end

def empty
'.'
end

def auth_app_fallback_tag
view.link_to(
t('links.two_factor_authentication.app'),
Expand Down
28 changes: 28 additions & 0 deletions app/services/phone_number_capabilities.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
class PhoneNumberCapabilities
VOICE_UNSUPPORTED_US_AREA_CODES = {
'648' => 'American Samoa',
'671' => 'Guam',
'670' => 'Northern Mariana Islands',
'340' => 'United States Virgin Islands',
}.freeze

attr_reader :phone

def initialize(phone)
@phone = phone
end

def sms_only?
VOICE_UNSUPPORTED_US_AREA_CODES[area_code].present?
end

def unsupported_location
VOICE_UNSUPPORTED_US_AREA_CODES[area_code]
end

private

def area_code
@area_code ||= Phony.split(Phony.normalize(phone, cc: '1')).second
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think using Phonelib here is clearer, like we do here? https://github.com/18F/identity-idp/blob/master/app/forms/otp_delivery_selection_form.rb#L37-L48

Phonelib.parse(phone).area_code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fancy 👔. So many phone gems.....

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, so this started breaking the spec for the IdV flow. The problem is that at this point the phone number has been formatted for the vendor and no longer includes the country code. It appears to still work for US numbers, but gets confused for numbers with a US country code.

Using Phony to normalize it to add the +1 works, so I may do that.

Related: We may want to make sure we only accept US phone numbers for IdV in #1544

end
end
20 changes: 20 additions & 0 deletions app/validators/otp_delivery_preference_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module OtpDeliveryPreferenceValidator
extend ActiveSupport::Concern

included do
validate :otp_delivery_preference_supported
end

def otp_delivery_preference_supported
capabilities = PhoneNumberCapabilities.new(phone)
return unless otp_delivery_preference == 'voice' && capabilities.sms_only?

errors.add(
:phone,
I18n.t(
'devise.two_factor_authentication.otp_delivery_preference.phone_unsupported',
location: capabilities.unsupported_location
)
)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ p.mt-tiny.mb0
= t('devise.two_factor_authentication.otp_setup_html')
= simple_form_for(@two_factor_setup_form,
html: { autocomplete: 'off', role: 'form' },
data: { unsupported_area_codes: @unsupported_area_codes },
method: :patch,
url: phone_setup_path) do |f|
= f.label :phone, class: 'block'
strong.left = t('devise.two_factor_authentication.otp_phone_label')
span.ml1.italic = t('devise.two_factor_authentication.otp_phone_label_info')
span#otp_phone_label_info.ml1.italic
= t('devise.two_factor_authentication.otp_phone_label_info')
= f.input :phone, as: :tel, label: false, required: true,
input_html: { class: 'phone sm-col-8 mb4' }
.mb3
Expand All @@ -25,7 +27,8 @@ p.mt-tiny.mb0
= radio_button_tag 'two_factor_setup_form[otp_delivery_preference]', :voice
span.indicator
= t('devise.two_factor_authentication.otp_delivery_preference.voice')
p.mt1.mb0 = t('devise.two_factor_authentication.otp_delivery_preference.instruction')
p#otp_delivery_preference_instruction.mt1.mb0
= t('devise.two_factor_authentication.otp_delivery_preference.instruction')
= f.button :submit, t('forms.buttons.send_security_code')

= render 'shared/cancel', link: destroy_user_session_path
3 changes: 3 additions & 0 deletions config/locales/devise/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,14 @@ en:
because you have entered the personal key incorrectly too many times.
otp_delivery_preference:
instruction: You can change your choice the next time you sign in
phone_unsupported: We're unable to make phone calls to people in %{location}
at this time. You will receive your security code via text message
sms: Text message (SMS)
title: How would you like to receive your security code?
voice: Phone call
otp_phone_label: Phone number
otp_phone_label_info: Mobile or landline okay
otp_phone_label_info_modile_only: Mobile only
otp_setup_html: "<strong>Every time you log in,</strong> we will send you a
one-time security code via text message or phone call. This helps safeguard
your account."
Expand Down
2 changes: 2 additions & 0 deletions config/locales/devise/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,13 @@ es:
porque ha ingresado incorrectamente la clave personal demasiadas veces.
otp_delivery_preference:
instruction: Puede cambiar su elección la próxima vez que inicie una sesión.
phone_unsupported: NOT TRANSLATED YET
sms: Mensaje de texto (SMS, sigla en inglés)
title: "¿Cómo le gustaría recibir su código de seguridad?"
voice: Llamada telefónica
otp_phone_label: Número de teléfono
otp_phone_label_info: El móvil o teléfono fijo está bien.
otp_phone_label_info_modile_only: NOT TRANSLATED YET
otp_setup_html: "<strong>Cada vez que inicie una sesión,</ strong> le enviaremos
un código de seguridad de sólo un uso por mensaje de texto o llamada telefónica.
Esto ayuda a proteger su cuenta."
Expand Down
2 changes: 2 additions & 0 deletions config/locales/devise/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,13 @@ fr:
otp_delivery_preference:
instruction: Vous pourrez changer votre choix la prochaine fois que vous vous
connecterez
phone_unsupported: NOT TRANSLATED YET
sms: Message texte (SMS)
title: Comment souhaitez-vous recevoir votre code de sécurité?
voice: Appel téléphonique
otp_phone_label: Numéro de téléphone
otp_phone_label_info: Cellulaire ou ligne fixe est acceptable
otp_phone_label_info_modile_only: NOT TRANSLATED YET
otp_setup_html: "<strong>Chaque fois que vous vous connecterez,</strong> nous
vous enverrons un code de sécurité à utilisation unique par message texte
ou par appel téléphonique. Cela aide à protéger votre compte."
Expand Down
22 changes: 22 additions & 0 deletions spec/features/idv/phone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,28 @@

expect(current_path).to eq account_path
end

scenario 'phone number with no voice otp support only allows sms delivery' do
guam_phone = '671-555-5000'
user = create(
:user, :signed_up,
otp_delivery_preference: 'voice',
password: Features::SessionHelper::VALID_PASSWORD
)

sign_in_and_2fa_user(user)
visit verify_session_path

allow(VoiceOtpSenderJob).to receive(:perform_later)
allow(SmsOtpSenderJob).to receive(:perform_later)

complete_idv_profile_with_phone(guam_phone)

expect(current_path).to eq login_two_factor_path(otp_delivery_preference: :sms)
expect(VoiceOtpSenderJob).to_not have_received(:perform_later)
expect(SmsOtpSenderJob).to have_received(:perform_later)
expect(page).to_not have_content(t('links.two_factor_authentication.resend_code.phone'))
end
end

scenario 'phone field only allows numbers', js: true, idv_job: true do
Expand Down
Loading