From 9cf0b0311503a379356ae661363a53203de15e3e Mon Sep 17 00:00:00 2001 From: Adam Biagianti Date: Mon, 5 Jun 2017 17:21:52 -0400 Subject: [PATCH] Add controller/routes/view/specs for personal key **Why**: When a user reactivates their account, the personal key entry and re encryption of pii are being split out into separate steps Add personal key form partial, new form, alert **Why**: The personal key input form is separate from the rest of the presentation. Required for the new personal key entry form. Added an excalaimation point in the alert notice to match designs and other alert messages Adds verify password class and emphemera **Why**: We are splitting out the account reactivation flow, which requires an additional controller, shared views, new tests and routes Removes old account reactivation files **Why**: The flow isn't the same and these files are uneeded Spec updates, manage_reactivate to reactivate **Why**: The old account reactivation code was removed, so we can safely move manage_reactivate_account to reactivate_account --- app/assets/images/alert/notice.svg | 1 + app/assets/stylesheets/components/_alert.scss | 7 +- .../stylesheets/components/_personal-key.scss | 11 +++ app/assets/stylesheets/variables/_colors.scss | 1 + .../concerns/account_recovery_concern.rb | 8 ++ .../concerns/two_factor_authenticatable.rb | 2 +- .../reactivate_account_controller.rb | 9 +- .../users/reactivate_account_controller.rb | 25 ----- .../users/verify_password_controller.rb | 48 +++++++++ .../users/verify_personal_key_controller.rb | 55 +++++++++++ app/controllers/verify_controller.rb | 2 +- app/forms/reactivate_account_form.rb | 80 --------------- app/forms/verify_password_form.rb | 49 +++++++++ app/forms/verify_personal_key_form.rb | 57 +++++++++++ app/services/personal_key_formatter.rb | 4 + app/views/accounts/_password_reset.html.slim | 2 +- app/views/reactivate_account/index.html.slim | 4 +- ..._personal_key_confirmation_modal.html.slim | 11 +-- .../shared/_personal_key_input.html.slim | 10 ++ app/views/shared/_pii_review.html.slim | 19 ++++ .../shared/_user_verify_password.html.slim | 4 + .../users/reactivate_account/index.html.slim | 13 --- app/views/users/verify_password/new.html.slim | 9 ++ .../users/verify_personal_key/new.html.slim | 16 +++ app/views/verify/review/new.html.slim | 25 +---- config/locales/forms/en.yml | 5 +- config/locales/forms/es.yml | 5 +- config/locales/idv/en.yml | 2 +- config/locales/links/en.yml | 1 + config/locales/links/es.yml | 1 + config/locales/notices/en.yml | 3 +- config/locales/notices/es.yml | 1 + config/routes.rb | 10 +- .../reactivate_account_controller_spec.rb | 44 --------- .../users/verify_password_controller_spec.rb | 71 +++++++++++++ .../verify_personal_key_controller_spec.rb | 90 +++++++++++++++++ spec/controllers/verify_controller_spec.rb | 2 +- spec/features/saml/loa1_sso_spec.rb | 2 +- ...assword_recovery_via_recovery_code_spec.rb | 22 +++-- .../users/regenerate_personal_key_spec.rb | 2 +- spec/forms/reactivate_account_form_spec.rb | 99 ------------------- spec/support/features/session_helper.rb | 2 +- spec/views/accounts/show.html.slim_spec.rb | 2 +- 43 files changed, 503 insertions(+), 333 deletions(-) create mode 100644 app/assets/images/alert/notice.svg create mode 100644 app/controllers/concerns/account_recovery_concern.rb delete mode 100644 app/controllers/users/reactivate_account_controller.rb create mode 100644 app/controllers/users/verify_password_controller.rb create mode 100644 app/controllers/users/verify_personal_key_controller.rb delete mode 100644 app/forms/reactivate_account_form.rb create mode 100644 app/forms/verify_password_form.rb create mode 100644 app/forms/verify_personal_key_form.rb create mode 100644 app/views/shared/_personal_key_input.html.slim create mode 100644 app/views/shared/_pii_review.html.slim create mode 100644 app/views/shared/_user_verify_password.html.slim delete mode 100644 app/views/users/reactivate_account/index.html.slim create mode 100644 app/views/users/verify_password/new.html.slim create mode 100644 app/views/users/verify_personal_key/new.html.slim delete mode 100644 spec/controllers/users/reactivate_account_controller_spec.rb create mode 100644 spec/controllers/users/verify_password_controller_spec.rb create mode 100644 spec/controllers/users/verify_personal_key_controller_spec.rb delete mode 100644 spec/forms/reactivate_account_form_spec.rb diff --git a/app/assets/images/alert/notice.svg b/app/assets/images/alert/notice.svg new file mode 100644 index 00000000000..79a9e96d604 --- /dev/null +++ b/app/assets/images/alert/notice.svg @@ -0,0 +1 @@ +warning diff --git a/app/assets/stylesheets/components/_alert.scss b/app/assets/stylesheets/components/_alert.scss index 69b6475a4ca..48dd85515eb 100644 --- a/app/assets/stylesheets/components/_alert.scss +++ b/app/assets/stylesheets/components/_alert.scss @@ -2,7 +2,7 @@ $ico-size: 1rem; $ico-offset: 1rem; .alert { - background-color: $blue-light; + background-color: $blue-lighter; border-radius: $space-1; color: #5b616a; font-size: 1rem; @@ -51,3 +51,8 @@ $ico-offset: 1rem; &::before { background-image: url(image-path('alert/warning.svg')); } } + +.alert-notice { + padding-left: $space-4; + &::before { background-image: url(image-path('alert/notice.svg')); } +} diff --git a/app/assets/stylesheets/components/_personal-key.scss b/app/assets/stylesheets/components/_personal-key.scss index bb10d945530..2add0346dcc 100644 --- a/app/assets/stylesheets/components/_personal-key.scss +++ b/app/assets/stylesheets/components/_personal-key.scss @@ -1,3 +1,14 @@ +.key-badge::before { + background-image: url(image-path('p-key.svg')); + background-repeat: no-repeat; + content: ''; + height: 60px; + left: 45%; + position: absolute; + top: -25px; + width: 60px; +} + .separator-text > div { &::after { color: $silver; diff --git a/app/assets/stylesheets/variables/_colors.scss b/app/assets/stylesheets/variables/_colors.scss index 11dd1fef9b4..84545970702 100644 --- a/app/assets/stylesheets/variables/_colors.scss +++ b/app/assets/stylesheets/variables/_colors.scss @@ -1,6 +1,7 @@ $aqua: #7fdbff !default; $blue: #0071bb !default; $blue-light: #ebf3fa !default; +$blue-lighter: #ecfcff !default; $blue-lightest: #f2f9ff !default; $navy: #112e51 !default; $teal: #00bfe7 !default; diff --git a/app/controllers/concerns/account_recovery_concern.rb b/app/controllers/concerns/account_recovery_concern.rb new file mode 100644 index 00000000000..1f51b9feb1e --- /dev/null +++ b/app/controllers/concerns/account_recovery_concern.rb @@ -0,0 +1,8 @@ +module AccountRecoveryConcern + extend ActiveSupport::Concern + + def confirm_password_reset_profile + return if current_user.decorate.password_reset_profile + redirect_to root_url + end +end diff --git a/app/controllers/concerns/two_factor_authenticatable.rb b/app/controllers/concerns/two_factor_authenticatable.rb index f7a5818d6ce..837c867cf68 100644 --- a/app/controllers/concerns/two_factor_authenticatable.rb +++ b/app/controllers/concerns/two_factor_authenticatable.rb @@ -190,7 +190,7 @@ def after_otp_action_path elsif @updating_existing_number account_path elsif decorated_user.password_reset_profile.present? - manage_reactivate_account_path + reactivate_account_path else account_path end diff --git a/app/controllers/reactivate_account_controller.rb b/app/controllers/reactivate_account_controller.rb index c8e43c8b85f..b76bbf04f32 100644 --- a/app/controllers/reactivate_account_controller.rb +++ b/app/controllers/reactivate_account_controller.rb @@ -1,4 +1,6 @@ class ReactivateAccountController < ApplicationController + include AccountRecoveryConcern + before_action :confirm_two_factor_authenticated before_action :confirm_password_reset_profile @@ -10,11 +12,4 @@ def update user_session.delete(:acknowledge_personal_key) redirect_to verify_url end - - protected - - def confirm_password_reset_profile - return if current_user.decorate.password_reset_profile - redirect_to root_url - end end diff --git a/app/controllers/users/reactivate_account_controller.rb b/app/controllers/users/reactivate_account_controller.rb deleted file mode 100644 index b7b4d9d492e..00000000000 --- a/app/controllers/users/reactivate_account_controller.rb +++ /dev/null @@ -1,25 +0,0 @@ -module Users - class ReactivateAccountController < ApplicationController - def index - @reactivate_account_form = ReactivateAccountForm.new(current_user) - end - - def create - @reactivate_account_form = build_reactivate_account_form - if @reactivate_account_form.submit(flash) - redirect_to account_path - else - render :index - end - end - - protected - - def build_reactivate_account_form - ReactivateAccountForm.new( - current_user, - params[:reactivate_account_form].permit(:password, :personal_key) - ) - end - end -end diff --git a/app/controllers/users/verify_password_controller.rb b/app/controllers/users/verify_password_controller.rb new file mode 100644 index 00000000000..a1ba8cbaf8a --- /dev/null +++ b/app/controllers/users/verify_password_controller.rb @@ -0,0 +1,48 @@ +module Users + class VerifyPasswordController < ApplicationController + include AccountRecoveryConcern + + before_action :confirm_two_factor_authenticated + before_action :confirm_password_reset_profile + before_action :confirm_personal_key + + def new + @verify_password_form = VerifyPasswordForm.new( + user: current_user, + password: '', + decrypted_pii: decrypted_pii + ) + end + + def update + result = verify_password_form.submit + + if result.success? + flash[:personal_key] = result.extra[:personal_key] + user_session.delete(:account_recovery) + redirect_to account_url + else + render :new + end + end + + private + + def confirm_personal_key + account_recovery = user_session[:account_recovery] + redirect_to root_url unless account_recovery[:personal_key] + end + + def decrypted_pii + @_decrypted_pii ||= Pii::Attributes.new_from_json(user_session[:decrypted_pii]) + end + + def verify_password_form + VerifyPasswordForm.new( + user: current_user, + password: params.require(:user).permit(:password)[:password], + decrypted_pii: decrypted_pii + ) + end + end +end diff --git a/app/controllers/users/verify_personal_key_controller.rb b/app/controllers/users/verify_personal_key_controller.rb new file mode 100644 index 00000000000..83c40c5bf13 --- /dev/null +++ b/app/controllers/users/verify_personal_key_controller.rb @@ -0,0 +1,55 @@ +module Users + class VerifyPersonalKeyController < ApplicationController + include AccountRecoveryConcern + + before_action :confirm_two_factor_authenticated + before_action :confirm_password_reset_profile + before_action :init_account_recovery, only: [:create] + + def new + flash.now[:notice] = t('notices.account_recovery') unless user_session[:account_recovery] + + @personal_key_form = VerifyPersonalKeyForm.new( + user: current_user, + personal_key: '' + ) + end + + def create + result = personal_key_form.submit + + if result.success? + handle_success(result) + else + handle_failure(result) + end + end + + private + + def init_account_recovery + user_session[:account_recovery] ||= { + personal_key: false, + } + end + + def handle_success(result) + user_session[:account_recovery][:personal_key] = true + user_session[:decrypted_pii] = result.extra[:decrypted_pii] + + redirect_to verify_password_url + end + + def handle_failure(result) + flash[:error] = result.errors[:personal_key].last + render :new + end + + def personal_key_form + VerifyPersonalKeyForm.new( + user: current_user, + personal_key: params.permit(:personal_key)[:personal_key] + ) + end + end +end diff --git a/app/controllers/verify_controller.rb b/app/controllers/verify_controller.rb index aa9452e8c13..52f0be985bc 100644 --- a/app/controllers/verify_controller.rb +++ b/app/controllers/verify_controller.rb @@ -29,7 +29,7 @@ def fail def profile_needs_reactivation? return unless password_reset_profile && user_session[:acknowledge_personal_key] == true - redirect_to manage_reactivate_account_url + redirect_to reactivate_account_url end def password_reset_profile diff --git a/app/forms/reactivate_account_form.rb b/app/forms/reactivate_account_form.rb deleted file mode 100644 index ccde4703e62..00000000000 --- a/app/forms/reactivate_account_form.rb +++ /dev/null @@ -1,80 +0,0 @@ -class ReactivateAccountForm - include ActiveModel::Model - include PersonalKeyValidator - - validates :personal_key, :password, presence: true - validate :validate_password_reset_profile - validate :validate_password - validate :validate_personal_key - - attr_accessor :personal_key, :password - attr_reader :user - - def initialize(user, attrs = {}) - attrs[:personal_key] ||= nil - @user = user - super attrs - - @personal_key = normalize_personal_key(personal_key) - end - - def submit(flash) - if valid? - flash[:personal_key] = reencrypt_pii - true - else - reset_sensitive_fields - false - end - end - - protected - - def password_reset_profile - @_password_reset_profile ||= user.decorate.password_reset_profile - end - - def user_access_key - @_uak ||= user.unlock_user_access_key(password) - end - - def decrypted_pii - @_pii ||= password_reset_profile.recover_pii(personal_key) - end - - def reencrypt_pii - personal_key = password_reset_profile.encrypt_pii(user_access_key, decrypted_pii) - password_reset_profile.deactivation_reason = nil - password_reset_profile.active = true - password_reset_profile.save! - personal_key - end - - def validate_password_reset_profile - errors.add :base, :no_password_reset_profile unless password_reset_profile - end - - def validate_password - return if valid_password? - errors.add :password, :password_incorrect - end - - def validate_personal_key - return check_personal_key if personal_key_decrypts? - errors.add :personal_key, :personal_key_incorrect - end - - def reset_sensitive_fields - self.password = nil - end - - def valid_password? - user.valid_password?(password) - end - - def personal_key_decrypts? - decrypted_pii.present? - rescue Pii::EncryptionError => _err - false - end -end diff --git a/app/forms/verify_password_form.rb b/app/forms/verify_password_form.rb new file mode 100644 index 00000000000..2a0f3858637 --- /dev/null +++ b/app/forms/verify_password_form.rb @@ -0,0 +1,49 @@ +class VerifyPasswordForm + include ActiveModel::Model + + validates :password, presence: true + validate :validate_password + + attr_reader :user, :password, :decrypted_pii + + def initialize(user:, password:, decrypted_pii:) + @user = user + @password = password + @decrypted_pii = decrypted_pii + end + + def submit + success = valid? + extra = {} + + extra[:personal_key] = reencrypt_pii if success + + FormResponse.new(success: success, errors: errors, extra: extra) + end + + private + + def validate_password + return if valid_password? + errors.add :password, :password_incorrect + end + + def valid_password? + user.valid_password?(password) + end + + def reencrypt_pii + personal_key = profile.encrypt_pii(user_access_key, decrypted_pii) + profile.update(deactivation_reason: nil, active: true) + profile.save! + personal_key + end + + def profile + @_profile ||= user.decorate.password_reset_profile + end + + def user_access_key + @_uak ||= user.unlock_user_access_key(password) + end +end diff --git a/app/forms/verify_personal_key_form.rb b/app/forms/verify_personal_key_form.rb new file mode 100644 index 00000000000..c718ec1a732 --- /dev/null +++ b/app/forms/verify_personal_key_form.rb @@ -0,0 +1,57 @@ +class VerifyPersonalKeyForm + include ActiveModel::Model + include PersonalKeyValidator + + validates :personal_key, presence: true + validate :validate_personal_key + + attr_accessor :personal_key + attr_reader :user + + def initialize(user:, personal_key:) + @user = user + @personal_key = normalize_personal_key(personal_key) + end + + def submit + extra = {} + success = valid? + + if success + extra[:decrypted_pii] = decrypted_pii_json + else + reset_sensitive_fields + end + + FormResponse.new(success: valid?, errors: errors, extra: extra) + end + + private + + def decrypted_pii_json + decrypted_pii.to_json + end + + def password_reset_profile + user.decorate.password_reset_profile + end + + def decrypted_pii + @_pii ||= password_reset_profile.recover_pii(personal_key) + end + + def validate_personal_key + return check_personal_key if personal_key_decrypts? + errors.add :personal_key, :personal_key_incorrect + end + + def reset_sensitive_fields + self.personal_key = nil + end + + def personal_key_decrypts? + decrypted_pii.present? + rescue Pii::EncryptionError => _err + false + end +end diff --git a/app/services/personal_key_formatter.rb b/app/services/personal_key_formatter.rb index 6fec9ecb7f7..776a0b9f0c6 100644 --- a/app/services/personal_key_formatter.rb +++ b/app/services/personal_key_formatter.rb @@ -12,4 +12,8 @@ def self.regexp def self.regexp_string REGEXP end + + def self.code_length + CHAR_COUNT * WORD_COUNT + (WORD_COUNT - 1) + end end diff --git a/app/views/accounts/_password_reset.html.slim b/app/views/accounts/_password_reset.html.slim index ab541b0f1e8..4b54b67f917 100644 --- a/app/views/accounts/_password_reset.html.slim +++ b/app/views/accounts/_password_reset.html.slim @@ -1,3 +1,3 @@ .mb4.alert.alert-warning p = t('account.index.reactivation.instructions') - p.mb0 = link_to t('account.index.reactivation.link'), manage_reactivate_account_path + p.mb0 = link_to t('account.index.reactivation.link'), reactivate_account_path diff --git a/app/views/reactivate_account/index.html.slim b/app/views/reactivate_account/index.html.slim index 845f0f0b6aa..37a09dcee96 100644 --- a/app/views/reactivate_account/index.html.slim +++ b/app/views/reactivate_account/index.html.slim @@ -15,8 +15,8 @@ p.mb0 = t('instructions.account.reactivate.explanation') .block-center.center.col-10 .col-12.mb2 - = link_to t('links.account.reactivate.with_key'), reactivate_account_path, + = link_to t('links.account.reactivate.with_key'), verify_personal_key_path, class: 'btn btn-primary block' - = form_tag manage_reactivate_account_path, method: :put, class: 'col-12' do + = form_tag reactivate_account_path, method: :put, class: 'col-12' do = button_tag t('links.account.reactivate.without_key'), type: 'submit', class: 'btn btn-secondary block col-12' diff --git a/app/views/shared/_personal_key_confirmation_modal.html.slim b/app/views/shared/_personal_key_confirmation_modal.html.slim index ba53666619c..3552b5b2dc3 100644 --- a/app/views/shared/_personal_key_confirmation_modal.html.slim +++ b/app/views/shared/_personal_key_confirmation_modal.html.slim @@ -8,16 +8,7 @@ = t('users.personal_key.confirmation_error') form(id='confirm-key' method='post' action="#{update_path}" name="key-confirm") - input( - aria-label="#{t('forms.personal_key.confirmation_label')}" - maxlength="#{code.length}" - autocomplete='off' - class='col-12 mb4 border-dashed field monospace personal-key caps' - name="personal-key" - pattern="^#{PersonalKeyFormatter.regexp_string}$" - required=true - spellcheck='false' - type='text') + = render 'shared/personal_key_input', code: code = hidden_field_tag :authenticity_token, form_authenticity_token .clearfix.mxn2 diff --git a/app/views/shared/_personal_key_input.html.slim b/app/views/shared/_personal_key_input.html.slim new file mode 100644 index 00000000000..9144a329d67 --- /dev/null +++ b/app/views/shared/_personal_key_input.html.slim @@ -0,0 +1,10 @@ +input( + aria-label="#{t('forms.personal_key.confirmation_label')}" + maxlength="#{PersonalKeyFormatter.code_length}" + autocomplete='off' + class='col-12 mb4 border-dashed field monospace personal-key caps' + name="personal_key" + pattern="^#{PersonalKeyFormatter.regexp_string}$" + required=true + spellcheck='false' + type='text') diff --git a/app/views/shared/_pii_review.html.slim b/app/views/shared/_pii_review.html.slim new file mode 100644 index 00000000000..13b5ae8d0fd --- /dev/null +++ b/app/views/shared/_pii_review.html.slim @@ -0,0 +1,19 @@ +.pl5 + .h6 = t('idv.review.full_name') + .h4.bold.ico-absolute.ico-absolute-success + = "#{pii[:first_name]} #{pii[:last_name]}" + + .mt3.h6 = t('idv.review.mailing_address') + .h4.bold.ico-absolute.ico-absolute-success + = render 'shared/address', address: pii + .mt3.h6 = t('idv.review.dob') + .h4.bold.ico-absolute.ico-absolute-success #{pii[:dob].to_date.to_formatted_s(:long)} + + .mt3.h6 = t('idv.review.ssn') + .h4.bold.ico-absolute.ico-absolute-success #{pii[:ssn]} + + .h6.mt3 = t('idv.messages.phone.phone_of_record') + .h4.bold.ico-absolute.ico-absolute-success #{pii[:phone]} + + .mt3.mb3 + = link_to t('idv.messages.review.financial_info'), MarketingSite.help_url diff --git a/app/views/shared/_user_verify_password.html.slim b/app/views/shared/_user_verify_password.html.slim new file mode 100644 index 00000000000..fb16c017c4c --- /dev/null +++ b/app/views/shared/_user_verify_password.html.slim @@ -0,0 +1,4 @@ += simple_form_for(current_user, url: update_path, + html: { autocomplete: 'off', method: :put, role: 'form' }) do |f| + = f.input :password, label: t('idv.form.password'), required: true + = f.button :submit, t('forms.buttons.submit.default'), class: 'btn btn-primary btn-wide' diff --git a/app/views/users/reactivate_account/index.html.slim b/app/views/users/reactivate_account/index.html.slim deleted file mode 100644 index d56954d0e6d..00000000000 --- a/app/views/users/reactivate_account/index.html.slim +++ /dev/null @@ -1,13 +0,0 @@ -- title t('titles.reactivate_account') - -h1.h3.my0 = t('forms.reactivate_profile.title') -p.mt-tiny.mb0 = t('forms.reactivate_profile.instructions') -= simple_form_for(@reactivate_account_form, url: reactivate_account_path, - html: { autocomplete: 'off', method: :post, role: 'form' }) do |f| - = f.error :base - = render 'partials/personal_key/entry_fields', f: f, attribute_name: :personal_key - = f.input :password, required: true - = f.button :submit, t('forms.reactivate_profile.submit'), class: 'mb1' - -.mt2.pt1.border-top - = link_to t('links.cancel'), account_path diff --git a/app/views/users/verify_password/new.html.slim b/app/views/users/verify_password/new.html.slim new file mode 100644 index 00000000000..20ea38eab25 --- /dev/null +++ b/app/views/users/verify_password/new.html.slim @@ -0,0 +1,9 @@ +- title t('idv.titles.review') + +h1.h3.my0 = t('idv.titles.session.review') + += render 'shared/user_verify_password', update_path: update_verify_password_path + +.mt4 + = accordion('review-verified-info', t('idv.messages.review.intro')) do + = render 'shared/pii_review', pii: @verify_password_form.decrypted_pii diff --git a/app/views/users/verify_personal_key/new.html.slim b/app/views/users/verify_personal_key/new.html.slim new file mode 100644 index 00000000000..83f9dc3724b --- /dev/null +++ b/app/views/users/verify_personal_key/new.html.slim @@ -0,0 +1,16 @@ +.relative + form(id='verify-key' method='post' action="#{create_verify_personal_key_path}" name="verify-key") + .pt4.px1.sm-px5.key-badge.border.border-dashed.border-red.rounded-xl + h3.mt0.mb2 = t('forms.personal_key.title') + p.mb3 = t('forms.personal_key.instructions') + + = render 'shared/personal_key_input', code: '' + = hidden_field_tag :authenticity_token, form_authenticity_token + .mt2 + = t('forms.personal_key.alternative') + = button_to(t('links.reverify'), { method: :put, + url: reactivate_account_path }, class: 'btn btn-link ml1') + .mt4 + = button_tag t('forms.buttons.continue'), type: 'submit', class: 'btn btn-primary btn-wide' + += render 'shared/cancel', link: account_path diff --git a/app/views/verify/review/new.html.slim b/app/views/verify/review/new.html.slim index 7c33a27e80c..4cd7d7768d6 100644 --- a/app/views/verify/review/new.html.slim +++ b/app/views/verify/review/new.html.slim @@ -2,29 +2,8 @@ h1.h3.my0 = t('idv.titles.session.review') -= simple_form_for(current_user, url: verify_review_path, - html: { autocomplete: 'off', method: :put, role: 'form' }) do |f| - = f.input :password, label: t('idv.form.password'), required: true - = f.button :submit, t('forms.buttons.submit.default'), class: 'btn btn-primary btn-wide' += render 'shared/user_verify_password', update_path: verify_review_path .mt4 = accordion('review-verified-info', t('idv.messages.review.intro')) do - .pl5 - .h6 = t('idv.review.full_name') - .h4.bold.ico-absolute.ico-absolute-success - = "#{@idv_params[:first_name]} #{@idv_params[:last_name]}" - - .mt3.h6 = t('idv.review.mailing_address') - .h4.bold.ico-absolute.ico-absolute-success - = render 'shared/address', address: @idv_params - .mt3.h6 = t('idv.review.dob') - .h4.bold.ico-absolute.ico-absolute-success #{@idv_params[:dob].to_date.to_formatted_s(:long)} - - .mt3.h6 = t('idv.review.ssn') - .h4.bold.ico-absolute.ico-absolute-success #{@idv_params[:ssn]} - - .h6.mt3 = t('idv.messages.phone.phone_of_record').capitalize - .h4.bold.ico-absolute.ico-absolute-success #{@idv_params[:phone]} - - .mt3.mb3 - = link_to t('idv.messages.review.financial_info'), MarketingSite.help_url + = render 'shared/pii_review', pii: @idv_params diff --git a/config/locales/forms/en.yml b/config/locales/forms/en.yml index fe6a908efc5..480bfc9b794 100644 --- a/config/locales/forms/en.yml +++ b/config/locales/forms/en.yml @@ -22,11 +22,8 @@ en: labels: password: New password show: Show password - reactivate_profile: - instructions: Provide the information below to reactivate your account. - submit: Reactivate profile - title: Reactivate profile personal_key: + alternative: Don't have your personal key? title: Enter your personal key instructions: Please confirm you have a copy of your personal key by entering it below. confirmation_label: Personal key diff --git a/config/locales/forms/es.yml b/config/locales/forms/es.yml index 3320f316d6c..6facc87038c 100644 --- a/config/locales/forms/es.yml +++ b/config/locales/forms/es.yml @@ -22,11 +22,8 @@ es: labels: password: Nueva contraseña show: NOT TRANSLATED YET - reactivate_profile: - instructions: Proveer la siguiente información para reactivar su cuenta. - submit: Reactivar perfil - title: Reactivar perfil personal_key: + alternative: NOT TRANSLATED YET title: NOT TRANSLATED YET instructions: NOT TRANSLATED YET confirmation_label: NOT TRANSLATED YET diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index ce747d86ecb..fd3104718a8 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -134,7 +134,7 @@ en: in_your_name: in your name or a family member's name intro: We can only send a confirmation code to a telephone number linked to your legal identity. - phone_of_record: phone of record + phone_of_record: Phone of record prepaid: on a contract, not prepaid same_as_2fa: > This phone number can be the same one you used to set up your one-time diff --git a/config/locales/links/en.yml b/config/locales/links/en.yml index 9ac5c7482cf..e70bd4206fe 100644 --- a/config/locales/links/en.yml +++ b/config/locales/links/en.yml @@ -20,6 +20,7 @@ en: privacy_policy: Privacy & security remove: Remove resend: Resend email + reverify: Please verify your identity again. sign_in: Sign in sign_out: Sign out cancel: Cancel diff --git a/config/locales/links/es.yml b/config/locales/links/es.yml index 82e7ddbfe50..da1070539d6 100644 --- a/config/locales/links/es.yml +++ b/config/locales/links/es.yml @@ -20,6 +20,7 @@ es: privacy_policy: Privacidad y seguridad remove: NOT TRANSLATED YET resend: Enviar de nuevo + reverify: NOT TRANSLATED YET sign_in: Iniciar sesión sign_out: Cerrar sesión cancel: Cancelar diff --git a/config/locales/notices/en.yml b/config/locales/notices/en.yml index e51b75ba272..943e54acbc4 100644 --- a/config/locales/notices/en.yml +++ b/config/locales/notices/en.yml @@ -1,6 +1,7 @@ --- en: notices: + account_recovery: Great! You have your personal key. dap_html: > @@ -24,7 +25,7 @@ en: continue: Keep me signed in sign_out: Sign me out message_html: > - For your security, we will sign you out in %{time_left_in_session} unless + For your security, we will sign you out in %{time_left_in_session} unless you tell us otherwise. partially_signed_in: continue: Continue signing in diff --git a/config/locales/notices/es.yml b/config/locales/notices/es.yml index 9c3217130d2..46c9667b8f6 100644 --- a/config/locales/notices/es.yml +++ b/config/locales/notices/es.yml @@ -1,6 +1,7 @@ --- es: notices: + account_recovery: NOT TRANSLATED YET dap_html: NOT TRANSLATED YET forgot_password: use_diff_email: diff --git a/config/routes.rb b/config/routes.rb index 9f4ab0e068a..9e81f39e9fa 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -45,10 +45,14 @@ as: :openid_connect_configuration get '/account' => 'accounts#show' - get '/account/reactivate' => 'users/reactivate_account#index', as: :reactivate_account - post '/account/reactivate' => 'users/reactivate_account#create' - get '/account/reactivate/start' => 'reactivate_account#index', as: :manage_reactivate_account + get '/account/reactivate/start' => 'reactivate_account#index', as: :reactivate_account put '/account/reactivate/start' => 'reactivate_account#update' + get '/account/reactivate/verify_password' => 'users/verify_password#new', as: :verify_password + put '/account/reactivate/verify_password' => 'users/verify_password#update', as: :update_verify_password + get '/account/reactivate/verify_personal_key' => 'users/verify_personal_key#new', + as: :verify_personal_key + post '/account/reactivate/verify_personal_key' => 'users/verify_personal_key#create', + 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' diff --git a/spec/controllers/users/reactivate_account_controller_spec.rb b/spec/controllers/users/reactivate_account_controller_spec.rb deleted file mode 100644 index 38337b04a9c..00000000000 --- a/spec/controllers/users/reactivate_account_controller_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'rails_helper' - -RSpec.describe Users::ReactivateAccountController do - include Features::LocalizationHelper - - describe '#create' do - subject(:action) do - post( - :create, - reactivate_account_form: { - password: 'password', - personal_key: 'personal_key', - } - ) - end - - before do - stub_sign_in - - form = instance_double('ReactivateAccountForm', submit: success) - expect(controller).to receive(:build_reactivate_account_form).and_return(form) - end - - context 'with a valid form' do - let(:success) { true } - - it 'redirects to the profile page' do - action - - expect(response).to redirect_to(account_path) - end - end - - context 'with an invalid form' do - let(:success) { false } - - it 'renders the index page to show errors' do - action - - expect(response).to render_template('users/reactivate_account/index') - end - end - end -end diff --git a/spec/controllers/users/verify_password_controller_spec.rb b/spec/controllers/users/verify_password_controller_spec.rb new file mode 100644 index 00000000000..b6709dca53d --- /dev/null +++ b/spec/controllers/users/verify_password_controller_spec.rb @@ -0,0 +1,71 @@ +require 'rails_helper' + +describe Users::VerifyPasswordController do + let(:user) { create(:user, profiles: profiles, personal_key: personal_key) } + let(:profiles) { [] } + let(:recovery_hash) { { personal_key: personal_key } } + let(:personal_key) { 'key' } + + before do + stub_sign_in(user) + subject.user_session[:account_recovery] = recovery_hash + end + + context 'without password_reset_profile' do + describe '#new' do + it 'redirects user to the home page' do + get :new + + expect(response).to redirect_to(root_url) + end + end + end + + context 'with password reset profile' do + let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] } + let(:response_ok) { FormResponse.new(success: true, errors: {}, extra: { personal_key: key }) } + let(:response_bad) { FormResponse.new(success: false, errors: {}) } + let(:key) { 'key' } + + describe '#new' do + it 'renders the `new` template' do + get :new + + expect(response).to render_template(:new) + end + end + + describe '#update' do + let(:form) { instance_double(VerifyPasswordForm) } + + before do + expect(controller).to receive(:verify_password_form).and_return(form) + end + + context 'with valid password' do + before do + allow(form).to receive(:submit).and_return(response_ok) + put :update, user: { password: user.password } + end + + it 'redirects to the account page' do + expect(response).to redirect_to(account_url) + end + + it 'sets a new personal key as a flash message' do + expect(flash[:personal_key]).to eq(key) + end + end + + context 'without valid password' do + it 'renders the new template' do + allow(form).to receive(:submit).and_return(response_bad) + + put :update, user: { password: user.password } + + expect(response).to render_template(:new) + end + end + end + end +end diff --git a/spec/controllers/users/verify_personal_key_controller_spec.rb b/spec/controllers/users/verify_personal_key_controller_spec.rb new file mode 100644 index 00000000000..6b9a73ab4c1 --- /dev/null +++ b/spec/controllers/users/verify_personal_key_controller_spec.rb @@ -0,0 +1,90 @@ +require 'rails_helper' + +describe Users::VerifyPersonalKeyController do + let(:user) { create(:user, profiles: profiles, personal_key: personal_key) } + let(:profiles) { [] } + let(:personal_key) { 'key' } + + before { stub_sign_in(user) } + + describe 'before actions' do + it 'only allows 2fa users through' do + expect(subject).to have_actions(:confirm_two_factor_authenticated) + end + end + + describe '#new' do + context 'without password_reset_profile' do + it 'redirects user to the home page' do + get :new + expect(response).to redirect_to(root_url) + end + end + + context 'with password reset profile' do + let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] } + + it 'renders the `new` template' do + get :new + + expect(response).to render_template(:new) + end + + it 'displays a flash message to the user' do + get :new + + expect(subject.flash[:notice]).to eq(t('notices.account_recovery')) + end + end + end + + describe '#create' do + let(:profiles) { [create(:profile, deactivation_reason: :password_reset)] } + let(:form) { instance_double(VerifyPersonalKeyForm) } + let(:error_text) { 'bad_key' } + let(:personal_key_error) { { personal_key: [error_text] } } + let(:response_ok) { FormResponse.new(success: true, errors: {}) } + let(:response_bad) { FormResponse.new(success: false, errors: personal_key_error, extra: {}) } + + context 'wth a valid form' do + before do + allow(VerifyPersonalKeyForm).to receive(:new). + with(user: subject.current_user, personal_key: personal_key). + and_return(form) + allow(form).to receive(:submit).and_return(response_ok) + end + + it 'redirects to the next step of the account recovery flow' do + post :create, personal_key: personal_key + + expect(response).to redirect_to(verify_password_url) + end + + it 'stores that the personal key was entered in the user session' do + post :create, personal_key: personal_key + + expect(subject.user_session[:account_recovery][:personal_key]).to eq(true) + end + end + + context 'with an invalid form' do + let(:bad_key) { 'baaad' } + + before do + allow(VerifyPersonalKeyForm).to receive(:new). + with(user: subject.current_user, personal_key: bad_key). + and_return(form) + allow(form).to receive(:submit).and_return(response_bad) + post :create, personal_key: bad_key + end + + it 'sets an error in the flash' do + expect(flash[:error]).to eq(error_text) + end + + it 'renders the new template' do + expect(response).to render_template(:new) + end + end + end +end diff --git a/spec/controllers/verify_controller_spec.rb b/spec/controllers/verify_controller_spec.rb index ca51bf85787..d35ffcb8dff 100644 --- a/spec/controllers/verify_controller_spec.rb +++ b/spec/controllers/verify_controller_spec.rb @@ -42,7 +42,7 @@ get :index - expect(response).to redirect_to manage_reactivate_account_url + expect(response).to redirect_to reactivate_account_url end end diff --git a/spec/features/saml/loa1_sso_spec.rb b/spec/features/saml/loa1_sso_spec.rb index 93d17e37cdd..b74e07152fa 100644 --- a/spec/features/saml/loa1_sso_spec.rb +++ b/spec/features/saml/loa1_sso_spec.rb @@ -174,6 +174,6 @@ def stub_personal_key(user:, code:) end def enter_personal_key_words_on_modal(code) - fill_in 'personal-key', with: code + fill_in 'personal_key', with: code end end 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 b6f33f6adb6..8bb181b121c 100644 --- a/spec/features/users/password_recovery_via_recovery_code_spec.rb +++ b/spec/features/users/password_recovery_via_recovery_code_spec.rb @@ -7,7 +7,7 @@ let(:new_password) { 'some really awesome new password' } let(:pii) { { ssn: '666-66-1234', dob: '1920-01-01', first_name: 'alice' } } - scenario 'resets password and reactivates profile with personal key', email: true do + scenario 'resets password and reactivates profile with personal key', email: true, js: true do personal_key = personal_key_from_pii(user, pii) trigger_reset_password_and_click_email_link(user.email) @@ -16,10 +16,6 @@ click_submit_default enter_correct_otp_code_for_user(user) - expect(current_path).to eq manage_reactivate_account_path - - click_on t('links.account.reactivate.with_key') - expect(current_path).to eq reactivate_account_path reactivate_profile(new_password, personal_key) @@ -44,7 +40,9 @@ expect(current_path).to eq reactivate_account_path - reactivate_profile(new_password, new_personal_key) + click_on t('links.account.reactivate.with_key') + fill_in 'personal_key', with: new_personal_key + click_on t('forms.buttons.continue') expect(page).to have_content t('errors.messages.personal_key_incorrect') end @@ -85,8 +83,16 @@ def scrape_personal_key end def reactivate_profile(password, personal_key) + click_on t('links.account.reactivate.with_key') + + expect(current_path).to eq verify_personal_key_path + + fill_in 'personal_key', with: personal_key + click_on t('forms.buttons.continue') + + expect(current_path).to eq verify_password_path + fill_in 'Password', with: password - enter_personal_key(personal_key: personal_key) - click_button t('forms.reactivate_profile.submit') + click_on t('forms.buttons.submit.default') end end diff --git a/spec/features/users/regenerate_personal_key_spec.rb b/spec/features/users/regenerate_personal_key_spec.rb index 26927907574..dcd88ffc7e1 100644 --- a/spec/features/users/regenerate_personal_key_spec.rb +++ b/spec/features/users/regenerate_personal_key_spec.rb @@ -119,7 +119,7 @@ def expect_accordion_content_to_become_visible def expect_confirmation_modal_to_appear_with_first_code_field_in_focus expect(page).not_to have_xpath("//div[@id='personal-key-confirm'][@class='display-none']") expect(page).not_to have_xpath("//#{invisible_selector}[@id='personal-key']") - expect(page.evaluate_script('document.activeElement.name')).to eq 'personal-key' + expect(page.evaluate_script('document.activeElement.name')).to eq 'personal_key' end def press_shift_tab diff --git a/spec/forms/reactivate_account_form_spec.rb b/spec/forms/reactivate_account_form_spec.rb deleted file mode 100644 index 3fd7ba23d57..00000000000 --- a/spec/forms/reactivate_account_form_spec.rb +++ /dev/null @@ -1,99 +0,0 @@ -require 'rails_helper' - -describe ReactivateAccountForm do - subject(:form) do - ReactivateAccountForm.new(user, - password: password) - end - - let(:user) { build_stubbed(:user) } - let(:personal_key) { nil } - let(:password) { nil } - - describe '#valid?' do - let(:password) { 'asd' } - let(:personal_key) { 'foo' } - let(:valid_personal_key?) { true } - let(:valid_password?) { true } - let(:personal_key_decrypts?) { true } - - before do - allow(form).to receive(:valid_password?).and_return(valid_password?) - allow(form).to receive(:valid_personal_key?).and_return(valid_personal_key?) - allow(form).to receive(:personal_key_decrypts?).and_return(personal_key_decrypts?) - end - - context 'when required attributes are not present' do - let(:password) { nil } - let(:personal_key) { nil } - - it 'is invalid' do - expect(subject).to_not be_valid - expect(subject.errors[:personal_key]).to include t('errors.messages.blank') - expect(subject.errors[:password]).to eq [t('errors.messages.blank')] - end - end - - context 'when there is no profile that has had its password reset' do - let(:password_reset_profile) { nil } - - it 'is invalid' do - expect(subject).to_not be_valid - expect(subject.errors[:base]).to eq [t('errors.messages.no_password_reset_profile')] - end - end - - context 'when personal key does not match' do - subject(:form) do - ReactivateAccountForm.new(user, - personal_key: personal_key, - password: password) - end - - let(:valid_personal_key?) { false } - - it 'is invalid' do - expect(subject).to_not be_valid - expect(subject.errors[:personal_key]).to eq [t('errors.messages.personal_key_incorrect')] - end - end - - context 'when password does not match' do - let(:valid_password?) { false } - - it 'is invalid' do - expect(subject).to_not be_valid - expect(subject.errors[:password]).to eq [t('errors.messages.password_incorrect')] - end - end - end - - describe '#submit' do - before { expect(form).to receive(:valid?).and_return(valid?) } - - let(:flash) { {} } - - context 'with a valid form' do - let(:valid?) { true } - - it 're-encrypts the PII and sets the personal key in the flash' do - personal_key = '555' - expect(form).to receive(:reencrypt_pii).and_return(personal_key) - - form.submit(flash) - - expect(flash[:personal_key]).to eq(personal_key) - end - end - - context 'with an invalid form' do - let(:valid?) { false } - - it 'clears the password' do - form.submit(flash) - - expect(form.password).to be_nil - end - end - end -end diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 4174dbce389..80a6bec46bc 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -162,7 +162,7 @@ def acknowledge_and_confirm_personal_key click_on button_text, class: 'personal-key-continue' - fill_in 'personal-key', with: code_words.join('-').downcase + extra_characters_get_ignored + fill_in 'personal_key', with: code_words.join('-').downcase + extra_characters_get_ignored click_on button_text, class: 'personal-key-confirm' end diff --git a/spec/views/accounts/show.html.slim_spec.rb b/spec/views/accounts/show.html.slim_spec.rb index 4d6c9e1c826..73851ae246c 100644 --- a/spec/views/accounts/show.html.slim_spec.rb +++ b/spec/views/accounts/show.html.slim_spec.rb @@ -93,7 +93,7 @@ render expect(rendered).to have_link(t('account.index.reactivation.link'), - href: manage_reactivate_account_path) + href: reactivate_account_path) end end