From 34a938e3757abc5c8f9061d65ae436ad0a43620c Mon Sep 17 00:00:00 2001 From: Moncef Belyamani Date: Tue, 12 Jun 2018 08:36:12 -0400 Subject: [PATCH] LG-322 PIV/CAC users must set up a phone number **Why**: PIV/CAC users are required to configure a backup phone number so that they can sign in/recover their account if they no longer have access to their PIV/CAC card. --- .reek | 1 + .../account_recovery_setup_controller.rb | 18 ++++++++++ .../concerns/account_recoverable.rb | 5 +++ app/controllers/concerns/authorizable.rb | 11 ++++++ .../authorization_controller.rb | 8 ++++- app/controllers/saml_idp_controller.rb | 2 ++ .../piv_cac_verification_controller.rb | 8 ++++- .../users/phone_setup_controller.rb | 11 ++---- ...piv_cac_authentication_setup_controller.rb | 7 +++- ..._factor_authentication_setup_controller.rb | 11 ++---- .../account_recovery_options_presenter.rb | 32 +++++++++++++++++ .../account_recovery_setup/index.html.slim | 23 ++++++++++++ config/locales/forms/en.yml | 2 ++ config/locales/forms/es.yml | 2 ++ config/locales/forms/fr.yml | 2 ++ config/locales/headings/en.yml | 2 ++ config/locales/headings/es.yml | 2 ++ config/locales/headings/fr.yml | 2 ++ config/locales/instructions/en.yml | 2 ++ config/locales/instructions/es.yml | 2 ++ config/locales/instructions/fr.yml | 2 ++ config/locales/titles/en.yml | 1 + config/locales/titles/es.yml | 1 + config/locales/titles/fr.yml | 1 + config/routes.rb | 1 + .../account_recovery_setup_controller_spec.rb | 35 +++++++++++++++++++ spec/controllers/saml_idp_controller_spec.rb | 1 + .../users/phone_setup_controller_spec.rb | 9 ++--- ...or_authentication_setup_controller_spec.rb | 15 ++++++-- .../features/users/piv_cac_management_spec.rb | 30 ++++++++++++++-- spec/features/users/sign_in_spec.rb | 2 ++ spec/features/users/sign_up_spec.rb | 28 ++------------- spec/support/features/session_helper.rb | 33 +++++++++++++++++ .../shared_examples/account_creation.rb | 27 ++++++++++++++ spec/support/shared_examples/sign_in.rb | 22 ++++++++++++ 35 files changed, 307 insertions(+), 54 deletions(-) create mode 100644 app/controllers/account_recovery_setup_controller.rb create mode 100644 app/controllers/concerns/account_recoverable.rb create mode 100644 app/controllers/concerns/authorizable.rb create mode 100644 app/presenters/account_recovery_options_presenter.rb create mode 100644 app/views/account_recovery_setup/index.html.slim create mode 100644 spec/controllers/account_recovery_setup_controller_spec.rb diff --git a/.reek b/.reek index cd2da766a4a..f13c3d4d284 100644 --- a/.reek +++ b/.reek @@ -99,6 +99,7 @@ TooManyStatements: - Idv::Agent#proof - Idv::Proofer#configure_vendors - Idv::VendorResult#initialize + - SamlIdpController#auth - Upaya::QueueConfig#self.choose_queue_adapter - Upaya::RandomTools#self.random_weighted_sample - UserFlowFormatter#stop diff --git a/app/controllers/account_recovery_setup_controller.rb b/app/controllers/account_recovery_setup_controller.rb new file mode 100644 index 00000000000..eb22586fee4 --- /dev/null +++ b/app/controllers/account_recovery_setup_controller.rb @@ -0,0 +1,18 @@ +class AccountRecoverySetupController < ApplicationController + include AccountRecoverable + include UserAuthenticator + + before_action :confirm_two_factor_authenticated + + def index + return redirect_to account_url unless piv_cac_enabled_but_not_phone_enabled? + @two_factor_options_form = TwoFactorOptionsForm.new(current_user) + @presenter = account_recovery_options_presenter + end + + private + + def account_recovery_options_presenter + AccountRecoveryOptionsPresenter.new + end +end diff --git a/app/controllers/concerns/account_recoverable.rb b/app/controllers/concerns/account_recoverable.rb new file mode 100644 index 00000000000..5d80977bfff --- /dev/null +++ b/app/controllers/concerns/account_recoverable.rb @@ -0,0 +1,5 @@ +module AccountRecoverable + def piv_cac_enabled_but_not_phone_enabled? + current_user.piv_cac_enabled? && !current_user.phone_enabled? + end +end diff --git a/app/controllers/concerns/authorizable.rb b/app/controllers/concerns/authorizable.rb new file mode 100644 index 00000000000..524f3bff347 --- /dev/null +++ b/app/controllers/concerns/authorizable.rb @@ -0,0 +1,11 @@ +module Authorizable + def authorize_user + return unless current_user.phone_enabled? + + if user_fully_authenticated? + redirect_to account_url + elsif current_user.two_factor_enabled? + redirect_to user_two_factor_authentication_url + end + end +end diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index fa63a51f879..3d499416659 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -1,5 +1,6 @@ module OpenidConnect class AuthorizationController < ApplicationController + include AccountRecoverable include FullyAuthenticatable include VerifyProfileConcern include VerifySPAttributesConcern @@ -13,7 +14,8 @@ class AuthorizationController < ApplicationController def index return confirm_two_factor_authenticated(request_id) unless user_fully_authenticated? - @authorize_form.link_identity_to_service_provider(current_user, session.id) + link_identity_to_service_provider + return redirect_to account_recovery_setup_url if piv_cac_enabled_but_not_phone_enabled? return redirect_to_account_or_verify_profile_url if profile_or_identity_needs_verification? return redirect_to(sign_up_completed_url) if needs_sp_attribute_verification? handle_successful_handoff @@ -21,6 +23,10 @@ def index private + def link_identity_to_service_provider + @authorize_form.link_identity_to_service_provider(current_user, session.id) + end + def handle_successful_handoff redirect_to @authorize_form.success_redirect_uri delete_branded_experience diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index 5e4d7ce475d..c6bc5ab98ab 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -6,6 +6,7 @@ class SamlIdpController < ApplicationController include SamlIdp::Controller include SamlIdpAuthConcern include SamlIdpLogoutConcern + include AccountRecoverable include FullyAuthenticatable include VerifyProfileConcern include VerifySPAttributesConcern @@ -17,6 +18,7 @@ def auth return confirm_two_factor_authenticated(request_id) unless user_fully_authenticated? link_identity_from_session_data capture_analytics + return redirect_to account_recovery_setup_url if piv_cac_enabled_but_not_phone_enabled? return redirect_to_account_or_verify_profile_url if profile_or_identity_needs_verification? return redirect_to(sign_up_completed_url) if needs_sp_attribute_verification? handle_successful_handoff diff --git a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb index bf836a596f6..2673991d530 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -35,10 +35,16 @@ def handle_valid_piv_cac ) handle_valid_otp_for_authentication_context - redirect_to after_otp_verification_confirmation_url + redirect_to next_step reset_otp_session_data end + def next_step + return account_recovery_setup_url unless current_user.phone_enabled? + + after_otp_verification_confirmation_url + end + def handle_invalid_piv_cac clear_piv_cac_information # create new nonce for retry diff --git a/app/controllers/users/phone_setup_controller.rb b/app/controllers/users/phone_setup_controller.rb index 4fd8c903220..08f47a27176 100644 --- a/app/controllers/users/phone_setup_controller.rb +++ b/app/controllers/users/phone_setup_controller.rb @@ -2,9 +2,10 @@ module Users class PhoneSetupController < ApplicationController include UserAuthenticator include PhoneConfirmation + include Authorizable before_action :authenticate_user - before_action :authorize_phone_setup + before_action :authorize_user def index @user_phone_form = UserPhoneForm.new(current_user) @@ -27,14 +28,6 @@ def create private - def authorize_phone_setup - if user_fully_authenticated? - redirect_to account_url - elsif current_user.two_factor_enabled? - redirect_to user_two_factor_authentication_url - end - end - def user_phone_form_params params.require(:user_phone_form).permit( :international_code, diff --git a/app/controllers/users/piv_cac_authentication_setup_controller.rb b/app/controllers/users/piv_cac_authentication_setup_controller.rb index f145f8e9621..e4982c9f1cc 100644 --- a/app/controllers/users/piv_cac_authentication_setup_controller.rb +++ b/app/controllers/users/piv_cac_authentication_setup_controller.rb @@ -59,7 +59,12 @@ def process_valid_submission subject: user_piv_cac_form.x509_dn, presented: true ) - redirect_to account_url + redirect_to next_step + end + + def next_step + return account_url if current_user.phone_enabled? + account_recovery_setup_url end def process_invalid_submission diff --git a/app/controllers/users/two_factor_authentication_setup_controller.rb b/app/controllers/users/two_factor_authentication_setup_controller.rb index 07b65001cda..1ffec9ad70a 100644 --- a/app/controllers/users/two_factor_authentication_setup_controller.rb +++ b/app/controllers/users/two_factor_authentication_setup_controller.rb @@ -1,9 +1,10 @@ module Users class TwoFactorAuthenticationSetupController < ApplicationController include UserAuthenticator + include Authorizable before_action :authenticate_user - before_action :authorize_2fa_setup + before_action :authorize_user def index @two_factor_options_form = TwoFactorOptionsForm.new(current_user) @@ -30,14 +31,6 @@ def two_factor_options_presenter TwoFactorOptionsPresenter.new(current_user, current_sp) end - def authorize_2fa_setup - if user_fully_authenticated? - redirect_to account_url - elsif current_user.two_factor_enabled? - redirect_to user_two_factor_authentication_url - end - end - def process_valid_form case @two_factor_options_form.selection when 'sms', 'voice' diff --git a/app/presenters/account_recovery_options_presenter.rb b/app/presenters/account_recovery_options_presenter.rb new file mode 100644 index 00000000000..7c0f450f2aa --- /dev/null +++ b/app/presenters/account_recovery_options_presenter.rb @@ -0,0 +1,32 @@ +class AccountRecoveryOptionsPresenter + include ActionView::Helpers::TranslationHelper + + AVAILABLE_2FA_TYPES = %w[sms voice].freeze + + def title + t('titles.account_recovery_setup') + end + + def heading + t('headings.account_recovery_setup.piv_cac_linked') + end + + def info + t('instructions.account_recovery_setup.piv_cac_next_step') + end + + def label + t('forms.account_recovery_setup.legend') + ':' + end + + def options + AVAILABLE_2FA_TYPES.map do |type| + OpenStruct.new( + type: type, + label: t("devise.two_factor_authentication.two_factor_choice_options.#{type}"), + info: t("devise.two_factor_authentication.two_factor_choice_options.#{type}_info"), + selected: type == :sms + ) + end + end +end diff --git a/app/views/account_recovery_setup/index.html.slim b/app/views/account_recovery_setup/index.html.slim new file mode 100644 index 00000000000..d0dc36506b2 --- /dev/null +++ b/app/views/account_recovery_setup/index.html.slim @@ -0,0 +1,23 @@ +- title @presenter.title + +h1.h3.my0 = @presenter.heading +p.mt-tiny.mb3 = @presenter.info + += simple_form_for(@two_factor_options_form, + html: { autocomplete: 'off', role: 'form' }, + method: :patch, + url: two_factor_options_path) do |f| + .mb3 + fieldset.m0.p0.border-none. + legend.mb1.h4.serif.bold = @presenter.label + - @presenter.options.each do |option| + label.btn-border.col-12.mb1 for="two_factor_options_form_selection_#{option.type}" + .radio + = radio_button_tag('two_factor_options_form[selection]', + option.type, + @two_factor_options_form.selected?(option.type)) + 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') diff --git a/config/locales/forms/en.yml b/config/locales/forms/en.yml index 7b0286d8c45..b6b79abf966 100644 --- a/config/locales/forms/en.yml +++ b/config/locales/forms/en.yml @@ -1,6 +1,8 @@ --- en: forms: + account_recovery_setup: + legend: Select a secondary authentication option buttons: back: Back continue: Continue diff --git a/config/locales/forms/es.yml b/config/locales/forms/es.yml index 5f45b02309d..2c4e0c85f20 100644 --- a/config/locales/forms/es.yml +++ b/config/locales/forms/es.yml @@ -1,6 +1,8 @@ --- es: forms: + account_recovery_setup: + legend: NOT TRANSLATED YET buttons: back: Atrás continue: Continuar diff --git a/config/locales/forms/fr.yml b/config/locales/forms/fr.yml index 463332376ff..597437d0a31 100644 --- a/config/locales/forms/fr.yml +++ b/config/locales/forms/fr.yml @@ -1,6 +1,8 @@ --- fr: forms: + account_recovery_setup: + legend: NOT TRANSLATED YET buttons: back: Retour continue: Continuer diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index d925758a965..12bde6b447a 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -9,6 +9,8 @@ en: reactivate: Reactivate your account two_factor: Two-factor authentication verified_account: Verified Account + account_recovery_setup: + piv_cac_linked: Your PIV/CAC card is linked to your account confirmations: new: Send another confirmation email create_account_with_sp: diff --git a/config/locales/headings/es.yml b/config/locales/headings/es.yml index 52a5e2528bc..b64ac5c5cb6 100644 --- a/config/locales/headings/es.yml +++ b/config/locales/headings/es.yml @@ -9,6 +9,8 @@ es: reactivate: Reactive su cuenta two_factor: Autenticación de dos factores verified_account: Cuenta verificada + account_recovery_setup: + piv_cac_linked: NOT TRANSLATED YET confirmations: new: Enviar otro email de confirmación create_account_with_sp: diff --git a/config/locales/headings/fr.yml b/config/locales/headings/fr.yml index f7d539628e6..fea1dcc54ae 100644 --- a/config/locales/headings/fr.yml +++ b/config/locales/headings/fr.yml @@ -9,6 +9,8 @@ fr: reactivate: Réactivez votre compte two_factor: Authentification à deux facteurs verified_account: Compte vérifié + account_recovery_setup: + piv_cac_linked: NOT TRANSLATED YET confirmations: new: Envoyer un autre courriel de confirmation create_account_with_sp: diff --git a/config/locales/instructions/en.yml b/config/locales/instructions/en.yml index 6619b10f1ce..3b99851d550 100644 --- a/config/locales/instructions/en.yml +++ b/config/locales/instructions/en.yml @@ -13,6 +13,8 @@ en: identity again. heading: Don't have your personal key? with_key: Do you have your personal key? + account_recovery_setup: + piv_cac_next_step: Next we need to give you a way to recover your account. forgot_password: close_window: You can close this browser window once you have reset your password. go_back_to_mobile_app: To continue, please go back to the %{friendly_name} app diff --git a/config/locales/instructions/es.yml b/config/locales/instructions/es.yml index 2dd82925b89..5d6010562f2 100644 --- a/config/locales/instructions/es.yml +++ b/config/locales/instructions/es.yml @@ -13,6 +13,8 @@ es: copy: Si no tiene su clave personal, verifique su identidad nuevamente. heading: NOT TRANSLATED YET with_key: "¿Tiene su clave personal?" + account_recovery_setup: + piv_cac_next_step: NOT TRANSLATED YET forgot_password: close_window: Puede cerrar esta ventana del navegador después que haya restablecido su contraseña. diff --git a/config/locales/instructions/fr.yml b/config/locales/instructions/fr.yml index d7e6ee49cc0..96c2c7ad5eb 100644 --- a/config/locales/instructions/fr.yml +++ b/config/locales/instructions/fr.yml @@ -15,6 +15,8 @@ fr: identité de nouveau. heading: Vous n'avez pas votre clé personnelle? with_key: Vous n'avez pas votre clé personnelle? + account_recovery_setup: + piv_cac_next_step: NOT TRANSLATED YET forgot_password: close_window: Vous pourrez fermer cette fenêtre de navigateur lorsque vous aurez réinitialisé votre mot de passe. diff --git a/config/locales/titles/en.yml b/config/locales/titles/en.yml index 11459b4fd35..4e4d8ac16ab 100644 --- a/config/locales/titles/en.yml +++ b/config/locales/titles/en.yml @@ -3,6 +3,7 @@ en: titles: account: Account account_locked: Account temporarily locked + account_recovery_setup: Account Recovery Setup 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 b6d6e03eff2..2c4bc569bd4 100644 --- a/config/locales/titles/es.yml +++ b/config/locales/titles/es.yml @@ -3,6 +3,7 @@ es: titles: account: Cuenta account_locked: Cuenta bloqueada temporalmente + account_recovery_setup: NOT TRANSLATED YET 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 5059a46b48b..cc6b8a10858 100644 --- a/config/locales/titles/fr.yml +++ b/config/locales/titles/fr.yml @@ -3,6 +3,7 @@ fr: titles: account: Compte account_locked: Compte temporairement verrouillé + account_recovery_setup: NOT TRANSLATED YET 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 cea7e3dee93..c29d2a27cd4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -99,6 +99,7 @@ as: :create_verify_personal_key get '/account/verify_phone' => 'users/verify_profile_phone#index', as: :verify_profile_phone post '/account/verify_phone' => 'users/verify_profile_phone#create' + get '/account_recovery_setup' => 'account_recovery_setup#index' if FeatureManagement.piv_cac_enabled? get '/piv_cac' => 'users/piv_cac_authentication_setup#new', as: :setup_piv_cac diff --git a/spec/controllers/account_recovery_setup_controller_spec.rb b/spec/controllers/account_recovery_setup_controller_spec.rb new file mode 100644 index 00000000000..4c77947abf2 --- /dev/null +++ b/spec/controllers/account_recovery_setup_controller_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +describe AccountRecoverySetupController do + context 'user is not piv_cac enabled' do + it 'redirects to account_url' do + stub_sign_in + + get :index + + expect(response).to redirect_to account_url + end + end + + context 'user is piv_cac enabled and phone enabled' do + it 'redirects to account_url' do + user = build(:user, :signed_up, :with_piv_or_cac) + stub_sign_in(user) + + get :index + + expect(response).to redirect_to account_url + end + end + + context 'user is piv_cac enabled but not phone enabled' do + it 'redirects to account_url' do + user = build(:user, :signed_up, :with_piv_or_cac, phone: nil) + stub_sign_in(user) + + get :index + + expect(response).to render_template(:index) + end + end +end diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index f202715267e..e2ffb79274a 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -894,6 +894,7 @@ def stub_auth allow(controller).to receive(:validate_saml_request_and_authn_context).and_return(true) allow(controller).to receive(:user_fully_authenticated?).and_return(true) allow(controller).to receive(:link_identity_from_session_data).and_return(true) + allow(controller).to receive(:current_user).and_return(build(:user)) end context 'user requires ID verification' do diff --git a/spec/controllers/users/phone_setup_controller_spec.rb b/spec/controllers/users/phone_setup_controller_spec.rb index f8797d1b3e6..d96c2364cd6 100644 --- a/spec/controllers/users/phone_setup_controller_spec.rb +++ b/spec/controllers/users/phone_setup_controller_spec.rb @@ -161,15 +161,16 @@ expect(subject).to have_actions( :before, :authenticate_user, - :authorize_phone_setup + :authorize_user ) end end - describe '#authorize_otp_setup' do - context 'when the user is fully authenticated' do + describe '#authorize_user' do + context 'when the user is fully authenticated and phone enabled' do it 'redirects to account url' do - stub_sign_in + user = build_stubbed(:user, :with_phone) + stub_sign_in(user) get :index diff --git a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb index 63fe066c199..e5af8d4b2e5 100644 --- a/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_setup_controller_spec.rb @@ -20,9 +20,10 @@ end end - context 'when fully authenticated' do + context 'when fully authenticated and phone enabled' do it 'redirects to account page' do - stub_sign_in + user = build(:user, :signed_up) + stub_sign_in(user) get :index @@ -30,6 +31,16 @@ end end + context 'when fully authenticated but not phone enabled' do + it 'allows access' do + stub_sign_in + + get :index + + expect(response).to render_template(:index) + end + end + context 'already two factor enabled but not fully authenticated' do it 'prompts for 2FA' do user = build(:user, :signed_up) diff --git a/spec/features/users/piv_cac_management_spec.rb b/spec/features/users/piv_cac_management_spec.rb index 7e8c77b3d42..9a033a90408 100644 --- a/spec/features/users/piv_cac_management_spec.rb +++ b/spec/features/users/piv_cac_management_spec.rb @@ -37,10 +37,10 @@ def find_form(page, attributes) sign_in_and_2fa_user(user) visit account_path - expect(page).to have_link(t('forms.buttons.enable'), href: setup_piv_cac_url) + click_link t('forms.buttons.enable'), href: setup_piv_cac_url - visit setup_piv_cac_url expect(page).to have_link(t('forms.piv_cac_setup.submit')) + nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_setup.submit'))) visit_piv_cac_service(setup_piv_cac_url, @@ -66,6 +66,32 @@ def find_form(page, attributes) form = find_form(page, action: disable_piv_cac_url) expect(form).to be_nil end + + context 'when the user does not have a phone number yet' do + it 'prompts to set one up after configuring PIV/CAC' do + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + stub_piv_cac_service + + user.update(phone: nil, otp_secret_key: 'secret') + sign_in_and_2fa_user(user) + visit account_path + click_link t('forms.buttons.enable'), href: setup_piv_cac_url + + expect(page).to have_current_path(setup_piv_cac_path) + + nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_setup.submit'))) + visit_piv_cac_service(setup_piv_cac_url, + nonce: nonce, + uuid: SecureRandom.uuid, + subject: 'SomeIgnoredSubject') + + expect(page).to have_current_path(account_recovery_setup_path) + + configure_backup_phone + + expect(page).to have_current_path account_path + end + end end context 'with a service provider not allowed to use piv/cac' do diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 860fb3210e0..21441cfb3ce 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -403,6 +403,8 @@ it_behaves_like 'signing in as LOA3 with personal key', :oidc it_behaves_like 'signing in with wrong credentials', :saml it_behaves_like 'signing in with wrong credentials', :oidc + it_behaves_like 'signing with while PIV/CAC enabled but not phone enabled', :saml + it_behaves_like 'signing with while PIV/CAC enabled but not phone enabled', :oidc context 'user signs in with personal key, visits account page before viewing new key' do # this can happen if you submit the personal key form multiple times quickly diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 8e72c903b47..adbe4f299b2 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -155,6 +155,9 @@ it_behaves_like 'creating an account using authenticator app for 2FA', :saml it_behaves_like 'creating an account using authenticator app for 2FA', :oidc + it_behaves_like 'creating an account using PIV/CAC for 2FA', :saml + it_behaves_like 'creating an account using PIV/CAC for 2FA', :oidc + it 'allows a user to choose TOTP as 2FA method during sign up' do sign_in_user set_up_2fa_with_authenticator_app @@ -174,31 +177,6 @@ ) end - context 'when piv/cac is allowed' do - it 'allows a user to choose piv/cac as 2FA method during sign up' do - allow(PivCacService).to receive(:piv_cac_available_for_agency?).and_return(true) - allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) - - begin_sign_up_with_sp_and_loa(loa3: false) - - expect(page).to have_current_path two_factor_options_path - expect(page).to have_content( - t('devise.two_factor_authentication.two_factor_choice_options.piv_cac') - ) - end - - it 'directs to the piv/cac setup page' do - allow(PivCacService).to receive(:piv_cac_available_for_agency?).and_return(true) - allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) - - begin_sign_up_with_sp_and_loa(loa3: false) - - expect(page).to have_current_path two_factor_options_path - select_2fa_option('piv_cac') - expect(page).to have_current_path setup_piv_cac_path - end - end - it 'does not bypass 2FA when accessing authenticator_setup_path if the user is 2FA enabled' do user = create(:user, :signed_up) sign_in_user(user) diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index d3beb46f646..bdfb8b76be6 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -407,6 +407,32 @@ def set_up_2fa_with_authenticator_app click_button 'Submit' end + def register_user_with_piv_cac(email = 'test@test.com') + allow(PivCacService).to receive(:piv_cac_available_for_agency?).and_return(true) + allow(FeatureManagement).to receive(:piv_cac_enabled?).and_return(true) + confirm_email_and_password(email) + + expect(page).to have_current_path two_factor_options_path + expect(page).to have_content( + t('devise.two_factor_authentication.two_factor_choice_options.piv_cac') + ) + + set_up_2fa_with_piv_cac + end + + def set_up_2fa_with_piv_cac + stub_piv_cac_service + select_2fa_option('piv_cac') + + expect(page).to have_current_path setup_piv_cac_path + + nonce = get_piv_cac_nonce_from_link(find_link(t('forms.piv_cac_setup.submit'))) + visit_piv_cac_service(setup_piv_cac_url, + nonce: nonce, + uuid: SecureRandom.uuid, + subject: 'SomeIgnoredSubject') + end + def sign_in_via_branded_page(user) allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) click_link t('links.sign_in') @@ -456,5 +482,12 @@ def link_identity(user, client_id, ial = nil) ial: ial ) end + + def configure_backup_phone + select_2fa_option('sms') + fill_in 'user_phone_form_phone', with: '202-555-1212' + click_send_security_code + click_submit_default + end end end diff --git a/spec/support/shared_examples/account_creation.rb b/spec/support/shared_examples/account_creation.rb index 8c19d125035..b5f0381ea93 100644 --- a/spec/support/shared_examples/account_creation.rb +++ b/spec/support/shared_examples/account_creation.rb @@ -68,3 +68,30 @@ end end end + +shared_examples 'creating an account using PIV/CAC for 2FA' do |sp| + it 'redirects to the SP', email: true do + allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true) + visit_idp_from_sp_with_loa1(sp) + register_user_with_piv_cac + + expect(page).to have_current_path(account_recovery_setup_path) + expect(page).to have_content t('instructions.account_recovery_setup.piv_cac_next_step') + configure_backup_phone + click_acknowledge_personal_key + + if sp == :oidc + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\' http://localhost:7654')) + end + + click_on t('forms.buttons.continue') + expect(current_url).to eq @saml_authn_request if sp == :saml + + if sp == :oidc + redirect_uri = URI(current_url) + + expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') + end + end +end diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index 40631bdf3fe..6a89df88c95 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -177,6 +177,28 @@ end end +shared_examples 'signing with while PIV/CAC enabled but not phone enabled' do |sp| + it 'does not allow bypassing setting up backup phone' do + stub_piv_cac_service + + user = create(:user, :signed_up, :with_piv_or_cac, phone: nil) + visit_idp_from_sp_with_loa1(sp) + click_link t('links.sign_in') + fill_in_credentials_and_submit(user.email, user.password) + nonce = visit_login_two_factor_piv_cac_and_get_nonce + visit_piv_cac_service(login_two_factor_piv_cac_path, + uuid: user.x509_dn_uuid, + dn: 'C=US, O=U.S. Government, OU=DoD, OU=PKI, CN=DOE.JOHN.1234', + nonce: nonce) + + expect(current_path).to eq account_recovery_setup_path + + visit_idp_from_sp_with_loa1(sp) + + expect(current_path).to eq account_recovery_setup_path + end +end + def personal_key_for_loa3_user(user, pii) pii_attrs = Pii::Attributes.new_from_hash(pii) profile = user.profiles.last