From 4667492a42ceecc013fdfa051beb30bebbbb5383 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Mon, 5 Nov 2018 13:03:17 -0600 Subject: [PATCH 01/39] LG-767 Alert a user on personal key sign in (#2630) **Why**: So that users will know when their personal key is being used to sign in and be aware of any suspicious activity around their personal key. --- app/forms/personal_key_form.rb | 10 +++++++ .../sms_personal_key_sign_in_notifier_job.rb | 14 +++++++++ app/mailers/user_mailer.rb | 4 +++ .../personal_key_sign_in.html.slim | 16 ++++++++++ config/locales/jobs/en.yml | 3 ++ config/locales/jobs/es.yml | 3 ++ config/locales/jobs/fr.yml | 4 +++ config/locales/user_mailer/en.yml | 18 +++++++++++ config/locales/user_mailer/es.yml | 20 +++++++++++++ config/locales/user_mailer/fr.yml | 21 +++++++++++++ .../sign_in_via_personal_key_spec.rb | 10 +++++-- spec/features/users/sign_in_spec.rb | 1 + spec/forms/personal_key_form_spec.rb | 20 +++++++++++++ .../sms_account_reset_notifier_job_spec.rb | 2 -- ..._personal_key_sign_in_notifier_job_spec.rb | 30 +++++++++++++++++++ spec/mailers/user_mailer_spec.rb | 20 +++++++++++++ spec/support/shared_examples/sign_in.rb | 1 + 17 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 app/jobs/sms_personal_key_sign_in_notifier_job.rb create mode 100644 app/views/user_mailer/personal_key_sign_in.html.slim create mode 100644 spec/jobs/sms_personal_key_sign_in_notifier_job_spec.rb diff --git a/app/forms/personal_key_form.rb b/app/forms/personal_key_form.rb index d4d57d234c5..a037559f609 100644 --- a/app/forms/personal_key_form.rb +++ b/app/forms/personal_key_form.rb @@ -15,6 +15,7 @@ def submit @success = valid? reset_sensitive_fields unless success + send_personal_key_sign_in_notification if success FormResponse.new(success: success, errors: errors.messages, extra: extra_analytics_attributes) end @@ -32,4 +33,13 @@ def extra_analytics_attributes def reset_sensitive_fields self.personal_key = nil end + + def send_personal_key_sign_in_notification + user.confirmed_email_addresses.each do |email_address| + UserMailer.personal_key_sign_in(email_address.email).deliver_now + end + MfaContext.new(user).phone_configurations.each do |phone_configuration| + SmsPersonalKeySignInNotifierJob.perform_now(phone: phone_configuration.phone) + end + end end diff --git a/app/jobs/sms_personal_key_sign_in_notifier_job.rb b/app/jobs/sms_personal_key_sign_in_notifier_job.rb new file mode 100644 index 00000000000..0e26f02d7ae --- /dev/null +++ b/app/jobs/sms_personal_key_sign_in_notifier_job.rb @@ -0,0 +1,14 @@ +class SmsPersonalKeySignInNotifierJob < ApplicationJob + queue_as :sms + + # :reek:UtilityFunction + def perform(phone:) + TwilioService::Utils.new.send_sms( + to: phone, + body: I18n.t( + 'jobs.sms_personal_key_sign_in_notifier_job.message', + app: APP_NAME + ) + ) + end +end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 8f8fd516642..dcf0044e6a0 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -27,6 +27,10 @@ def account_does_not_exist(email, request_id) mail(to: email, subject: t('user_mailer.account_does_not_exist.subject')) end + def personal_key_sign_in(email) + mail(to: email, subject: t('user_mailer.personal_key_sign_in.subject')) + end + def account_reset_request(email_address, account_reset) @token = account_reset&.request_token mail(to: email_address.email, subject: t('user_mailer.account_reset_request.subject')) diff --git a/app/views/user_mailer/personal_key_sign_in.html.slim b/app/views/user_mailer/personal_key_sign_in.html.slim new file mode 100644 index 00000000000..9ae7e2becc0 --- /dev/null +++ b/app/views/user_mailer/personal_key_sign_in.html.slim @@ -0,0 +1,16 @@ +p.lead + strong == t('.intro') + +table.spacer + tbody + tr + td.s10 height="10px" + |   +table.hr + tr + th + |   + +p == t('.help_html', + reset_password_url: forgot_password_url, + account_url: account_url) diff --git a/config/locales/jobs/en.yml b/config/locales/jobs/en.yml index b3e2214936e..864b89c864a 100644 --- a/config/locales/jobs/en.yml +++ b/config/locales/jobs/en.yml @@ -12,3 +12,6 @@ en: in to your account. This code will expire in %{expiration} minutes." verify_message: "%{code} is your %{app} confirmation code. Use this to confirm your phone number. This code will expire in %{expiration} minutes." + sms_personal_key_sign_in_notifier_job: + message: Your personal key was just used to sign into your login.gov account. + If this wasn’t you, reset your password and contact us at security@login.gov. diff --git a/config/locales/jobs/es.yml b/config/locales/jobs/es.yml index e1b9283f282..d2aec114d8d 100644 --- a/config/locales/jobs/es.yml +++ b/config/locales/jobs/es.yml @@ -12,3 +12,6 @@ es: ingresando a su cuenta. Este código caducará en %{expiration} minutos." verify_message: "%{code} es tu código de confirmación de %{app}. Use esto para confirmar su número de teléfono. Este código caducará en %{expiration} minutos." + sms_personal_key_sign_in_notifier_job: + message: Su clave personal solo se utilizó para iniciar sesión en su cuenta + login.gov. Si no fue así, reinicie su contraseña y contáctenos en security@login.gov. diff --git a/config/locales/jobs/fr.yml b/config/locales/jobs/fr.yml index c8f6abbcb13..a2aa952d6ab 100644 --- a/config/locales/jobs/fr.yml +++ b/config/locales/jobs/fr.yml @@ -14,3 +14,7 @@ fr: verify_message: "%{code} est votre code de confirmation %{app}. Utilisez-le pour confirmer votre numéro de téléphone. Ce code expirera dans %{expiration} minutes." + sms_personal_key_sign_in_notifier_job: + message: Votre clé personnelle a été utilisée pour vous connecter à votre compte + login.gov. Si ce n’était pas vous, changez votre mot de passe et contactez-nous + à security@login.gov. diff --git a/config/locales/user_mailer/en.yml b/config/locales/user_mailer/en.yml index f2103222dea..aaf2a4491e0 100644 --- a/config/locales/user_mailer/en.yml +++ b/config/locales/user_mailer/en.yml @@ -44,6 +44,24 @@ en: help: If you did not make this change, please visit the %{app} %{help_link} or %{contact_link}. intro: You have a new password for your %{app} account. + personal_key_sign_in: + help_html:

Your login.gov account was just signed into using your 16-character + personal key. You're getting this email to make sure it was you.

If + you just signed in using your personal key, great! There's nothing you need + to do.

If you did not sign in with a personal key, or if you're not + sure, please immediately take these steps to secure your account:

  1. Change your password. Choose a password + that you haven't already used with this account.
  2. Sign + in to your login.gov account and make sure you recognize all + of the information on your account page, including the methods you use for + two-factor authentication, such as phone number, authentication app, or security + key.
  3. On your login.gov account page, request a new personal + key. Remember, never share it unless you are using it to sign into + a trusted website that uses login.gov.

You should then contact + us by calling 844-875-6446 or emailing security@login.gov.

+
Thanks,
The login.gov team + intro: Personal key used to sign in + subject: Account Security Alert phone_changed: help: If you did not want to change your phone number, please visit the %{app} %{help_link} or %{contact_link}. diff --git a/config/locales/user_mailer/es.yml b/config/locales/user_mailer/es.yml index 1c8488cca5b..eb4eb6a58dc 100644 --- a/config/locales/user_mailer/es.yml +++ b/config/locales/user_mailer/es.yml @@ -43,6 +43,26 @@ es: password_changed: help: Si no realizó este cambio, visite el %{app} %{help_link} o el %{contact_link}. intro: Tiene una contraseña nueva para su cuenta de %{app}. + personal_key_sign_in: + help_html:

Su cuenta login.gov acaba de iniciar sesión con su clave personal + de 16 caracteres. Usted está recibiendo este e-mail para asegurarse de que + era usted.

Si acaba de iniciar sesión con su clave personal, ¡genial! + No hay nada que tenga que hacer.

Si no inició sesión con una clave personal, + o si no está seguro, por favor, siga estos pasos de inmediato para proteger + su cuenta.

  1. Cambie tu contraseña. + Elija una contraseña que no haya utilizado con esta cuenta.
  2. Inicie sesión en su cuenta login.gov y + asegúrese de reconocer toda la información en la página de su cuenta, incluidos + los métodos que utiliza para la autenticación de dos factores, como el número + de teléfono, la aplicación de autenticación, o la clave de seguridad.
  3. En + la página de su cuenta login.gov, solicite una nueva clave personal. + Recuerde, nunca lo comparta a menos que lo esté utilizando para iniciar sesión + en un sitio web de confianza que utiliza login.gov.

Luego + debe comunicarse con nosotros llamando al 844-875-6446 o enviando un e-mail + a security@login.gov.


Gracias,
El + equipo de login.gov + intro: Clave personal utilizada para iniciar sesión + subject: Alerta de seguridad de cuenta phone_changed: help: Si no desea cambiar su número de teléfono, visite el %{app} %{help_link} o el %{contact_link}. diff --git a/config/locales/user_mailer/fr.yml b/config/locales/user_mailer/fr.yml index 056c325adc3..1c386e50dba 100644 --- a/config/locales/user_mailer/fr.yml +++ b/config/locales/user_mailer/fr.yml @@ -45,6 +45,27 @@ fr: help: Si vous n'avez pas changé votre mot de passe, veuillez visiter le %{help_link} de %{app} ou %{contact_link}. intro: Le mot de passe de votre compte %{app} a été changé. + personal_key_sign_in: + help_html:

Votre compte login.gov a été connecté à l'aide de votre clé personnelle. + Vous recevez cet email pour vous assurer que c'était bien vous.

Si vous + venez de vous connecter avec votre clé personnelle, c'est parfait! Vous ne + devez rien faire.

Si vous ne vous êtes pas connecté avec une clé personnelle + ou si vous n’êtes pas sûr, prenez immédiatement les mesures suivantes pour + sécuriser votre compte:

  1. Changez + votre mot de passe. Choisissez un mot de passe que vous n'avez + pas encore utilisé avec ce compte.
  2. Connectez-vous + à votre compte login.gov et assurez-vous de reconnaître toutes + les informations de la page de votre compte, y compris les méthodes que vous + utilisez pour l'authentification à deux facteurs, telles que le numéro de + téléphone, l'application d'authentification ou la clé de sécurité.
  3. Sur + la page de votre compte login.gov, demandez une nouvelle clé personnelle. + N'oubliez pas de ne jamais le partager à moins que vous ne l'utilisiez pour + vous connecter à un site Web de confiance utilisant login.gov.

Vous + devriez a nous contacter par téléphone au 844-875-6446 ou par courriel à security@login.gov.


Merci,
L'équipe + login.gov + intro: La clé personnelle utilisée pour vous connecter + subject: Alerte de sécurité du compte phone_changed: help: Si vous ne souhaitiez pas changer votre numéro de téléphone, veuillez visiter le %{help_link} de %{app} ou %{contact_link}. diff --git a/spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb b/spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb index d28fede5c2f..79c0603a246 100644 --- a/spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb +++ b/spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb @@ -1,11 +1,17 @@ require 'rails_helper' feature 'Signing in via one-time use personal key' do - it 'destroys old key, displays new one, and redirects to profile after acknowledging' do - user = create(:user, :signed_up) + it 'destroys old key, displays new one, notifies, and redirects to profile after acknowledging' do + user = create(:user, :signed_up, :with_phone, with: { phone: '+1 (202) 345-6789' }) raw_key = PersonalKeyGenerator.new(user).create old_key = user.reload.encrypted_recovery_code_digest + personal_key_sign_in_mail = double + expect(personal_key_sign_in_mail).to receive(:deliver_now) + expect(UserMailer).to receive(:personal_key_sign_in).and_return(personal_key_sign_in_mail) + expect(SmsPersonalKeySignInNotifierJob).to receive(:perform_now). + with(phone: '+1 (202) 345-6789') + sign_in_before_2fa(user) choose_another_security_option('personal_key') enter_personal_key(personal_key: raw_key) diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 47d14a1e334..5f36a1173a9 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -418,6 +418,7 @@ # this can happen if you submit the personal key form multiple times quickly it 'redirects to the personal key page' do user = create(:user, :signed_up) + stub_twilio_service old_personal_key = PersonalKeyGenerator.new(user).create signin(user.email, user.password) choose_another_security_option('personal_key') diff --git a/spec/forms/personal_key_form_spec.rb b/spec/forms/personal_key_form_spec.rb index 83120fc7bb9..005a78a1dff 100644 --- a/spec/forms/personal_key_form_spec.rb +++ b/spec/forms/personal_key_form_spec.rb @@ -17,6 +17,26 @@ expect(form.submit).to eq result expect(user.reload.encrypted_recovery_code_digest).to eq old_code end + + it 'sends an email and SMS notification to the user' do + user = create( + :user, + :with_phone, + email: 'jonny.hoops@gsa.gov', + with: { phone: '+1 (202) 345-6789' } + ) + raw_code = PersonalKeyGenerator.new(user).create + + personal_key_sign_in_mail = double + expect(personal_key_sign_in_mail).to receive(:deliver_now) + expect(UserMailer).to receive(:personal_key_sign_in). + with('jonny.hoops@gsa.gov'). + and_return(personal_key_sign_in_mail) + expect(SmsPersonalKeySignInNotifierJob).to receive(:perform_now). + with(phone: '+1 (202) 345-6789') + + PersonalKeyForm.new(user, raw_code).submit + end end context 'when the form is invalid' do diff --git a/spec/jobs/sms_account_reset_notifier_job_spec.rb b/spec/jobs/sms_account_reset_notifier_job_spec.rb index 822ce2935fa..651d9f3191f 100644 --- a/spec/jobs/sms_account_reset_notifier_job_spec.rb +++ b/spec/jobs/sms_account_reset_notifier_job_spec.rb @@ -21,8 +21,6 @@ it 'sends a message containing the cancel link to the mobile number', twilio: true do allow(Figaro.env).to receive(:twilio_messaging_service_sid).and_return('fake_sid') - TwilioService::Utils.telephony_service = FakeSms - perform messages = FakeSms.messages diff --git a/spec/jobs/sms_personal_key_sign_in_notifier_job_spec.rb b/spec/jobs/sms_personal_key_sign_in_notifier_job_spec.rb new file mode 100644 index 00000000000..96b13f4766f --- /dev/null +++ b/spec/jobs/sms_personal_key_sign_in_notifier_job_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +describe SmsPersonalKeySignInNotifierJob do + include Features::ActiveJobHelper + + before do + reset_job_queues + TwilioService::Utils.telephony_service = FakeSms + FakeSms.messages = [] + end + + describe '.perform' do + it 'sends a message about the personal key sign in to the user' do + allow(Figaro.env).to receive(:twilio_messaging_service_sid).and_return('fake_sid') + + described_class.perform_now(phone: '+1 (202) 345-6789') + + messages = FakeSms.messages + + expect(messages.size).to eq(1) + + msg = messages.first + + expect(msg.messaging_service_sid).to eq('fake_sid') + expect(msg.to).to eq('+1 (202) 345-6789') + expect(msg.body). + to eq(I18n.t('jobs.sms_personal_key_sign_in_notifier_job.message', app: APP_NAME)) + end + end +end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index d9384e7a0b0..2ca18021e43 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -46,6 +46,26 @@ end end + describe 'personal_key_sign_in' do + let(:mail) { UserMailer.personal_key_sign_in(user.email) } + + it_behaves_like 'a system email' + + it 'sends to the current email' do + expect(mail.to).to eq [user.email] + end + + it 'renders the subject' do + expect(mail.subject).to eq t('user_mailer.personal_key_sign_in.subject') + end + + it 'renders the body' do + expect(mail.html_part.body).to have_content( + t('user_mailer.personal_key_sign_in.intro') + ) + end + end + describe 'signup_with_your_email' do let(:mail) { UserMailer.signup_with_your_email(user.email) } diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index 9e165e77f8b..9bb44b599f7 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -52,6 +52,7 @@ shared_examples 'signing in as LOA3 with personal key' do |sp| it 'redirects to the SP after acknowledging new personal key', :email do + stub_twilio_service user = create_loa3_account_go_back_to_sp_and_sign_out(sp) pii = { ssn: '666-66-1234', dob: '1920-01-01', first_name: 'alice' } From 6b46f6a076e16f160911eb6ccd561ce6cc3f2c59 Mon Sep 17 00:00:00 2001 From: James G Smith Date: Mon, 5 Nov 2018 14:32:27 -0500 Subject: [PATCH 02/39] Stub twilio when testing personal key as mfa (#2644) **Why**: Now we send a text when a user generates a new personal key. --- spec/features/users/password_recovery_via_recovery_code_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/features/users/password_recovery_via_recovery_code_spec.rb b/spec/features/users/password_recovery_via_recovery_code_spec.rb index 0bdf1da1fda..ba615fb3188 100644 --- a/spec/features/users/password_recovery_via_recovery_code_spec.rb +++ b/spec/features/users/password_recovery_via_recovery_code_spec.rb @@ -56,6 +56,7 @@ end scenario 'resets password, uses personal key as 2fa', email: true do + stub_twilio_service personal_key = personal_key_from_pii(user, pii) trigger_reset_password_and_click_email_link(user.email) From b81c005321239e1dfa5877e0178d8a7cf2c96769 Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Wed, 7 Nov 2018 12:53:43 -0500 Subject: [PATCH 03/39] LG-775 Do not present FIDO auth option if browser does not support FIDO (#2642) **Why**: So the user is not presented with 2FA options they can't use. **How**: Display the security key option as hidden in the list of 2FA options. Use javascript to determine if FIDO is supported. If it is supported unhide the security key option. If it is not supported make sure that the next 2FA option is selected by default. --- app/javascript/packs/webauthn-setup.js | 2 +- app/javascript/packs/webauthn-unhide.js | 17 +++++++++++++++++ .../selection_presenter.rb | 4 ++++ .../webauthn_selection_presenter.rb | 4 ++++ .../options/index.html.slim | 18 +++++++++++------- .../webauthn_selection_presenter_spec.rb | 6 ++++++ 6 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 app/javascript/packs/webauthn-unhide.js diff --git a/app/javascript/packs/webauthn-setup.js b/app/javascript/packs/webauthn-setup.js index 158b4010309..0eba58c8d48 100644 --- a/app/javascript/packs/webauthn-setup.js +++ b/app/javascript/packs/webauthn-setup.js @@ -71,7 +71,7 @@ function webauthn() { }; if (location.href.indexOf('?error=') === -1 && !(navigator && navigator.credentials && navigator.credentials.create)) { - window.location.href = '/webauthn_setup?error=NotSupportedError'; + window.location.href = '/login/two_factor/options'; } const continueButton = document.getElementById('continue-button'); continueButton.addEventListener('click', () => { diff --git a/app/javascript/packs/webauthn-unhide.js b/app/javascript/packs/webauthn-unhide.js new file mode 100644 index 00000000000..872f4220070 --- /dev/null +++ b/app/javascript/packs/webauthn-unhide.js @@ -0,0 +1,17 @@ +function unhideWebauthn() { + if (navigator && navigator.credentials && navigator.credentials.create) { + const elem = document.getElementById('select_webauthn'); + if (elem) { + elem.classList.remove('hidden'); + } + } else { + const checkboxes = document.querySelectorAll('input[name="two_factor_options_form[selection]"]'); + for (let i = 0, len = checkboxes.length; i < len; i += 1) { + if (!checkboxes[i].classList.contains('hidden')) { + checkboxes[i].checked = true; + break; + } + } + } +} +document.addEventListener('DOMContentLoaded', unhideWebauthn); diff --git a/app/presenters/two_factor_authentication/selection_presenter.rb b/app/presenters/two_factor_authentication/selection_presenter.rb index 9cc06235fe1..6e6b383168d 100644 --- a/app/presenters/two_factor_authentication/selection_presenter.rb +++ b/app/presenters/two_factor_authentication/selection_presenter.rb @@ -21,6 +21,10 @@ def info t("two_factor_authentication.#{option_mode}.#{method}_info") end + def html_class + '' + end + private def option_mode diff --git a/app/presenters/two_factor_authentication/webauthn_selection_presenter.rb b/app/presenters/two_factor_authentication/webauthn_selection_presenter.rb index 53d9ddff59f..8a697032cd3 100644 --- a/app/presenters/two_factor_authentication/webauthn_selection_presenter.rb +++ b/app/presenters/two_factor_authentication/webauthn_selection_presenter.rb @@ -3,5 +3,9 @@ class WebauthnSelectionPresenter < SelectionPresenter def method :webauthn end + + def html_class + 'hidden' + end end end diff --git a/app/views/two_factor_authentication/options/index.html.slim b/app/views/two_factor_authentication/options/index.html.slim index 4ad5242d9af..5b51075e6fc 100644 --- a/app/views/two_factor_authentication/options/index.html.slim +++ b/app/views/two_factor_authentication/options/index.html.slim @@ -11,14 +11,16 @@ p.mt-tiny.mb3 = @presenter.info fieldset.m0.p0.border-none. legend.mb2.serif.bold = @presenter.label - @presenter.options.each_with_index do |option, index| - label.btn-border.col-12.mb2 for="two_factor_options_form_selection_#{option.type}" - .radio - = radio_button_tag('two_factor_options_form[selection]', + span id="select_#{option.type}" class="#{option.html_class}" + label.btn-border.col-12.mb2 for="two_factor_options_form_selection_#{option.type}" + .radio + = radio_button_tag('two_factor_options_form[selection]', option.type, - index.zero?) - span.indicator.mt-tiny - span.blue.bold.fs-20p = option.label - .regular.gray-dark.fs-10p.mb-tiny = option.info + index.zero?, + class: option.html_class.to_s) + span.indicator.mt-tiny + span.blue.bold.fs-20p = option.label + .regular.gray-dark.fs-10p.mb-tiny = option.info = f.button :submit, t('forms.buttons.continue') @@ -26,3 +28,5 @@ br - if @presenter.should_display_account_reset_or_cancel_link? p = @presenter.account_reset_or_cancel_link = render 'shared/cancel', link: destroy_user_session_path + +== javascript_pack_tag 'webauthn-unhide' diff --git a/spec/presenters/two_factor_authentication/webauthn_selection_presenter_spec.rb b/spec/presenters/two_factor_authentication/webauthn_selection_presenter_spec.rb index 74457bdb301..1743d1e9e4d 100644 --- a/spec/presenters/two_factor_authentication/webauthn_selection_presenter_spec.rb +++ b/spec/presenters/two_factor_authentication/webauthn_selection_presenter_spec.rb @@ -9,4 +9,10 @@ expect(subject.type).to eq 'webauthn' end end + + describe '#html_class' do + it 'returns hidden' do + expect(subject.html_class).to eq 'hidden' + end + end end From 635e98bd1ca9e44daadaed9400a41073cb62743c Mon Sep 17 00:00:00 2001 From: Gregory John Casamento Date: Wed, 17 Oct 2018 13:36:29 -0400 Subject: [PATCH 04/39] LG-570 Update typography scale to match spec **Why**: The scale wasn't updated after the spec changed --- app/assets/stylesheets/variables/_app.scss | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/variables/_app.scss b/app/assets/stylesheets/variables/_app.scss index 2125b2ec0fb..8d19a7fa99c 100644 --- a/app/assets/stylesheets/variables/_app.scss +++ b/app/assets/stylesheets/variables/_app.scss @@ -22,15 +22,15 @@ $line-height-2: 1.3 !default; $line-height-3: 1.75 !default; $line-height-4: 2 !default; -$h1: 1.75rem !default; -$h2: 1.5rem !default; -$h3: 1.125rem !default; +$h1: 1.625rem !default; +$h2: 1.25rem !default; +$h3: 1rem !default; $h4: .875rem !default; -$h5: .75rem !default; -$h6: .75rem !default; +$h5: .8125rem !default; +$h6: .625rem !default; -$sm-h1: 2.5rem !default; -$sm-h2: 2rem !default; +$sm-h1: 1.3125rem !default; +$sm-h2: 1.125rem !default; $sm-h3: 1.5rem !default; $sm-h4: 1.125rem !default; $sm-h5: .875rem !default; From b96b6e8798bc5eeda6124094a8d671fcecd10ff0 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Wed, 7 Nov 2018 15:35:33 -0500 Subject: [PATCH 05/39] LG-746 Return 400 error for invalid String params (#2648) **Why**: When a controller uses Strong Parameters such as: `params.require(:user).permit(:email)`, the `user` param is assumed to be a Hash, but it's easy for a pentester (for example) to set the `user` param to a String instead, which by default would raise a 500 error because the String class doesn't respond to `permit`. **How**: To get around that, we monkey patched the Ruby String class to raise an instance of `ActionController::ParameterMissing`, which will return a 400 error. 500 errors can potentially page people in the middle of the night, whereas 400 errors don't. Note that Devise handles this scenario gracefully in SessionsController, but because it determines whether or not to call `permit` based on whether or not the object responds to `permit`, the Devise code will end up calling `permit` because all Strings respond to it now after our monkey patch. This is why the `invalid_sign_in_params_spec` had to change. --- app/controllers/application_controller.rb | 3 ++ .../sign_up/email_resend_controller.rb | 2 -- lib/core_extensions/string/permit.rb | 18 +++++++++++ spec/requests/invalid_sign_in_params_spec.rb | 8 +++-- .../params_is_string_instead_of_hash_spec.rb | 30 +++++++++++++++++++ 5 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 lib/core_extensions/string/permit.rb create mode 100644 spec/requests/params_is_string_instead_of_hash_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 462c8f3e7df..a36efcf8e2f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,4 +1,7 @@ +require 'core_extensions/string/permit' + class ApplicationController < ActionController::Base + String.include CoreExtensions::String::Permit include UserSessionContext include VerifyProfileConcern include LocaleHelper diff --git a/app/controllers/sign_up/email_resend_controller.rb b/app/controllers/sign_up/email_resend_controller.rb index 5a46b03636d..2ee962a5af0 100644 --- a/app/controllers/sign_up/email_resend_controller.rb +++ b/app/controllers/sign_up/email_resend_controller.rb @@ -1,7 +1,5 @@ module SignUp class EmailResendController < ApplicationController - include UnconfirmedUserConcern - def new @user = User.new @resend_email_confirmation_form = ResendEmailConfirmationForm.new diff --git a/lib/core_extensions/string/permit.rb b/lib/core_extensions/string/permit.rb new file mode 100644 index 00000000000..37fb341ebd4 --- /dev/null +++ b/lib/core_extensions/string/permit.rb @@ -0,0 +1,18 @@ +# When a controller uses Strong Parameters such as: +# params.require(:user).permit(:email), the `user` param is assumed to +# be a Hash, but it's easy for a pentester (for example) to set the +# `user` param to a String instead, which by default would raise a 500 +# error because the String class doesn't respond to `permit`. To get +# around that, we monkey patched the Ruby String class to raise an +# instance of ActionController::ParameterMissing, which will return +# a 400 error. 500 errors can potentially page people in the middle of +# the night, whereas 400 errors don't. +module CoreExtensions + module String + module Permit + def permit(*) + raise ActionController::ParameterMissing, '#permit called on String' + end + end + end +end diff --git a/spec/requests/invalid_sign_in_params_spec.rb b/spec/requests/invalid_sign_in_params_spec.rb index c4721221143..b42a0196ea2 100644 --- a/spec/requests/invalid_sign_in_params_spec.rb +++ b/spec/requests/invalid_sign_in_params_spec.rb @@ -1,8 +1,12 @@ require 'rails_helper' describe 'visiting sign in page with invalid user params' do - it 'does not raise an exception' do - get new_user_session_path, params: { user: 'test@test.com' } + it 'raises ActionController::ParameterMissing' do + params = { user: 'test@test.com' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { get new_user_session_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) end end diff --git a/spec/requests/params_is_string_instead_of_hash_spec.rb b/spec/requests/params_is_string_instead_of_hash_spec.rb new file mode 100644 index 00000000000..8c265907ce8 --- /dev/null +++ b/spec/requests/params_is_string_instead_of_hash_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +# When a controller uses Strong Parameters such as: +# params.require(:user).permit(:email), the `user` param is assumed to +# be a Hash, but it's easy for a pentester (for example) to set the +# `user` param to a String instead, which by default would raise a 500 +# error because the String class doesn't respond to `permit`. To get +# around that, we monkey patched the Ruby String class to raise an +# instance of ActionController::ParameterMissing, which will return +# a 400 error. 500 errors can potentially page people in the middle of +# the night, whereas 400 errors don't. +describe 'submitting email resend form with required param as String' do + it 'raises ActionController::ParameterMissing' do + params = { resend_email_confirmation_form: 'abcdef' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { post sign_up_create_email_resend_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) + end +end + +describe 'submitting email registration form with required param as String' do + it 'raises ActionController::ParameterMissing' do + params = { user: 'abcdef' } + message_string = 'param is missing or the value is empty: #permit called on String' + + expect { post sign_up_register_path, params: params }. + to raise_error(ActionController::ParameterMissing, message_string) + end +end From a48dabb32f3241b17985d192e433d92db87141ee Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Fri, 9 Nov 2018 14:54:10 -0500 Subject: [PATCH 06/39] Fix redirect when not presenting FIDO (#2651) **Why**: The 2FA options list for login does not display FIDO when it is not supported. However the code bypasses the selection list and takes you to the preferred option that you have configured. A redirect was added to take the user back to the selection list but it was added to the setup js and not the authenticate js **How**: In webauthn-authenticate.js, redirect to /login/two_factor/options when the user is on the webauthn login screen and FIDO is not supported. --- app/javascript/packs/webauthn-authenticate.js | 4 ++++ app/javascript/packs/webauthn-setup.js | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/javascript/packs/webauthn-authenticate.js b/app/javascript/packs/webauthn-authenticate.js index 7be4c2ee672..3b271d95009 100644 --- a/app/javascript/packs/webauthn-authenticate.js +++ b/app/javascript/packs/webauthn-authenticate.js @@ -17,6 +17,10 @@ function webauthn() { } return window.btoa(binary); }; + // If webauthn is not supported redirect back to the 2fa options list + if (!(navigator && navigator.credentials && navigator.credentials.create)) { + window.location.href = '/login/two_factor/options'; + } const challengeBytes = new Uint8Array(JSON.parse(document.getElementById('user_challenge').value)); let credentialIds = document.getElementById('credential_ids').value; credentialIds = credentialIds.split(','); diff --git a/app/javascript/packs/webauthn-setup.js b/app/javascript/packs/webauthn-setup.js index 0eba58c8d48..158b4010309 100644 --- a/app/javascript/packs/webauthn-setup.js +++ b/app/javascript/packs/webauthn-setup.js @@ -71,7 +71,7 @@ function webauthn() { }; if (location.href.indexOf('?error=') === -1 && !(navigator && navigator.credentials && navigator.credentials.create)) { - window.location.href = '/login/two_factor/options'; + window.location.href = '/webauthn_setup?error=NotSupportedError'; } const continueButton = document.getElementById('continue-button'); continueButton.addEventListener('click', () => { From 0e8ffaf91474eb566ed2f69c1921dc95a66e2e29 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Fri, 9 Nov 2018 19:58:54 -0500 Subject: [PATCH 07/39] LG-793 Add user event when removing phone number (#2649) **Why**: To allow the user to check for unusual activity on their account page, and to make it easier for us to troubleshoot security issues. --- app/controllers/users/phones_controller.rb | 7 ++++++- app/models/event.rb | 1 + config/locales/event_types/en.yml | 1 + config/locales/event_types/es.yml | 1 + config/locales/event_types/fr.yml | 1 + spec/features/users/user_edit_spec.rb | 1 + 6 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index 0d33f537179..f32bcd2bce7 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -26,7 +26,7 @@ def delete ).submit analytics.track_event(Analytics::PHONE_DELETION_REQUESTED, result.to_h) if result.success? - flash[:success] = t('two_factor_authentication.phone.delete.success') + handle_successful_delete else flash[:error] = t('two_factor_authentication.phone.delete.failure') end @@ -63,5 +63,10 @@ def process_updates redirect_to account_url end end + + def handle_successful_delete + flash[:success] = t('two_factor_authentication.phone.delete.success') + create_user_event(:phone_removed) + end end end diff --git a/app/models/event.rb b/app/models/event.rb index 13d468e6fc7..9683ca8ecdc 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -17,6 +17,7 @@ class Event < ApplicationRecord personal_key_used: 13, webauthn_key_added: 14, webauthn_key_removed: 15, + phone_removed: 16, } validates :event_type, presence: true diff --git a/config/locales/event_types/en.yml b/config/locales/event_types/en.yml index 6dc6d1abcca..1bd9c6ad802 100644 --- a/config/locales/event_types/en.yml +++ b/config/locales/event_types/en.yml @@ -14,6 +14,7 @@ en: personal_key_used: Personal key used to sign in phone_changed: Phone number changed phone_confirmed: Phone confirmed + phone_removed: Phone number removed piv_cac_disabled: PIV/CAC card unassociated piv_cac_enabled: PIV/CAC card associated usps_mail_sent: Letter sent diff --git a/config/locales/event_types/es.yml b/config/locales/event_types/es.yml index e6d3666452d..fd997966804 100644 --- a/config/locales/event_types/es.yml +++ b/config/locales/event_types/es.yml @@ -14,6 +14,7 @@ es: personal_key_used: Clave personal utilizada para iniciar sesión phone_changed: Número de teléfono cambiado phone_confirmed: Teléfono confirmado + phone_removed: Teléfono eliminado piv_cac_disabled: Tarjeta PIV/CAC no asociada piv_cac_enabled: Tarjeta PIV/CAC asociada usps_mail_sent: Carta enviada diff --git a/config/locales/event_types/fr.yml b/config/locales/event_types/fr.yml index 4d9fc103f25..fc313f97567 100644 --- a/config/locales/event_types/fr.yml +++ b/config/locales/event_types/fr.yml @@ -14,6 +14,7 @@ fr: personal_key_used: Clé personnelle utilisée pour la connexion phone_changed: Numéro de téléphone modifié phone_confirmed: Numéro de téléphone confirmé + phone_removed: Numéro de téléphone supprimé piv_cac_disabled: Carte PIV/CAC non associée piv_cac_enabled: Carte PIV/CAC associée usps_mail_sent: Lettre envoyée diff --git a/spec/features/users/user_edit_spec.rb b/spec/features/users/user_edit_spec.rb index cc8a4d59f78..0942096977b 100644 --- a/spec/features/users/user_edit_spec.rb +++ b/spec/features/users/user_edit_spec.rb @@ -89,6 +89,7 @@ click_button t('forms.phone.buttons.delete') expect(page).to have_current_path(account_path) expect(MfaPolicy.new(user.reload).multiple_factors_enabled?).to eq false + expect(page).to have_content t('event_types.phone_removed') end end end From e343e5bcd14a607de549f0af177e2467df0f408d Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:42:20 -0500 Subject: [PATCH 08/39] Update aws-sdk-kms from 1.9.0 to 1.11.0 --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 34a3ec3d899..d7c0822ba89 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -117,13 +117,13 @@ GEM arel (8.0.0) ast (2.4.0) aws-eventstream (1.0.1) - aws-partitions (1.103.0) - aws-sdk-core (3.27.0) + aws-partitions (1.111.0) + aws-sdk-core (3.38.0) aws-eventstream (~> 1.0) aws-partitions (~> 1.0) aws-sigv4 (~> 1.0) jmespath (~> 1.0) - aws-sdk-kms (1.9.0) + aws-sdk-kms (1.11.0) aws-sdk-core (~> 3, >= 3.26.0) aws-sigv4 (~> 1.0) aws-sdk-s3 (1.17.0) @@ -756,4 +756,4 @@ RUBY VERSION ruby 2.5.1p57 BUNDLED WITH - 1.16.5 + 1.16.6 From e8074c97ab8e74179f0aa8c9f54929b29b7101f4 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:42:31 -0500 Subject: [PATCH 09/39] Update aws-sdk-ses from 1.10.0 to 1.13.0 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index d7c0822ba89..a85a7b1fe1d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -130,7 +130,7 @@ GEM aws-sdk-core (~> 3, >= 3.21.2) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.0) - aws-sdk-ses (1.10.0) + aws-sdk-ses (1.13.0) aws-sdk-core (~> 3, >= 3.26.0) aws-sigv4 (~> 1.0) aws-sigv4 (1.0.3) From c7c02158d5c15092f48df840385f59a7ed98fb41 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:42:45 -0500 Subject: [PATCH 10/39] Update bullet from 5.7.6 to 5.9.0 --- Gemfile.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a85a7b1fe1d..33d2411ed27 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -154,9 +154,9 @@ GEM brakeman (4.3.1) browser (2.5.3) builder (3.2.3) - bullet (5.7.6) + bullet (5.9.0) activesupport (>= 3.0.0) - uniform_notifier (~> 1.11.0) + uniform_notifier (~> 1.11) bummr (0.3.2) rainbow thor @@ -190,7 +190,7 @@ GEM coercible (1.0.0) descendants_tracker (~> 0.0.1) colorize (0.8.1) - concurrent-ruby (1.0.5) + concurrent-ruby (1.1.3) connection_pool (2.2.2) cose (0.1.0) cbor (~> 0.5.9.2) @@ -299,7 +299,7 @@ GEM httpi (2.4.3) rack socksify - i18n (1.1.0) + i18n (1.1.1) concurrent-ruby (~> 1.0) i18n-tasks (0.9.24) activesupport (>= 4.0.2) @@ -600,7 +600,7 @@ GEM uglifier (3.2.0) execjs (>= 0.3.0, < 3) unicode-display_width (1.4.0) - uniform_notifier (1.11.0) + uniform_notifier (1.12.1) user_agent_parser (2.4.1) uuid (2.3.9) macaddr (~> 1.0) From 565fffdb4284880067ab618c6ac2725ae59da49e Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:42:59 -0500 Subject: [PATCH 11/39] Update capybara-screenshot from 1.0.21 to 1.0.22 --- Gemfile.lock | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 33d2411ed27..2f87bb25198 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -161,14 +161,15 @@ GEM rainbow thor byebug (10.0.2) - capybara (3.7.2) + capybara (3.10.1) addressable mini_mime (>= 0.1.3) nokogiri (~> 1.8) rack (>= 1.6.0) rack-test (>= 0.6.3) - xpath (~> 3.1) - capybara-screenshot (1.0.21) + regexp_parser (~> 1.2) + xpath (~> 3.2) + capybara-screenshot (1.0.22) capybara (>= 1.0, < 4) launchy capybara-selenium (0.0.6) @@ -357,7 +358,7 @@ GEM net-ssh (4.1.0) newrelic_rpm (5.4.0.347) nio4r (2.3.1) - nokogiri (1.8.4) + nokogiri (1.8.5) mini_portile2 (~> 2.3.0) nori (2.6.0) notiffany (0.1.1) @@ -389,7 +390,7 @@ GEM pry (~> 0.10) public_suffix (3.0.3) puma (3.12.0) - rack (2.0.5) + rack (2.0.6) rack-attack (5.4.0) rack (>= 1.0, < 3) rack-cors (1.0.2) @@ -457,6 +458,7 @@ GEM parser (>= 2.5.0.0, < 2.6, != 2.5.1.1) rainbow (>= 2.0, < 4.0) referer-parser (0.3.0) + regexp_parser (1.2.0) request_store (1.4.1) rack (>= 1.4) responders (2.4.0) @@ -641,7 +643,7 @@ GEM xmlmapper (>= 0.7.3) xmlmapper (0.7.3) nokogiri (~> 1.5) - xpath (3.1.0) + xpath (3.2.0) nokogiri (~> 1.8) zonebie (0.6.1) zxcvbn-js (4.4.3) From d57e0e0316e55801ed4fd4cfeed64f62fd39c769 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:43:10 -0500 Subject: [PATCH 12/39] Update chromedriver-helper from 1.2.0 to 2.1.0 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 2f87bb25198..119f49410ad 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -179,7 +179,7 @@ GEM childprocess (0.9.0) ffi (~> 1.0, >= 1.0.11) choice (0.2.0) - chromedriver-helper (1.2.0) + chromedriver-helper (2.1.0) archive-zip (~> 0.10) nokogiri (~> 1.8) chunky_png (1.3.10) From b455fe4c6e551fbac52926f9aca1ef060a005b20 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:43:20 -0500 Subject: [PATCH 13/39] Update codeclimate-test-reporter from 1.0.8 to 1.0.9 --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 119f49410ad..f3042e644ff 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -185,7 +185,7 @@ GEM chunky_png (1.3.10) codeclimate-engine-rb (0.4.1) virtus (~> 1.0) - codeclimate-test-reporter (1.0.8) + codeclimate-test-reporter (1.0.9) simplecov (<= 0.13) coderay (1.1.2) coercible (1.0.0) @@ -543,7 +543,7 @@ GEM docile (~> 1.1.0) json (>= 1.8, < 3) simplecov-html (~> 0.10.0) - simplecov-html (0.10.0) + simplecov-html (0.10.2) sinatra (2.0.3) mustermann (~> 1.0) rack (~> 2.0) From 93e5614563ead014e5918f245467837ee45a0c8c Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:43:32 -0500 Subject: [PATCH 14/39] Update factory_bot_rails from 4.11.0 to 4.11.1 --- Gemfile.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f3042e644ff..fa989a2c063 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -244,10 +244,10 @@ GEM actionmailer (>= 4.0, < 6) activesupport (>= 4.0, < 6) execjs (2.7.0) - factory_bot (4.11.0) + factory_bot (4.11.1) activesupport (>= 3.0.0) - factory_bot_rails (4.11.0) - factory_bot (~> 4.11.0) + factory_bot_rails (4.11.1) + factory_bot (~> 4.11.1) railties (>= 3.0.0) fakefs (0.18.0) faker (1.9.1) @@ -336,7 +336,7 @@ GEM activesupport (>= 4) railties (>= 4) request_store (~> 1.0) - loofah (2.2.2) + loofah (2.2.3) crass (~> 1.0.2) nokogiri (>= 1.5.9) lumberjack (1.0.13) @@ -345,7 +345,7 @@ GEM mail (2.7.0) mini_mime (>= 0.1.1) memory_profiler (0.9.11) - method_source (0.9.0) + method_source (0.9.2) mini_mime (1.0.1) mini_portile2 (2.3.0) minitest (5.11.3) @@ -581,7 +581,7 @@ GEM daemons (~> 1.0, >= 1.0.9) eventmachine (~> 1.0, >= 1.0.4) rack (>= 1, < 3) - thor (0.20.0) + thor (0.20.3) thread_safe (0.3.6) tilt (2.0.8) timecop (0.9.1) From e06cb6450918c10272444efb8a30f566b4aa5861 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:44:03 -0500 Subject: [PATCH 15/39] Update hiredis from 0.6.1 to 0.6.3 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index fa989a2c063..b50e46312e1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -292,7 +292,7 @@ GEM hashie (3.6.0) heapy (0.1.3) highline (2.0.0) - hiredis (0.6.1) + hiredis (0.6.3) htmlentities (4.3.4) http_accept_language (2.1.1) httparty (0.16.2) From b901cbb9e2537462fb99b59794bcad3f55c382fb Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:44:14 -0500 Subject: [PATCH 16/39] Update httparty from 0.16.2 to 0.16.3 --- Gemfile.lock | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index b50e46312e1..04364434094 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -295,7 +295,8 @@ GEM hiredis (0.6.3) htmlentities (4.3.4) http_accept_language (2.1.1) - httparty (0.16.2) + httparty (0.16.3) + mime-types (~> 3.0) multi_xml (>= 0.5.2) httpi (2.4.3) rack @@ -346,6 +347,9 @@ GEM mini_mime (>= 0.1.1) memory_profiler (0.9.11) method_source (0.9.2) + mime-types (3.2.2) + mime-types-data (~> 3.2015) + mime-types-data (3.2018.0812) mini_mime (1.0.1) mini_portile2 (2.3.0) minitest (5.11.3) From 5f718c9f6c668074de2207c883b9880379f3a7bd Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:44:26 -0500 Subject: [PATCH 17/39] Update i18n-tasks from 0.9.24 to 0.9.28 --- Gemfile.lock | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 04364434094..273865bb20a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -303,13 +303,14 @@ GEM socksify i18n (1.1.1) concurrent-ruby (~> 1.0) - i18n-tasks (0.9.24) + i18n-tasks (0.9.28) activesupport (>= 4.0.2) ast (>= 2.1.0) erubi highline (>= 2.0.0) i18n parser (>= 2.2.3.0) + rails-i18n rainbow (>= 2.2.2, < 4.0) terminal-table (>= 1.5.1) ice_nine (0.11.2) @@ -373,7 +374,7 @@ GEM childprocess (~> 0.6, >= 0.6.3) iniparse (~> 1.4) parallel (1.12.1) - parser (2.5.1.2) + parser (2.5.3.0) ast (~> 2.4.0) pg (1.1.3) phonelib (0.6.24) @@ -437,6 +438,9 @@ GEM ruby-graphviz (~> 1.2) rails-html-sanitizer (1.0.4) loofah (~> 2.2, >= 2.2.2) + rails-i18n (5.1.2) + i18n (>= 0.7, < 2) + railties (>= 5.0, < 6) railties (5.1.6) actionpack (= 5.1.6) activesupport (= 5.1.6) From 8b9dcaeee534249563794698220a91ba24bdebaf Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:44:37 -0500 Subject: [PATCH 18/39] Update newrelic_rpm from 5.4.0.347 to 5.5.0.348 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 273865bb20a..c6b0263abb7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -361,7 +361,7 @@ GEM net-sftp (2.1.2) net-ssh (>= 2.6.5) net-ssh (4.1.0) - newrelic_rpm (5.4.0.347) + newrelic_rpm (5.5.0.348) nio4r (2.3.1) nokogiri (1.8.5) mini_portile2 (~> 2.3.0) From 5923c720fcb8e82f40afed09045f027bccf8a477 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:44:47 -0500 Subject: [PATCH 19/39] Update phonelib from 0.6.24 to 0.6.27 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index c6b0263abb7..cb23ef62159 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -377,7 +377,7 @@ GEM parser (2.5.3.0) ast (~> 2.4.0) pg (1.1.3) - phonelib (0.6.24) + phonelib (0.6.27) pkcs11 (0.2.7) powerpack (0.1.2) premailer (1.11.1) From ce8e69c69b02d4cf997cb702ce906d389d79b5dc Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:44:57 -0500 Subject: [PATCH 20/39] Update rack-attack from 5.4.0 to 5.4.2 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index cb23ef62159..b0e27b5880f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -396,7 +396,7 @@ GEM public_suffix (3.0.3) puma (3.12.0) rack (2.0.6) - rack-attack (5.4.0) + rack-attack (5.4.2) rack (>= 1.0, < 3) rack-cors (1.0.2) rack-headers_filter (0.0.1) From 536447295248f1ffb3bf43daa8b5daa0945bc729 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:45:07 -0500 Subject: [PATCH 21/39] Update recaptcha from 4.12.0 to 4.13.0 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index b0e27b5880f..f83041d61cb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -457,7 +457,7 @@ GEM readthis (2.2.0) connection_pool (~> 2.1) redis (>= 3.0, < 5.0) - recaptcha (4.12.0) + recaptcha (4.13.0) json redis (3.3.5) reek (5.0.2) From 17ea580f6d248ba414130f176bea0ef8d44592ea Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:45:19 -0500 Subject: [PATCH 22/39] Update reek from 5.0.2 to 5.2.0 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index f83041d61cb..ef94b340d10 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -460,7 +460,7 @@ GEM recaptcha (4.13.0) json redis (3.3.5) - reek (5.0.2) + reek (5.2.0) codeclimate-engine-rb (~> 0.4.0) kwalify (~> 0.7.0) parser (>= 2.5.0.0, < 2.6, != 2.5.1.1) From a37b46939830887f35937696f6022b149c56c039 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:45:31 -0500 Subject: [PATCH 23/39] Update rspec-rails from 3.8.0 to 3.8.1 --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index ef94b340d10..94d957d61c1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -481,13 +481,13 @@ GEM rspec-mocks (~> 3.8.0) rspec-core (3.8.0) rspec-support (~> 3.8.0) - rspec-expectations (3.8.1) + rspec-expectations (3.8.2) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.8.0) rspec-mocks (3.8.0) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.8.0) - rspec-rails (3.8.0) + rspec-rails (3.8.1) actionpack (>= 3.0) activesupport (>= 3.0) railties (>= 3.0) From 64d161db2b3f9f121d7cfc77fe54175bd629d56b Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:45:41 -0500 Subject: [PATCH 24/39] Update scrypt from 3.0.5 to 3.0.6 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 94d957d61c1..3ecd77dafe4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -534,7 +534,7 @@ GEM nokogiri (>= 1.8.1) nori (~> 2.4) wasabi (~> 3.4) - scrypt (3.0.5) + scrypt (3.0.6) ffi-compiler (>= 1.0, < 2.0) secure_headers (6.0.0) selenium-webdriver (3.14.0) From 73811f538ad94d2e96081c4f5bf5da767fbc6990 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:45:52 -0500 Subject: [PATCH 25/39] Update sinatra from 2.0.3 to 2.0.4 --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 3ecd77dafe4..b568698e291 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -356,7 +356,7 @@ GEM minitest (5.11.3) multi_xml (0.6.0) multipart-post (2.0.0) - mustermann (1.0.2) + mustermann (1.0.3) nenv (0.3.0) net-sftp (2.1.2) net-ssh (>= 2.6.5) @@ -402,7 +402,7 @@ GEM rack-headers_filter (0.0.1) rack-mini-profiler (1.0.0) rack (>= 1.2.0) - rack-protection (2.0.3) + rack-protection (2.0.4) rack rack-proxy (0.6.4) rack @@ -552,10 +552,10 @@ GEM json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) - sinatra (2.0.3) + sinatra (2.0.4) mustermann (~> 1.0) rack (~> 2.0) - rack-protection (= 2.0.3) + rack-protection (= 2.0.4) tilt (~> 2.0) slim (3.0.9) temple (>= 0.7.6, < 0.9) From 08bcf26c60c7ee1413226957176a2c8e2537ba58 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:46:03 -0500 Subject: [PATCH 26/39] Update slim-rails from 3.1.3 to 3.2.0 --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index b568698e291..74fac9bee92 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -557,13 +557,13 @@ GEM rack (~> 2.0) rack-protection (= 2.0.4) tilt (~> 2.0) - slim (3.0.9) + slim (4.0.1) temple (>= 0.7.6, < 0.9) - tilt (>= 1.3.3, < 2.1) - slim-rails (3.1.3) + tilt (>= 2.0.6, < 2.1) + slim-rails (3.2.0) actionpack (>= 3.1) railties (>= 3.1) - slim (~> 3.0) + slim (>= 3.0, < 5.0) slim_lint (0.16.0) rake (>= 10, < 13) rubocop (>= 0.50.0) From cc184d106f3641e98cd184d21901da483e0ff2e9 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:46:13 -0500 Subject: [PATCH 27/39] Update slim_lint from 0.16.0 to 0.16.1 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 74fac9bee92..497f095e22e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -564,7 +564,7 @@ GEM actionpack (>= 3.1) railties (>= 3.1) slim (>= 3.0, < 5.0) - slim_lint (0.16.0) + slim_lint (0.16.1) rake (>= 10, < 13) rubocop (>= 0.50.0) slim (>= 3.0, < 5.0) From 8611fadbdb3e86f73e78f542cfb2fd07154ac132 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:46:23 -0500 Subject: [PATCH 28/39] Update stringex from 2.8.4 to 2.8.5 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 497f095e22e..d63cd281dd3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -577,7 +577,7 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - stringex (2.8.4) + stringex (2.8.5) strong_migrations (0.2.3) activerecord (>= 3.2.0) sysexits (1.2.0) From 7e0d36e2c09e476384769314f061d33e3b10df3f Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:46:37 -0500 Subject: [PATCH 29/39] Update strong_migrations from 0.2.3 to 0.3.1 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index d63cd281dd3..0c323158582 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -578,7 +578,7 @@ GEM activesupport (>= 4.0) sprockets (>= 3.0.0) stringex (2.8.5) - strong_migrations (0.2.3) + strong_migrations (0.3.1) activerecord (>= 3.2.0) sysexits (1.2.0) systemu (2.6.5) From 57a8dde75fdbfdc9206bc4111ff0d1fbbb0ae688 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 19:46:42 -0500 Subject: [PATCH 30/39] LG-732 Add security key SVG to setup page (#2647) **Why**: To provide a visual explanation of "security key". --- app/assets/images/security-key.svg | 1 + app/views/users/webauthn_setup/new.html.slim | 1 + 2 files changed, 2 insertions(+) create mode 100644 app/assets/images/security-key.svg diff --git a/app/assets/images/security-key.svg b/app/assets/images/security-key.svg new file mode 100644 index 00000000000..0e609bd4a54 --- /dev/null +++ b/app/assets/images/security-key.svg @@ -0,0 +1 @@ +security-key \ No newline at end of file diff --git a/app/views/users/webauthn_setup/new.html.slim b/app/views/users/webauthn_setup/new.html.slim index e3b7d09b62f..7c736094c04 100644 --- a/app/views/users/webauthn_setup/new.html.slim +++ b/app/views/users/webauthn_setup/new.html.slim @@ -2,6 +2,7 @@ - help_link = link_to t('links.what_is_webauthn'), MarketingSite.help_hardware_security_key_url, target: :_blank += image_tag asset_url('security-key.svg'), width: '90', class: 'ml1' h1.h3.my0 = t('headings.webauthn_setup.new') p.mt-tiny.mb3 = t('forms.webauthn_setup.intro_html', link: help_link) div From ef42f48158d019f6da719de403ef1c3eb6003fce Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:46:49 -0500 Subject: [PATCH 31/39] Update twilio-ruby from 5.12.4 to 5.15.2 --- Gemfile.lock | 4 ++-- spec/services/phone_verification_spec.rb | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 0c323158582..900d4ae4673 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -252,7 +252,7 @@ GEM fakefs (0.18.0) faker (1.9.1) i18n (>= 0.7) - faraday (0.15.2) + faraday (0.15.3) multipart-post (>= 1.2, < 3) fasterer (0.4.1) colorize (~> 0.7) @@ -593,7 +593,7 @@ GEM thread_safe (0.3.6) tilt (2.0.8) timecop (0.9.1) - twilio-ruby (5.12.4) + twilio-ruby (5.15.2) faraday (~> 0.9) jwt (>= 1.5, <= 2.5) nokogiri (>= 1.6, < 2.0) diff --git a/spec/services/phone_verification_spec.rb b/spec/services/phone_verification_spec.rb index b771ba20d05..a67a0ec43d7 100644 --- a/spec/services/phone_verification_spec.rb +++ b/spec/services/phone_verification_spec.rb @@ -43,8 +43,14 @@ PhoneVerification.adapter = Faraday.new(url: PhoneVerification::AUTHY_HOST) locale = 'fr' - body = "code_length=6&country_code=1&custom_code=#{code}&locale=#{locale}&" \ - "phone_number=7035551212&via=sms" + body = { + code_length: '6', + country_code: '1', + custom_code: code, + locale: locale, + phone_number: '7035551212', + via: 'sms', + } stub_request(:post, 'https://api.authy.com/protected/json/phones/verification/start'). with( @@ -53,7 +59,7 @@ 'Accept' => '*/*', 'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Content-Type' => 'application/x-www-form-urlencoded', - 'User-Agent' => 'Faraday v0.15.2', + 'User-Agent' => 'Faraday v0.15.3', 'X-Authy-Api-Key' => Figaro.env.twilio_verify_api_key, } ). From f52aba637088e4bf1f450721affc736baf173ca8 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:46:59 -0500 Subject: [PATCH 32/39] Update typhoeus from 1.3.0 to 1.3.1 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 900d4ae4673..a4dd5b740d9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -603,7 +603,7 @@ GEM rails (>= 3.1.1) randexp rotp (>= 3.2.0) - typhoeus (1.3.0) + typhoeus (1.3.1) ethon (>= 0.9.0) tzinfo (1.2.5) thread_safe (~> 0.1) From bb51fc32ce8b38b6c9b33e085b9108de289adee7 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:47:10 -0500 Subject: [PATCH 33/39] Update valid_email from 0.1.0 to 0.1.2 --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a4dd5b740d9..e2668042a2d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -344,7 +344,7 @@ GEM lumberjack (1.0.13) macaddr (1.7.1) systemu (~> 2.6.2) - mail (2.7.0) + mail (2.7.1) mini_mime (>= 0.1.1) memory_profiler (0.9.11) method_source (0.9.2) @@ -614,7 +614,7 @@ GEM user_agent_parser (2.4.1) uuid (2.3.9) macaddr (~> 1.0) - valid_email (0.1.0) + valid_email (0.1.2) activemodel mail (>= 2.6.1) virtus (1.0.5) From 95ad062b38b8e614e80a6b347d56ac3b9a3d1558 Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 13 Nov 2018 15:47:51 -0500 Subject: [PATCH 34/39] Update webauthn from 1.3.0 to 1.7.0 --- Gemfile.lock | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index e2668042a2d..4e7be6be44c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -369,6 +369,7 @@ GEM notiffany (0.1.1) nenv (~> 0.1) shellany (~> 0.0) + openssl (2.1.2) orm_adapter (0.5.0) overcommit (0.46.0) childprocess (~> 0.6, >= 0.6.3) @@ -627,9 +628,11 @@ GEM wasabi (3.5.0) httpi (~> 2.0) nokogiri (>= 1.4.2) - webauthn (1.3.0) + webauthn (1.7.0) cbor (~> 0.5.9.2) cose (~> 0.1.0) + jwt (>= 1.5, < 3.0) + openssl (~> 2.0) webmock (3.4.2) addressable (>= 2.3.6) crack (>= 0.3.2) From 0ef41d91230c8d201ec9d7ea85f55afe1d0c8dd8 Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Thu, 15 Nov 2018 14:48:27 -0500 Subject: [PATCH 35/39] LG-742 Create password screen allows less than 12 characters (#2657) **Why**: As a user I don't want to be allowed to enter less than 12 characters for a new password even though it returns an error after I submit. **How**: Update the pw-strength.js to check for password length. --- app/javascript/packs/pw-strength.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/javascript/packs/pw-strength.js b/app/javascript/packs/pw-strength.js index 12c05851403..126adfb89a5 100644 --- a/app/javascript/packs/pw-strength.js +++ b/app/javascript/packs/pw-strength.js @@ -36,10 +36,10 @@ function getFeedback(z) { return `${suggestions.map(s => lookup(s)).join('')}`; } -function disableSubmit(submitEl, score = 0) { +function disableSubmit(submitEl, length = 0, score = 0) { if (!submitEl) return; - if (score < 3) { + if (score < 3 || length < 12) { submitEl.setAttribute('disabled', true); } else { submitEl.removeAttribute('disabled'); @@ -73,7 +73,7 @@ function analyzePw() { pwStrength.innerHTML = strength; pwFeedback.innerHTML = feedback; - disableSubmit(submit, z.score); + disableSubmit(submit, z.password.length, z.score); } if (/(msie 9)/i.test(userAgent)) { From f4274066469fb5541076e5b0d51f579123e4e9c1 Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Thu, 15 Nov 2018 18:48:26 -0500 Subject: [PATCH 36/39] LG-795 Display timestamps in the local timezone (#2654) **Why**: As a user I want to see events in my account history in my local timezone **How**: Since we do not store a user's timezone, update timestamps to the user's local timezone by adding client side javascript which uses the timezone of the browser. Use the local_time gem. Set the locale in the javascript by grabbing it from the path in the url. Add bug fixes and internationalization. --- .rubocop.yml | 1 + Gemfile | 1 + Gemfile.lock | 2 + app/decorators/event_decorator.rb | 2 +- app/decorators/identity_decorator.rb | 8 +- app/javascript/app/local-time.js | 108 +++++++++++++++++++ app/javascript/packs/application.js | 1 + app/presenters/utc_time_presenter.rb | 2 +- app/views/accounts/_connected_app.html.slim | 3 +- app/views/accounts/_event_item.html.slim | 2 +- app/views/accounts/_identity_item.html.slim | 2 +- config/i18n-tasks.yml | 1 + config/locales/account/en.yml | 2 +- config/locales/account/es.yml | 2 +- config/locales/account/fr.yml | 2 +- spec/features/account_connected_apps_spec.rb | 3 +- spec/presenters/utc_time_presenter_spec.rb | 2 +- 17 files changed, 131 insertions(+), 13 deletions(-) create mode 100644 app/javascript/app/local-time.js diff --git a/.rubocop.yml b/.rubocop.yml index 11d3b732a5e..e962e136b39 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -36,6 +36,7 @@ Metrics/BlockLength: Exclude: - 'Rakefile' - '**/*.rake' + - 'app/decorators/identity_decorator.rb' - 'app/decorators/user_decorator.rb' - 'app/services/omniauth_authorizer.rb' - 'app/services/single_logout_handler.rb' diff --git a/Gemfile b/Gemfile index b113cf1ea60..a5bace60a8e 100644 --- a/Gemfile +++ b/Gemfile @@ -25,6 +25,7 @@ gem 'http_accept_language' gem 'httparty' gem 'identity-hostdata', github: '18F/identity-hostdata', branch: 'master' gem 'json-jwt' +gem 'local_time' gem 'lograge' gem 'net-sftp' gem 'newrelic_rpm' diff --git a/Gemfile.lock b/Gemfile.lock index 4e7be6be44c..92e177232b4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -333,6 +333,7 @@ GEM rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) ruby_dep (~> 1.2) + local_time (2.1.0) lograge (0.10.0) actionpack (>= 4) activesupport (>= 4) @@ -707,6 +708,7 @@ DEPENDENCIES json-jwt knapsack lexisnexis! + local_time lograge net-sftp newrelic_rpm diff --git a/app/decorators/event_decorator.rb b/app/decorators/event_decorator.rb index 255747154e6..3937b08c368 100644 --- a/app/decorators/event_decorator.rb +++ b/app/decorators/event_decorator.rb @@ -8,7 +8,7 @@ def event_type end def happened_at - event.created_at + event.created_at.utc end def happened_at_in_words diff --git a/app/decorators/identity_decorator.rb b/app/decorators/identity_decorator.rb index 63c65cc6eeb..15d9544be6a 100644 --- a/app/decorators/identity_decorator.rb +++ b/app/decorators/identity_decorator.rb @@ -19,11 +19,15 @@ def return_to_sp_url end def created_at_in_words - UtcTimePresenter.new(identity.created_at).to_s + UtcTimePresenter.new(created_at).to_s + end + + def created_at + identity.created_at.utc end def happened_at - identity.last_authenticated_at + identity.last_authenticated_at.utc end def happened_at_in_words diff --git a/app/javascript/app/local-time.js b/app/javascript/app/local-time.js new file mode 100644 index 00000000000..f3689498693 --- /dev/null +++ b/app/javascript/app/local-time.js @@ -0,0 +1,108 @@ +// modifications marked with "login.gov" original here: https://github.com/basecamp/local_time/blob/master/app/assets/javascripts/local-time.js + +(function(){var t=this;(function(){(function(){var t=[].slice; + +// login.gov +window.LocalTime={ + + config:{},run:function(){return this.getController().processElements()},process:function(){var e,n,r,a;for(n=1<=arguments.length?t.call(arguments,0):[],r=0,a=n.length;r11?"pm":"am")).toUpperCase();case"P":return i("time."+(c>11?"pm":"am"));case"S":return n(h,m);case"w":return u;case"y":return n(f%100,m);case"Y":return f;case"Z":return r(e)}})},n=function(t,e){switch(e){case"-":return t;default:return("0"+t).slice(-2)}},r=function(t){var e,n,r,a,i;return i=t.toString(),(e=null!=(n=i.match(/\(([\w\s]+)\)$/))?n[1]:void 0)?/\s/.test(e)?e.match(/\b(\w)/g).join(""):e:(e=null!=(r=i.match(/(\w{3,4})\s\d{4}$/))?r[1]:void 0)?e:(e=null!=(a=i.match(/(UTC[\+\-]\d+)/))?a[1]:void 0)?e:""}}.call(this),function(){e.CalendarDate=function(){function t(t,e,n){this.date=new Date(Date.UTC(t,e-1)),this.date.setUTCDate(n),this.year=this.date.getUTCFullYear(),this.month=this.date.getUTCMonth()+1,this.day=this.date.getUTCDate(),this.value=this.date.getTime()}return t.fromDate=function(t){return new this(t.getFullYear(),t.getMonth()+1,t.getDate())},t.today=function(){return this.fromDate(new Date)},t.prototype.equals=function(t){return(null!=t?t.value:void 0)===this.value},t.prototype.is=function(t){return this.equals(t)},t.prototype.isToday=function(){return this.is(this.constructor.today())},t.prototype.occursOnSameYearAs=function(t){return this.year===(null!=t?t.year:void 0)},t.prototype.occursThisYear=function(){return this.occursOnSameYearAs(this.constructor.today())},t.prototype.daysSince=function(t){if(t)return(this.date-t.date)/864e5},t.prototype.daysPassed=function(){return this.constructor.today().daysSince(this)},t}()}.call(this),function(){var t,n,r;n=e.strftime,r=e.translate,t=e.getI18nValue,e.RelativeTime=function(){function a(t){this.date=t,this.calendarDate=e.CalendarDate.fromDate(this.date)}return a.prototype.toString=function(){var t,e;return(e=this.toTimeElapsedString())?r("time.elapsed",{time:e}):(t=this.toWeekdayString())?(e=this.toTimeString(),r("datetime.at",{date:t,time:e})):r("date.on",{date:this.toDateString()})},a.prototype.toTimeOrDateString=function(){return this.calendarDate.isToday()?this.toTimeString():this.toDateString()},a.prototype.toTimeElapsedString=function(){var t,e,n,a,i;return n=(new Date).getTime()-this.date.getTime(),a=Math.round(n/1e3),e=Math.round(a/60),t=Math.round(e/60),n<0?null:a<10?(i=r("time.second"),r("time.singular",{time:i})):a<45?a+" "+r("time.seconds"):a<90?(i=r("time.minute"),r("time.singular",{time:i})):e<45?e+" "+r("time.minutes"):e<90?(i=r("time.hour"),r("time.singularAn",{time:i})):t<24?t+" "+r("time.hours"):""},a.prototype.toWeekdayString=function(){switch(this.calendarDate.daysPassed()){case 0:return r("date.today");case 1:return r("date.yesterday");case-1:return r("date.tomorrow");case 2:case 3:case 4:case 5:case 6:return n(this.date,"%A");default:return""}},a.prototype.toDateString=function(){var e;return e=t(this.calendarDate.occursThisYear()?"date.formats.thisYear":"date.formats.default"),n(this.date,e)},a.prototype.toTimeString=function(){return n(this.date,t("time.formats.default"))},a}()}.call(this),function(){var t,n=function(t,e){return function(){return t.apply(e,arguments)}};t=e.elementMatchesSelector,e.PageObserver=function(){function e(t,e){this.selector=t,this.callback=e,this.processInsertion=n(this.processInsertion,this),this.processMutations=n(this.processMutations,this)}return e.prototype.start=function(){if(!this.started)return this.observeWithMutationObserver()||this.observeWithMutationEvent(),this.started=!0},e.prototype.observeWithMutationObserver=function(){var t;if("undefined"!=typeof MutationObserver&&null!==MutationObserver)return t=new MutationObserver(this.processMutations),t.observe(document.documentElement,{childList:!0,subtree:!0}),!0},e.prototype.observeWithMutationEvent=function(){return addEventListener("DOMNodeInserted",this.processInsertion,!1),!0},e.prototype.findSignificantElements=function(e){var n;return n=[],(null!=e?e.nodeType:void 0)===Node.ELEMENT_NODE&&(t(e,this.selector)&&n.push(e),n.push.apply(n,e.querySelectorAll(this.selector))),n},e.prototype.processMutations=function(t){var e,n,r,a,i,o,s,u;for(e=[],n=0,a=t.length;n Date: Fri, 16 Nov 2018 10:51:43 -0500 Subject: [PATCH 37/39] LG-583 Update account reset final delete screen design (#2652) **Why**: To make the delete option less prominent since the user will not be able to recover any information linked to their account. **How**: Use the same format as the confirmation of delete screen at the beginning of the process. --- .../account_reset/delete_account/show.html.slim | 12 +++++------- config/locales/account_reset/en.yml | 2 -- config/locales/account_reset/es.yml | 2 -- config/locales/account_reset/fr.yml | 2 -- spec/features/account_reset/delete_account_spec.rb | 2 +- .../delete_account/show.html.slim_spec.rb | 2 +- 6 files changed, 7 insertions(+), 15 deletions(-) diff --git a/app/views/account_reset/delete_account/show.html.slim b/app/views/account_reset/delete_account/show.html.slim index 0ad05055252..f74626f8f24 100644 --- a/app/views/account_reset/delete_account/show.html.slim +++ b/app/views/account_reset/delete_account/show.html.slim @@ -6,10 +6,8 @@ p.mt-tiny.mb0 br h4.my2 = t('account_reset.delete_account.are_you_sure') -= button_to t('account_reset.delete_account.delete_button'), \ - account_reset_delete_account_path, method: :delete, \ - class: 'btn btn-red col-6 p2 rounded-lg border bw2 bg-lightest-red border-red border-box' -br -br -hr -= link_to t('account_reset.delete_account.cancel'), root_url += button_to t('account_reset.request.no_cancel'), root_url, method: :get, + class: 'btn btn-primary btn-wide mb1 personal-key-continue', + 'data-toggle': 'modal' += button_to t('account_reset.request.yes_continue'), account_reset_delete_account_path, \ + method: :delete, class: 'btn btn-link' diff --git a/config/locales/account_reset/en.yml b/config/locales/account_reset/en.yml index aa5ca0313a3..45d5f44a951 100644 --- a/config/locales/account_reset/en.yml +++ b/config/locales/account_reset/en.yml @@ -21,8 +21,6 @@ en: phone number. delete_account: are_you_sure: Are you sure you want to delete your account? - cancel: Cancel - delete_button: Delete account info: Deleting your account should be your last resort if you are locked out of your account. You will not be able to recover any information linked to your account. Once your account is deleted, you can create a new one using diff --git a/config/locales/account_reset/es.yml b/config/locales/account_reset/es.yml index ec756dff785..216599f027d 100644 --- a/config/locales/account_reset/es.yml +++ b/config/locales/account_reset/es.yml @@ -22,8 +22,6 @@ es: a su registro número de teléfono. delete_account: are_you_sure: "¿Seguro que quieres eliminar tu cuenta?" - cancel: Cancelar - delete_button: Borrar cuenta info: Eliminar su cuenta debe ser su último recurso si está bloqueado          de tu cuenta No podrá recuperar ninguna información vinculada a su cuenta. Una vez que se elimine su cuenta, puede crear una nueva usando la misma dirección diff --git a/config/locales/account_reset/fr.yml b/config/locales/account_reset/fr.yml index bbd0758b6d5..12bf48555c0 100644 --- a/config/locales/account_reset/fr.yml +++ b/config/locales/account_reset/fr.yml @@ -23,8 +23,6 @@ fr: votre numéro de téléphone enregistré. delete_account: are_you_sure: Êtes-vous sûr de vouloir supprimer votre compte? - cancel: Annuler - delete_button: Supprimer le compte info: La suppression de votre compte devrait être votre dernier recours si vous êtes en lock-out de votre compte Vous ne pourrez pas récupérer les informations liées à ton compte. Une fois votre compte supprimé, vous pouvez en créer un diff --git a/spec/features/account_reset/delete_account_spec.rb b/spec/features/account_reset/delete_account_spec.rb index ef4d0461664..fbe44556a09 100644 --- a/spec/features/account_reset/delete_account_spec.rb +++ b/spec/features/account_reset/delete_account_spec.rb @@ -32,7 +32,7 @@ expect(page).to have_content(t('account_reset.delete_account.title')) expect(page).to have_current_path(account_reset_delete_account_path) - click_on t('account_reset.delete_account.delete_button') + click_button t('account_reset.request.yes_continue') expect(page).to have_content( strip_tags( diff --git a/spec/views/account_reset/delete_account/show.html.slim_spec.rb b/spec/views/account_reset/delete_account/show.html.slim_spec.rb index bd5573e0aba..eb22b80c76b 100644 --- a/spec/views/account_reset/delete_account/show.html.slim_spec.rb +++ b/spec/views/account_reset/delete_account/show.html.slim_spec.rb @@ -13,6 +13,6 @@ it 'has button to delete' do render - expect(rendered).to have_button t('account_reset.delete_account.delete_button') + expect(rendered).to have_button t('account_reset.request.yes_continue') end end From 8c2d6ec3d42d57093b5ebb81f99176e8dc848ae2 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Fri, 16 Nov 2018 15:15:28 -0600 Subject: [PATCH 38/39] LG-785 Alert users when personal key is regenerated (#2660) **Why**: So that user's can be aware of any potential suspicious activity associated with their account. --- .../users/personal_keys_controller.rb | 10 +++++++ ..._personal_key_regeneration_notifier_job.rb | 14 +++++++++ app/mailers/user_mailer.rb | 4 +++ .../personal_key_regenerated.html.slim | 16 ++++++++++ config/locales/jobs/en.yml | 7 +++-- config/locales/jobs/es.yml | 5 +++- config/locales/jobs/fr.yml | 6 +++- config/locales/user_mailer/en.yml | 26 +++++++++++++--- config/locales/user_mailer/es.yml | 21 +++++++++++++ config/locales/user_mailer/fr.yml | 21 +++++++++++++ .../users/regenerate_personal_key_spec.rb | 19 ++++++++++-- ...onal_key_regeneration_notifier_job_spec.rb | 30 +++++++++++++++++++ spec/mailers/user_mailer_spec.rb | 20 +++++++++++++ 13 files changed, 189 insertions(+), 10 deletions(-) create mode 100644 app/jobs/sms_personal_key_regeneration_notifier_job.rb create mode 100644 app/views/user_mailer/personal_key_regenerated.html.slim create mode 100644 spec/jobs/sms_personal_key_regeneration_notifier_job_spec.rb diff --git a/app/controllers/users/personal_keys_controller.rb b/app/controllers/users/personal_keys_controller.rb index 72f2b13d462..4dfccebce70 100644 --- a/app/controllers/users/personal_keys_controller.rb +++ b/app/controllers/users/personal_keys_controller.rb @@ -27,6 +27,7 @@ def create user_session[:personal_key] = create_new_code analytics.track_event(Analytics::PROFILE_PERSONAL_KEY_CREATE) Event.create(user_id: current_user.id, event_type: :new_personal_key) + send_new_personal_key_notification redirect_to manage_personal_key_url end @@ -45,5 +46,14 @@ def next_step def user_has_not_visited_any_sp_yet? current_user.identities.pluck(:last_authenticated_at).compact.empty? end + + def send_new_personal_key_notification + current_user.confirmed_email_addresses.each do |email_address| + UserMailer.personal_key_regenerated(email_address.email).deliver_now + end + MfaContext.new(current_user).phone_configurations.each do |phone_configuration| + SmsPersonalKeyRegenerationNotifierJob.perform_now(phone: phone_configuration.phone) + end + end end end diff --git a/app/jobs/sms_personal_key_regeneration_notifier_job.rb b/app/jobs/sms_personal_key_regeneration_notifier_job.rb new file mode 100644 index 00000000000..3ac8db56757 --- /dev/null +++ b/app/jobs/sms_personal_key_regeneration_notifier_job.rb @@ -0,0 +1,14 @@ +class SmsPersonalKeyRegenerationNotifierJob < ApplicationJob + queue_as :sms + + # :reek:UtilityFunction + def perform(phone:) + TwilioService::Utils.new.send_sms( + to: phone, + body: I18n.t( + 'jobs.sms_personal_key_regeneration_notifier_job.message', + app: APP_NAME + ) + ) + end +end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index dcf0044e6a0..f6461808faa 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -31,6 +31,10 @@ def personal_key_sign_in(email) mail(to: email, subject: t('user_mailer.personal_key_sign_in.subject')) end + def personal_key_regenerated(email) + mail(to: email, subject: t('user_mailer.personal_key_regenerated.subject')) + end + def account_reset_request(email_address, account_reset) @token = account_reset&.request_token mail(to: email_address.email, subject: t('user_mailer.account_reset_request.subject')) diff --git a/app/views/user_mailer/personal_key_regenerated.html.slim b/app/views/user_mailer/personal_key_regenerated.html.slim new file mode 100644 index 00000000000..9ae7e2becc0 --- /dev/null +++ b/app/views/user_mailer/personal_key_regenerated.html.slim @@ -0,0 +1,16 @@ +p.lead + strong == t('.intro') + +table.spacer + tbody + tr + td.s10 height="10px" + |   +table.hr + tr + th + |   + +p == t('.help_html', + reset_password_url: forgot_password_url, + account_url: account_url) diff --git a/config/locales/jobs/en.yml b/config/locales/jobs/en.yml index 864b89c864a..f74ba0a79ba 100644 --- a/config/locales/jobs/en.yml +++ b/config/locales/jobs/en.yml @@ -12,6 +12,9 @@ en: in to your account. This code will expire in %{expiration} minutes." verify_message: "%{code} is your %{app} confirmation code. Use this to confirm your phone number. This code will expire in %{expiration} minutes." + sms_personal_key_regeneration_notifier_job: + message: A new personal key has been issued for your %{app} account. If this + wasn’t you, reset your password and contact us at security@login.gov. sms_personal_key_sign_in_notifier_job: - message: Your personal key was just used to sign into your login.gov account. - If this wasn’t you, reset your password and contact us at security@login.gov. + message: Your personal key was just used to sign into your %{app} account. If + this wasn’t you, reset your password and contact us at security@login.gov. diff --git a/config/locales/jobs/es.yml b/config/locales/jobs/es.yml index d2aec114d8d..8d29586838d 100644 --- a/config/locales/jobs/es.yml +++ b/config/locales/jobs/es.yml @@ -12,6 +12,9 @@ es: ingresando a su cuenta. Este código caducará en %{expiration} minutos." verify_message: "%{code} es tu código de confirmación de %{app}. Use esto para confirmar su número de teléfono. Este código caducará en %{expiration} minutos." + sms_personal_key_regeneration_notifier_job: + message: Se ha emitido una nueva clave personal para tu cuenta %{app}. Si no + eres tú, restablece tu contraseña y ponte en contacto con nosotros en security@login.gov. sms_personal_key_sign_in_notifier_job: message: Su clave personal solo se utilizó para iniciar sesión en su cuenta - login.gov. Si no fue así, reinicie su contraseña y contáctenos en security@login.gov. + %{app}. Si no fue así, reinicie su contraseña y contáctenos en security@login.gov. diff --git a/config/locales/jobs/fr.yml b/config/locales/jobs/fr.yml index a2aa952d6ab..f4302b8bc74 100644 --- a/config/locales/jobs/fr.yml +++ b/config/locales/jobs/fr.yml @@ -14,7 +14,11 @@ fr: verify_message: "%{code} est votre code de confirmation %{app}. Utilisez-le pour confirmer votre numéro de téléphone. Ce code expirera dans %{expiration} minutes." + sms_personal_key_regeneration_notifier_job: + message: Une nouvelle clé personnelle a été émise pour votre compte %{app}. + Si vous ne l'avez pas demandée, réinitialisez votre mot de passe et contactez-nous + à security@login.gov. sms_personal_key_sign_in_notifier_job: message: Votre clé personnelle a été utilisée pour vous connecter à votre compte - login.gov. Si ce n’était pas vous, changez votre mot de passe et contactez-nous + %{app}. Si ce n’était pas vous, changez votre mot de passe et contactez-nous à security@login.gov. diff --git a/config/locales/user_mailer/en.yml b/config/locales/user_mailer/en.yml index aaf2a4491e0..55f0e59a478 100644 --- a/config/locales/user_mailer/en.yml +++ b/config/locales/user_mailer/en.yml @@ -44,6 +44,24 @@ en: help: If you did not make this change, please visit the %{app} %{help_link} or %{contact_link}. intro: You have a new password for your %{app} account. + personal_key_regenerated: + help_html:

Your login.gov account was just issued a new 16-character personal + key. You're getting this email to make sure it was you.

If you just + signed in and regenerated your personal key, great! There's nothing you need + to do.

If you did not just regenerate your personal key, or if you're + not sure, please immediately take these steps to secure your account:

  1. Change your password. Choose a password + that you haven't already used with this account.
  2. Sign + in to your login.gov account and make sure you recognize all + of the information on your account page, including the methods you use for + two-factor authentication, such as phone number, authentication app, or security + key.
  3. On your login.gov account page, + request a new personal key. Remember, never share it unless you are + using it to sign into a trusted website that uses login.gov.

You + should then contact us by calling 844-875-6446 or emailing security@login.gov.


Thanks,
The + login.gov team + intro: New personal key issued + subject: Account Security Alert personal_key_sign_in: help_html:

Your login.gov account was just signed into using your 16-character personal key. You're getting this email to make sure it was you.

If @@ -55,10 +73,10 @@ en: in to your login.gov account and make sure you recognize all of the information on your account page, including the methods you use for two-factor authentication, such as phone number, authentication app, or security - key.

  • On your login.gov account page, request a new personal - key. Remember, never share it unless you are using it to sign into - a trusted website that uses login.gov.

    • You should then contact - us by calling 844-875-6446 or emailing security@login.gov.

    + key.
  • On your login.gov account page, + request a new personal key. Remember, never share it unless you are + using it to sign into a trusted website that uses login.gov.

    • You + should then contact us by calling 844-875-6446 or emailing security@login.gov.


    Thanks,
    The login.gov team intro: Personal key used to sign in subject: Account Security Alert diff --git a/config/locales/user_mailer/es.yml b/config/locales/user_mailer/es.yml index eb4eb6a58dc..5cc4f6ba901 100644 --- a/config/locales/user_mailer/es.yml +++ b/config/locales/user_mailer/es.yml @@ -43,6 +43,27 @@ es: password_changed: help: Si no realizó este cambio, visite el %{app} %{help_link} o el %{contact_link}. intro: Tiene una contraseña nueva para su cuenta de %{app}. + personal_key_regenerated: + help_html:

    Tu cuenta de login.gov acaba de emitir una nueva clave personal + de 16 caracteres. Estás recibiendo este correo electrónico para verificar + que eras tú.

    Si acabas de iniciar sesión y has regenerado tu clave + personal, ¡fantástico! No es necesario que hagas nada.

    Si no acabas + de regenerar tu clave personal, o si no estás seguro, sigue estos pasos para + proteger tu cuenta:

    1. Cambia + tu contraseña. Elige una contraseña que aún no hayas utilizado + con esta cuenta.
    2. Inicia sesión en + tu cuenta de login.gov y asegúrate de que reconoces toda la información + de la página de tu cuenta, como los métodos que utilizas para la autenticación + de dos factores, como el número de teléfono, la aplicación de autenticación + o la clave de seguridad.
    3. En la página + de tu cuenta de login.gov, solicita una nueva clave personal. + Recuerda no compartirla nunca a menos que la estés usando para acceder a un + sitio web de confianza que utilice login.gov.

    Deberías ponerte + en contacto con nosotros llamando al 844-875-6446 o enviando un correo electrónico + a security@login.gov.


    Gracias,
    El + equipo de login.gov + intro: Nueva clave personal emitida + subject: Alerta de seguridad de la cuenta personal_key_sign_in: help_html:

    Su cuenta login.gov acaba de iniciar sesión con su clave personal de 16 caracteres. Usted está recibiendo este e-mail para asegurarse de que diff --git a/config/locales/user_mailer/fr.yml b/config/locales/user_mailer/fr.yml index 1c386e50dba..5e6e73b709c 100644 --- a/config/locales/user_mailer/fr.yml +++ b/config/locales/user_mailer/fr.yml @@ -45,6 +45,27 @@ fr: help: Si vous n'avez pas changé votre mot de passe, veuillez visiter le %{help_link} de %{app} ou %{contact_link}. intro: Le mot de passe de votre compte %{app} a été changé. + personal_key_regenerated: + help_html:

    Votre compte login.gov vient de recevoir une nouvelle clé personnelle + de 16 caractères. Le but de cet e-mail est de s'assurer que c'est bien vous + qui en êtes à l'origine.

    Si vous venez de vous connecter et de régénérer + votre clé personnelle, c'est parfait ! Vous n'avez rien à faire.

    Si + vous ne venez pas de régénérer votre clé personnelle, ou en cas de doute, + effectuez immédiatement les actions suivantes pour sécuriser votre compte :

    1. Modifiez votre mot de passe. Choisissez + un mot de passe que vous n'avez pas encore utilisé avec ce compte.
    2. Connectez-vous à votre compte login.gov + et vérifiez bien que toutes les informations sur la page de votre compte sont + correctes, y compris les méthodes que vous utilisez pour l’authentification + à deux facteurs, dont le numéro de téléphone, l’application d’authentification + ou la clé de sécurité.
    3. Sur votre page + de compte login.gov, demandez une nouvelle clé personnelle. N'oubliez + pas de ne jamais la partager, sauf si vous l'utilisez pour vous connecter + à un site de confiance qui utilise login.gov.

    Veuillez ensuite + nous contacter en appelant le 844-875-6446 ou par e-mail à security@login.gov.


    Merci,
    L'équipe + login.gov + intro: Nouvelle clé personnelle émise + subject: Alerte de sécurité du compte personal_key_sign_in: help_html:

    Votre compte login.gov a été connecté à l'aide de votre clé personnelle. Vous recevez cet email pour vous assurer que c'était bien vous.

    Si vous diff --git a/spec/features/users/regenerate_personal_key_spec.rb b/spec/features/users/regenerate_personal_key_spec.rb index 88f9887669c..1bceac55cd6 100644 --- a/spec/features/users/regenerate_personal_key_spec.rb +++ b/spec/features/users/regenerate_personal_key_spec.rb @@ -5,12 +5,18 @@ include PersonalKeyHelper include SamlAuthHelper + before { stub_twilio_service } + context 'during sign up' do - scenario 'user refreshes personal key page' do + scenario 'refreshing personal key page displays the same key and does not notify the user' do sign_up_and_view_personal_key personal_key = scrape_personal_key + # The user should not receive an SMS and an email + expect(UserMailer).to_not receive(:personal_key_regenerated) + expect(SmsPersonalKeyRegenerationNotifierJob).to_not receive(:perform_now) + visit sign_up_personal_key_path expect(current_path).to eq(sign_up_personal_key_path) @@ -24,10 +30,19 @@ context 'after sign up' do context 'regenerating personal key' do - scenario 'displays new code' do + scenario 'displays new code and notifies the user' do user = sign_in_and_2fa_user old_digest = user.encrypted_recovery_code_digest + # The user should receive an SMS and an email + personal_key_sign_in_mail = double + expect(personal_key_sign_in_mail).to receive(:deliver_now) + expect(UserMailer).to receive(:personal_key_regenerated). + with(user.email). + and_return(personal_key_sign_in_mail) + expect(SmsPersonalKeyRegenerationNotifierJob).to receive(:perform_now). + with(phone: user.phone_configurations.first.phone) + click_button t('account.links.regenerate_personal_key') expect(user.reload.encrypted_recovery_code_digest).to_not eq old_digest diff --git a/spec/jobs/sms_personal_key_regeneration_notifier_job_spec.rb b/spec/jobs/sms_personal_key_regeneration_notifier_job_spec.rb new file mode 100644 index 00000000000..68a1763527b --- /dev/null +++ b/spec/jobs/sms_personal_key_regeneration_notifier_job_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +describe SmsPersonalKeyRegenerationNotifierJob do + include Features::ActiveJobHelper + + before do + reset_job_queues + TwilioService::Utils.telephony_service = FakeSms + FakeSms.messages = [] + end + + describe '.perform' do + it 'sends a message about the personal key sign in to the user' do + allow(Figaro.env).to receive(:twilio_messaging_service_sid).and_return('fake_sid') + + described_class.perform_now(phone: '+1 (202) 345-6789') + + messages = FakeSms.messages + + expect(messages.size).to eq(1) + + msg = messages.first + + expect(msg.messaging_service_sid).to eq('fake_sid') + expect(msg.to).to eq('+1 (202) 345-6789') + expect(msg.body). + to eq(I18n.t('jobs.sms_personal_key_regeneration_notifier_job.message', app: APP_NAME)) + end + end +end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 2ca18021e43..91a91a4fb4d 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -66,6 +66,26 @@ end end + describe 'personal_key_regenerated' do + let(:mail) { UserMailer.personal_key_regenerated(user.email) } + + it_behaves_like 'a system email' + + it 'sends to the current email' do + expect(mail.to).to eq [user.email] + end + + it 'renders the subject' do + expect(mail.subject).to eq t('user_mailer.personal_key_regenerated.subject') + end + + it 'renders the body' do + expect(mail.html_part.body).to have_content( + t('user_mailer.personal_key_regenerated.intro') + ) + end + end + describe 'signup_with_your_email' do let(:mail) { UserMailer.signup_with_your_email(user.email) } From e265d2dd1b272c08866a03e98bd276bcee0ec905 Mon Sep 17 00:00:00 2001 From: Steve Urciuoli Date: Mon, 19 Nov 2018 14:31:38 -0500 Subject: [PATCH 39/39] LG-575 Add a phone (#2662) **Why**: We want the user to be able to add multiple phone numbers to their account without replacing any already configured. **How**: Replace the generic user.phone_configurations.first, which assumed one phone, with specific phone ids. Pass these ids when managing, updating, and authenticating. Create a new screen for adding a phone fashioned after the existing edit phone screen. --- .../concerns/phone_confirmation.rb | 8 ++--- .../concerns/two_factor_authenticatable.rb | 22 +++++++----- .../options_controller.rb | 7 +++- .../otp_verification_controller.rb | 2 +- .../users/phone_setup_controller.rb | 2 +- app/controllers/users/phones_controller.rb | 36 ++++++++++++++++--- .../two_factor_authentication_controller.rb | 5 +-- app/decorators/mfa_context.rb | 5 +++ app/forms/two_factor_login_options_form.rb | 16 ++++++--- app/forms/two_factor_options_form.rb | 2 +- .../otp_delivery_preference_updater.rb | 8 ++--- app/services/update_user.rb | 12 ++++--- app/views/accounts/_phone.html.slim | 7 ++-- app/views/users/phones/add.html.slim | 23 ++++++++++++ app/views/users/phones/edit.html.slim | 2 +- config/locales/headings/en.yml | 2 ++ config/locales/headings/es.yml | 2 ++ config/locales/headings/fr.yml | 2 ++ config/locales/titles/en.yml | 2 ++ config/locales/titles/es.yml | 2 ++ config/locales/titles/fr.yml | 2 ++ config/routes.rb | 2 ++ .../otp_verification_controller_spec.rb | 2 ++ .../users/phones_controller_spec.rb | 31 ++++++++++++++++ .../change_factor_spec.rb | 2 +- spec/features/users/sign_in_spec.rb | 2 +- .../otp_delivery_preference_updater_spec.rb | 6 ++-- spec/services/update_user_spec.rb | 15 -------- spec/views/accounts/show.html.slim_spec.rb | 12 +++---- 29 files changed, 174 insertions(+), 67 deletions(-) create mode 100644 app/views/users/phones/add.html.slim diff --git a/app/controllers/concerns/phone_confirmation.rb b/app/controllers/concerns/phone_confirmation.rb index ebf8df79f07..266a59df7f6 100644 --- a/app/controllers/concerns/phone_confirmation.rb +++ b/app/controllers/concerns/phone_confirmation.rb @@ -1,21 +1,21 @@ module PhoneConfirmation - def prompt_to_confirm_phone(phone:, selected_delivery_method: nil) + def prompt_to_confirm_phone(id:, phone:, selected_delivery_method: nil) user_session[:unconfirmed_phone] = phone user_session[:context] = 'confirmation' redirect_to otp_send_url( otp_delivery_selection_form: { - otp_delivery_preference: otp_delivery_method(phone, selected_delivery_method), + otp_delivery_preference: otp_delivery_method(id, phone, selected_delivery_method), } ) end private - def otp_delivery_method(phone, selected_delivery_method) + def otp_delivery_method(id, phone, selected_delivery_method) return :sms if PhoneNumberCapabilities.new(phone).sms_only? return selected_delivery_method if selected_delivery_method.present? - MfaContext.new(current_user).phone_configurations.first&.delivery_preference || + MfaContext.new(current_user).phone_configuration(id)&.delivery_preference || current_user.otp_delivery_preference end end diff --git a/app/controllers/concerns/two_factor_authenticatable.rb b/app/controllers/concerns/two_factor_authenticatable.rb index 52df8a0072f..6478ebfa0ce 100644 --- a/app/controllers/concerns/two_factor_authenticatable.rb +++ b/app/controllers/concerns/two_factor_authenticatable.rb @@ -131,7 +131,7 @@ def handle_valid_otp_for_authentication_context end def assign_phone - @updating_existing_number = old_phone.present? + @updating_existing_number = user_session[:phone_id].present? if @updating_existing_number && confirmation_context? phone_changed @@ -142,10 +142,6 @@ def assign_phone update_phone_attributes end - def old_phone - MfaContext.new(current_user).phone_configurations.first&.phone - end - def phone_changed create_user_event(:phone_changed) current_user.confirmed_email_addresses.each do |email_address| @@ -160,7 +156,8 @@ def phone_confirmed def update_phone_attributes UpdateUser.new( user: current_user, - attributes: { phone: user_session[:unconfirmed_phone], phone_confirmed_at: Time.zone.now } + attributes: { phone_id: user_session[:phone_id], phone: user_session[:unconfirmed_phone], + phone_confirmed_at: Time.zone.now } ).call end @@ -253,7 +250,7 @@ def generic_data def display_phone_to_deliver_to if authentication_context? - decorated_user.masked_two_factor_phone_number + masked_number(phone_configuration.phone) else user_session[:unconfirmed_phone] end @@ -261,7 +258,7 @@ def display_phone_to_deliver_to def voice_otp_delivery_unsupported? phone_number = if authentication_context? - MfaContext.new(current_user).phone_configurations.first&.phone + phone_configuration&.phone else user_session[:unconfirmed_phone] end @@ -297,4 +294,13 @@ def presenter_for_two_factor_authentication_method view: view_context ) end + + def phone_configuration + MfaContext.new(current_user).phone_configuration(user_session[:phone_id]) + end + + def masked_number(number) + return '' if number.blank? + "***-***-#{number[-4..-1]}" + end end diff --git a/app/controllers/two_factor_authentication/options_controller.rb b/app/controllers/two_factor_authentication/options_controller.rb index 60a5c9080c9..bba4b0fc10b 100644 --- a/app/controllers/two_factor_authentication/options_controller.rb +++ b/app/controllers/two_factor_authentication/options_controller.rb @@ -55,8 +55,13 @@ def mfa_redirect_url options = EXTRA_URL_OPTIONS[selection] || {} configuration_id = @two_factor_options_form.configuration_id - options[:id] = configuration_id if configuration_id.present? + user_session[:phone_id] = configuration_id if configuration_id.present? + options[:id] = user_session[:phone_id] + build_url(selection, options) + end + + def build_url(selection, options) method = FACTOR_TO_URL_METHOD[selection] public_send(method, options) if method.present? end diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 0de0813df51..e248d9c117c 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -58,7 +58,7 @@ def confirm_voice_capability end def phone - MfaContext.new(current_user).phone_configurations.first&.phone || + MfaContext.new(current_user).phone_configuration(user_session[:phone_id])&.phone || user_session[:unconfirmed_phone] end diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb index fdd95c94adf..574852e5ccd 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -21,7 +21,7 @@ def create analytics.track_event(Analytics::MULTI_FACTOR_AUTH_PHONE_SETUP, result.to_h) if result.success? - prompt_to_confirm_phone(phone: @user_phone_form.phone) + prompt_to_confirm_phone(id: nil, phone: @user_phone_form.phone) else render :index end diff --git a/app/controllers/users/phones_controller.rb b/app/controllers/users/phones_controller.rb index f32bcd2bce7..fa434197a11 100644 --- a/app/controllers/users/phones_controller.rb +++ b/app/controllers/users/phones_controller.rb @@ -4,7 +4,25 @@ class PhonesController < ReauthnRequiredController before_action :confirm_two_factor_authenticated + def add + user_session[:phone_id] = nil + @user_phone_form = UserPhoneForm.new(current_user, nil) + @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + end + + def create + @user_phone_form = UserPhoneForm.new(current_user, nil) + @presenter = PhoneSetupPresenter.new(current_user.otp_delivery_preference) + if @user_phone_form.submit(user_params).success? + confirm_phone + bypass_sign_in current_user + else + render :add + end + end + def edit + set_phone_id @user_phone_form = UserPhoneForm.new(current_user, phone_configuration) @presenter = PhoneSetupPresenter.new(delivery_preference) end @@ -40,7 +58,7 @@ def delete # doing away with this controller. Once we move to multiple phones, we'll allow # adding and deleting, but not editing. def phone_configuration - MfaContext.new(current_user).phone_configurations.first + MfaContext.new(current_user).phone_configuration(user_session[:phone_id]) end def user_params @@ -55,18 +73,26 @@ def process_updates form = @user_phone_form if form.phone_changed? analytics.track_event(Analytics::PHONE_CHANGE_REQUESTED) - flash[:notice] = t('devise.registrations.phone_update_needs_confirmation') - prompt_to_confirm_phone( - phone: form.phone, selected_delivery_method: form.otp_delivery_preference - ) + confirm_phone else redirect_to account_url end end + def confirm_phone + flash[:notice] = t('devise.registrations.phone_update_needs_confirmation') + prompt_to_confirm_phone(id: user_session[:phone_id], phone: @user_phone_form.phone, + selected_delivery_method: @user_phone_form.otp_delivery_preference) + end + def handle_successful_delete flash[:success] = t('two_factor_authentication.phone.delete.success') create_user_event(:phone_removed) end + + def set_phone_id + phone_id = params[:id] + user_session[:phone_id] = phone_id if phone_id.present? + end end end diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 89f31c2a2eb..988331434f8 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -31,7 +31,7 @@ def phone_enabled? end def phone_configuration - MfaContext.new(current_user).phone_configurations.first + MfaContext.new(current_user).phone_configuration(user_session[:phone_id]) end def validate_otp_delivery_preference_and_send_code @@ -51,10 +51,11 @@ def delivery_preference end def update_otp_delivery_preference_if_needed + return unless user_signed_in? OtpDeliveryPreferenceUpdater.new( user: current_user, preference: delivery_params[:otp_delivery_preference], - context: otp_delivery_selection_form.context + phone_id: user_session[:phone_id] ).call end diff --git a/app/decorators/mfa_context.rb b/app/decorators/mfa_context.rb index 4f1f9042559..4749a42a093 100644 --- a/app/decorators/mfa_context.rb +++ b/app/decorators/mfa_context.rb @@ -13,6 +13,11 @@ def phone_configurations end end + def phone_configuration(id = nil) + return phone_configurations.first if id.blank? + phone_configurations.find { |cfg| cfg.id.to_s == id.to_s } + end + def webauthn_configurations if user.present? user.webauthn_configurations diff --git a/app/forms/two_factor_login_options_form.rb b/app/forms/two_factor_login_options_form.rb index c3d324b404a..3002a759a8d 100644 --- a/app/forms/two_factor_login_options_form.rb +++ b/app/forms/two_factor_login_options_form.rb @@ -11,11 +11,7 @@ def initialize(user) end def submit(params) - selection = params[:selection] - (selection, configuration_id) = selection.split(':', 2) if selection.present? - - self.selection = selection - self.configuration_id = configuration_id + self.selection, self.configuration_id = selection_and_configuration_id(params) success = valid? @@ -28,6 +24,16 @@ def submit(params) attr_writer :selection attr_writer :configuration_id + def selection_and_configuration_id(params) + selection = params[:selection] + configuration_id = nil + if selection =~ /(.+)[:_](\d+)/ + selection = Regexp.last_match(1) + configuration_id = Regexp.last_match(2) + end + [selection, configuration_id] + end + def extra_analytics_attributes { selection: selection, diff --git a/app/forms/two_factor_options_form.rb b/app/forms/two_factor_options_form.rb index 07be228f3e0..4c7e2b5bd9f 100644 --- a/app/forms/two_factor_options_form.rb +++ b/app/forms/two_factor_options_form.rb @@ -37,7 +37,7 @@ def extra_analytics_attributes def user_needs_updating? %w[voice sms].include?(selection) && - selection != MfaContext.new(user).phone_configurations.first&.delivery_preference && + selection != MfaContext.new(user).phone_configuration&.delivery_preference && selection != user.otp_delivery_preference end diff --git a/app/services/otp_delivery_preference_updater.rb b/app/services/otp_delivery_preference_updater.rb index ce1592a504d..bead12dc960 100644 --- a/app/services/otp_delivery_preference_updater.rb +++ b/app/services/otp_delivery_preference_updater.rb @@ -1,8 +1,8 @@ class OtpDeliveryPreferenceUpdater - def initialize(user:, preference:, context:) + def initialize(user:, preference:, phone_id:) @user = user @preference = preference - @context = context + @phone_id = phone_id end def call @@ -12,7 +12,7 @@ def call private - attr_reader :user, :preference, :context + attr_reader :user, :preference, :phone_id def should_update_user? return false unless user @@ -21,7 +21,7 @@ def should_update_user? def otp_delivery_preference_changed? return true if preference != user.otp_delivery_preference - phone_configuration = MfaContext.new(user).phone_configurations.first + phone_configuration = MfaContext.new(user).phone_configuration(phone_id) phone_configuration.present? && preference != phone_configuration.delivery_preference end end diff --git a/app/services/update_user.rb b/app/services/update_user.rb index 014fa8e8277..a1cef214fae 100644 --- a/app/services/update_user.rb +++ b/app/services/update_user.rb @@ -5,7 +5,7 @@ def initialize(user:, attributes:) end def call - result = user.update!(attributes.except(:phone, :phone_confirmed_at)) + result = user.update!(attributes.except(:phone_id, :phone, :phone_confirmed_at)) manage_phone_configuration result end @@ -15,7 +15,7 @@ def call attr_reader :user, :attributes def manage_phone_configuration - if MfaContext.new(user).phone_configurations.any? + if attributes[:phone_id].present? update_phone_configuration else create_phone_configuration @@ -23,14 +23,18 @@ def manage_phone_configuration end def update_phone_configuration - MfaContext.new(user).phone_configurations.first.update!(phone_attributes) + MfaContext.new(user).phone_configuration(attributes[:phone_id]).update!(phone_attributes) end def create_phone_configuration - return if phone_attributes[:phone].blank? + return if phone_attributes[:phone].blank? || duplicate_phone? MfaContext.new(user).phone_configurations.create(phone_attributes) end + def duplicate_phone? + MfaContext.new(user).phone_configurations.map(&:phone).index(phone_attributes[:phone]) + end + def phone_attributes @phone_attributes ||= { phone: attributes[:phone], diff --git a/app/views/accounts/_phone.html.slim b/app/views/accounts/_phone.html.slim index a159b29c771..3a8c23ead90 100644 --- a/app/views/accounts/_phone.html.slim +++ b/app/views/accounts/_phone.html.slim @@ -3,14 +3,13 @@ .col.col-6.bold = t('account.index.phone') .right-align.col.col-6 - - if MfaContext.new(current_user).phone_configurations.empty? - .btn.btn-account-action.rounded-lg.bg-light-blue - = link_to t('account.index.phone_add'), manage_phone_path + .btn.btn-account-action.rounded-lg.bg-light-blue + = link_to t('account.index.phone_add'), add_phone_path - MfaContext.new(current_user).phone_configurations.each do |phone_configuration| .p2.col.col-12.border-top.border-blue-light.account-list-item .col.col-8.sm-6.truncate = phone_configuration.phone .col.col-4.sm-6.right-align = render @view_model.manage_action_partial, - path: manage_phone_url, + path: manage_phone_url(id: phone_configuration.id), name: t('account.index.phone') diff --git a/app/views/users/phones/add.html.slim b/app/views/users/phones/add.html.slim new file mode 100644 index 00000000000..6ad88eb65ea --- /dev/null +++ b/app/views/users/phones/add.html.slim @@ -0,0 +1,23 @@ +- title t('titles.add_info.phone') + +h1.h3.my0 = t('headings.add_info.phone') += simple_form_for(@user_phone_form, + html: { autocomplete: 'off', method: :post, role: 'form' }, + data: { international_phone_form: true }, + url: add_phone_path) do |f| + .sm-col-8.js-intl-tel-code-select + = f.input :international_code, + collection: international_phone_codes, + include_blank: false, + input_html: { class: 'international-code' } + .sm-col-8.mb3 + = f.label :phone + strong.left = @presenter.label + = f.input :phone, as: :tel, label: false, required: true, + input_html: { class: 'phone col-8 mb4' } + = render 'users/shared/otp_delivery_preference_selection' + = f.button :submit, t('forms.buttons.continue'), class: 'btn-wide' += render 'shared/cancel', link: account_path + += stylesheet_link_tag 'intl-tel-number/intlTelInput' += javascript_pack_tag 'intl-tel-input' diff --git a/app/views/users/phones/edit.html.slim b/app/views/users/phones/edit.html.slim index 58e1ca307b5..63793bab03a 100644 --- a/app/views/users/phones/edit.html.slim +++ b/app/views/users/phones/edit.html.slim @@ -20,7 +20,7 @@ h1.h3.my0 = t('headings.edit_info.phone') - if MfaPolicy.new(current_user).multiple_factors_enabled? && @user_phone_form.phone.present? br .sm-col-8.mb3 - = button_to t('forms.phone.buttons.delete'), manage_phone_url, \ + = button_to t('forms.phone.buttons.delete'), manage_phone_url(id: params[:id]), \ class: 'btn btn-danger btn-wide', \ method: :delete = render 'shared/cancel', link: account_path diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index bb563307eaa..9f733dfd293 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -12,6 +12,8 @@ en: verified_account: Verified Account account_recovery_setup: piv_cac_linked: Your PIV/CAC card is linked to your account + add_info: + phone: Add a phone number cancellations: confirmation: You have cancelled verifying your identity with login.gov prompt: Are you sure you want to cancel? diff --git a/config/locales/headings/es.yml b/config/locales/headings/es.yml index 58bfd4defc4..92a0f466cfd 100644 --- a/config/locales/headings/es.yml +++ b/config/locales/headings/es.yml @@ -12,6 +12,8 @@ es: verified_account: Cuenta verificada account_recovery_setup: piv_cac_linked: Tu tarjeta PIV/CAC está vinculada a tu cuenta + add_info: + phone: Agregar un número de teléfono cancellations: confirmation: Ha cancelado la verificación de su identidad con login.gov prompt: "¿Estas seguro que quieres cancelar?" diff --git a/config/locales/headings/fr.yml b/config/locales/headings/fr.yml index 9377c7c3812..ad9ae7794ce 100644 --- a/config/locales/headings/fr.yml +++ b/config/locales/headings/fr.yml @@ -12,6 +12,8 @@ fr: verified_account: Compte vérifié account_recovery_setup: piv_cac_linked: Votre carte PIV/CAC est liée à votre compte + add_info: + phone: Ajouter un numéro de téléphone cancellations: confirmation: Vous avez annulé la vérification de votre identité avec login.gov prompt: Es-tu sûre de vouloir annuler? diff --git a/config/locales/titles/en.yml b/config/locales/titles/en.yml index 6e5379493ed..6da95bf39e4 100644 --- a/config/locales/titles/en.yml +++ b/config/locales/titles/en.yml @@ -4,6 +4,8 @@ en: account: Account account_locked: Account temporarily locked account_recovery_setup: Account Recovery Setup + add_info: + phone: Add a phone number confirmations: new: Resend confirmation instructions for your account show: Choose a password diff --git a/config/locales/titles/es.yml b/config/locales/titles/es.yml index 470668e9336..fec5104b40c 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -4,6 +4,8 @@ es: account: Cuenta account_locked: Cuenta bloqueada temporalmente account_recovery_setup: Ajustes de recuperación de cuenta + add_info: + phone: Agregar un número de teléfono confirmations: new: Reenviar instrucciones de confirmación de su cuenta show: Elija una contraseña diff --git a/config/locales/titles/fr.yml b/config/locales/titles/fr.yml index 53c783c4a37..c05dfdf1fdd 100644 --- a/config/locales/titles/fr.yml +++ b/config/locales/titles/fr.yml @@ -4,6 +4,8 @@ fr: account: Compte account_locked: Compte temporairement verrouillé account_recovery_setup: Configuration de la récupération du compte + add_info: + phone: Ajouter un numéro de téléphone confirmations: new: Envoyer les instructions de confirmation pour votre compte show: Choisissez un mot de passe diff --git a/config/routes.rb b/config/routes.rb index fac1e4c448b..d08c590d510 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -137,6 +137,8 @@ match '/manage/email' => 'users/emails#update', via: %i[patch put] get '/manage/password' => 'users/passwords#edit' patch '/manage/password' => 'users/passwords#update' + get '/add/phone' => 'users/phones#add' + post '/add/phone' => 'users/phones#create' get '/manage/phone' => 'users/phones#edit' match '/manage/phone' => 'users/phones#update', via: %i[patch put] delete '/manage/phone' => 'users/phones#delete' diff --git a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb index 8f745eba529..2ea0bf80d33 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -281,6 +281,8 @@ context 'user has an existing phone number' do context 'user enters a valid code' do before do + controller.user_session[:phone_id] = \ + MfaContext.new(subject.current_user).phone_configurations.last.id post( :create, params: { diff --git a/spec/controllers/users/phones_controller_spec.rb b/spec/controllers/users/phones_controller_spec.rb index 4f2cbb5b602..3ddb880700a 100644 --- a/spec/controllers/users/phones_controller_spec.rb +++ b/spec/controllers/users/phones_controller_spec.rb @@ -216,4 +216,35 @@ end end end + + context 'user adds phone' do + let(:user) { create(:user, :signed_up, with: { phone: '+1 (202) 555-1234' }) } + let(:new_phone) { '202-555-4321' } + before do + stub_sign_in(user) + + stub_analytics + allow(@analytics).to receive(:track_event) + end + + it 'gives the user a form to enter a new phone number' do + get :add + expect(response).to render_template(:add) + end + + it 'lets user know they need to confirm their new phone' do + put :create, params: { + user_phone_form: { phone: new_phone, + international_code: 'US', + otp_delivery_preference: 'sms' }, + } + expect(flash[:notice]).to eq t('devise.registrations.phone_update_needs_confirmation') + expect( + MfaContext.new(user).phone_configurations.reload.first.phone + ).to_not eq '+1 202-555-4321' + expect(response).to redirect_to(otp_send_path(otp_delivery_selection_form: + { otp_delivery_preference: 'sms' })) + expect(subject.user_session[:context]).to eq 'confirmation' + end + end end diff --git a/spec/features/two_factor_authentication/change_factor_spec.rb b/spec/features/two_factor_authentication/change_factor_spec.rb index b9d03bf7324..311f5fdfa3d 100644 --- a/spec/features/two_factor_authentication/change_factor_spec.rb +++ b/spec/features/two_factor_authentication/change_factor_spec.rb @@ -26,7 +26,7 @@ MfaContext.new(user).phone_configurations.reload.first.confirmed_at new_phone = '+1 703-555-0100' - visit manage_phone_path + visit manage_phone_path(id: user.phone_configurations.first.id) expect(page).to have_content t('help_text.change_factor', factor: 'phone') diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 5f36a1173a9..31a6dd3a34e 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -396,7 +396,7 @@ expect(VoiceOtpSenderJob).to_not have_received(:perform_later) expect(SmsOtpSenderJob).to have_received(:perform_later).exactly(:once) expect(page). - to have_current_path(login_two_factor_path(otp_delivery_preference: 'sms')) + to have_current_path(login_two_factor_path(otp_delivery_preference: 'sms', reauthn: false)) expect(page).to have_content t( 'two_factor_authentication.otp_delivery_preference.phone_unsupported', location: 'India' diff --git a/spec/services/otp_delivery_preference_updater_spec.rb b/spec/services/otp_delivery_preference_updater_spec.rb index aa48a0e1e1b..e21f9bb44ec 100644 --- a/spec/services/otp_delivery_preference_updater_spec.rb +++ b/spec/services/otp_delivery_preference_updater_spec.rb @@ -5,7 +5,7 @@ OtpDeliveryPreferenceUpdater.new( user: build_stubbed(:user, otp_delivery_preference: 'sms'), preference: 'sms', - context: 'authentication' + phone_id: 1 ) end @@ -25,7 +25,7 @@ updater = OtpDeliveryPreferenceUpdater.new( user: user, preference: 'sms', - context: 'authentication' + phone_id: 1 ) attributes = { otp_delivery_preference: 'sms' } @@ -45,7 +45,7 @@ updater = OtpDeliveryPreferenceUpdater.new( user: nil, preference: 'sms', - context: 'idv' + phone_id: 1 ) expect(UpdateUser).to_not receive(:new) diff --git a/spec/services/update_user_spec.rb b/spec/services/update_user_spec.rb index cd55785b766..59776123cdf 100644 --- a/spec/services/update_user_spec.rb +++ b/spec/services/update_user_spec.rb @@ -16,21 +16,6 @@ context 'with a phone already configured' do let(:user) { create(:user, :with_phone) } - it 'updates the phone configuration' do - confirmed_at = 1.day.ago.change(usec: 0) - attributes = { - otp_delivery_preference: 'voice', - phone: '+1 222 333-4444', - phone_confirmed_at: confirmed_at, - } - updater = UpdateUser.new(user: user, attributes: attributes) - updater.call - phone_configuration = user.phone_configurations.reload.first - expect(phone_configuration.delivery_preference).to eq 'voice' - expect(phone_configuration.confirmed_at).to eq confirmed_at - expect(phone_configuration.phone).to eq '+1 222 333-4444' - end - it 'does not delete the phone configuration' do attributes = { phone: nil } updater = UpdateUser.new(user: user, attributes: attributes) diff --git a/spec/views/accounts/show.html.slim_spec.rb b/spec/views/accounts/show.html.slim_spec.rb index 8d598c732df..be4c5c10cde 100644 --- a/spec/views/accounts/show.html.slim_spec.rb +++ b/spec/views/accounts/show.html.slim_spec.rb @@ -166,18 +166,18 @@ render expect(rendered).to have_link( - t('account.index.phone_add'), href: manage_phone_path + t('account.index.phone_add'), href: add_phone_path ) end end context 'user has a phone' do - it 'shows no add phone link' do + it 'shows add phone link' do render - expect(rendered).to_not have_content t('account.index.phone_add') - expect(rendered).to_not have_link( - t('account.index.phone_add'), href: manage_phone_path + expect(rendered).to have_content t('account.index.phone_add') + expect(rendered).to have_link( + t('account.index.phone_add'), href: add_phone_path ) end @@ -185,7 +185,7 @@ render expect(rendered).to have_link( - t('account.index.phone'), href: manage_phone_url + t('account.index.phone'), href: manage_phone_url(id: user.phone_configurations.first.id) ) end end