diff --git a/app/forms/reactivate_profile_form.rb b/app/forms/reactivate_profile_form.rb index 0a64c4bb2f8..f6c79f70640 100644 --- a/app/forms/reactivate_profile_form.rb +++ b/app/forms/reactivate_profile_form.rb @@ -55,7 +55,7 @@ def validate_password end def validate_recovery_code - return if recovery_code.blank? || valid_recovery_code? + return if recovery_code.blank? || (valid_recovery_code? && recovery_code_decrypts?) errors.add :recovery_code, :recovery_code_incorrect end @@ -68,6 +68,12 @@ def valid_password? user.valid_password?(password) end + def recovery_code_decrypts? + decrypted_pii.present? + rescue Pii::EncryptionError => _err + false + end + def valid_recovery_code? RecoveryCodeGenerator.new(user).verify(recovery_code) end diff --git a/app/views/partials/personal_key/_key.slim b/app/views/partials/personal_key/_key.slim index 2de1956b0a6..f7d71f43eed 100644 --- a/app/views/partials/personal_key/_key.slim +++ b/app/views/partials/personal_key/_key.slim @@ -7,7 +7,7 @@ = t('users.recovery_code.header') .navy.monospace - code.split(' ').each do |word| - p.bold.mb0.h3 + p.bold.mb0.h3 data-recovery='word' = word .left.h5.mt2 diff --git a/app/views/profile/index.html.slim b/app/views/profile/index.html.slim index 413f4a79c6d..52b73e66ac8 100644 --- a/app/views/profile/index.html.slim +++ b/app/views/profile/index.html.slim @@ -85,6 +85,7 @@ h2.h3.my0 = link_to authenticator_start_url, class: btn_cls do span.hide = t('.authentication_app') = t('forms.buttons.enable') +- unless @has_password_reset_profile .py-12p.border-top.border-bottom .clearfix.mxn1 .sm-col.sm-col-10.px1 = t('profile.items.recovery_code') diff --git a/spec/features/visitors/password_recovery_spec.rb b/spec/features/visitors/password_recovery_spec.rb index f15b52d2471..4dc4357cf71 100644 --- a/spec/features/visitors/password_recovery_spec.rb +++ b/spec/features/visitors/password_recovery_spec.rb @@ -225,30 +225,69 @@ def reset_password_and_sign_back_in(user, password = 'a really long password') end end - scenario 'LOA3 user resets password and reactivates profile with recovery code', email: true do - user = create(:user, :signed_up) + context 'LOA3 user' do + let(:user) { create(:user, :signed_up) } + let(:new_password) { 'some really awesome new password' } + let(:pii) { { ssn: '666-66-1234', dob: '1920-01-01' } } - recovery_code = recovery_code_from_pii(user, ssn: '1234', dob: '1920-01-01') - password = 'some really awesome new password' + scenario 'resets password and reactivates profile with recovery code', email: true do + recovery_code = recovery_code_from_pii(user, pii) - visit new_user_password_path - fill_in 'Email', with: user.email - click_button t('forms.buttons.continue') - open_last_email - click_email_link_matching(/reset_password_token/) + visit new_user_password_path + fill_in 'Email', with: user.email + click_button t('forms.buttons.continue') + open_last_email + click_email_link_matching(/reset_password_token/) + + reset_password_and_sign_back_in(user, new_password) + click_submit_default + enter_correct_otp_code_for_user(user) + + expect(page).to have_content t('profile.index.reactivation.instructions') + click_link t('profile.index.reactivation.reactivate_button') + + fill_in 'Password', with: new_password + fill_in 'Recovery code', with: recovery_code + click_button t('forms.reactivate_profile.submit') + + expect(page).to have_content t('idv.messages.recovery_code') + end - reset_password_and_sign_back_in(user, password) - click_submit_default - enter_correct_otp_code_for_user(user) + scenario 'resets password, makes recovery code, attempts reactivate profile', email: true do + _recovery_code = recovery_code_from_pii(user, pii) - expect(page).to have_content t('profile.index.reactivation.instructions') - click_link t('profile.index.reactivation.reactivate_button') + visit new_user_password_path + fill_in 'Email', with: user.email + click_button t('forms.buttons.continue') + open_last_email + click_email_link_matching(/reset_password_token/) + + reset_password_and_sign_back_in(user, new_password) + click_submit_default + enter_correct_otp_code_for_user(user) + + expect(page).to have_content t('profile.index.reactivation.instructions') - fill_in 'Password', with: password - fill_in 'Recovery code', with: recovery_code - click_button t('forms.reactivate_profile.submit') + visit manage_recovery_code_path - expect(page).to have_content t('idv.messages.recovery_code') + new_recovery_code_words = [] + page.all(:css, 'p[data-recovery="word"]').each do |node| + new_recovery_code_words << node.text + end + new_recovery_code = new_recovery_code_words.join(' ') + + click_on(t('forms.buttons.continue')) + + expect(page).to have_content t('profile.index.reactivation.instructions') + + click_link t('profile.index.reactivation.reactivate_button') + + fill_in 'Password', with: new_password + fill_in 'Recovery code', with: new_recovery_code + click_button t('forms.reactivate_profile.submit') + + expect(page).to have_content t('errors.messages.recovery_code_incorrect') + end end scenario 'user takes too long to click the reset password link' do diff --git a/spec/forms/reactivate_profile_form_spec.rb b/spec/forms/reactivate_profile_form_spec.rb index f3b37872f38..51263f396f9 100644 --- a/spec/forms/reactivate_profile_form_spec.rb +++ b/spec/forms/reactivate_profile_form_spec.rb @@ -16,10 +16,12 @@ let(:recovery_code) { '123' } let(:valid_recovery_code?) { true } let(:valid_password?) { true } + let(:recovery_code_decrypts?) { true } before do allow(form).to receive(:valid_password?).and_return(valid_password?) allow(form).to receive(:valid_recovery_code?).and_return(valid_recovery_code?) + allow(form).to receive(:recovery_code_decrypts?).and_return(recovery_code_decrypts?) end context 'when required attributes are not present' do diff --git a/spec/views/profile/index.html.slim_spec.rb b/spec/views/profile/index.html.slim_spec.rb index 3ce2da92142..4f349f96585 100644 --- a/spec/views/profile/index.html.slim_spec.rb +++ b/spec/views/profile/index.html.slim_spec.rb @@ -42,15 +42,39 @@ end end - it 'contains a recovery code section' do - user = User.new - allow(view).to receive(:current_user).and_return(user) + context 'when has_password_reset_profile is false' do + before do + assign(:has_password_reset_profile, false) + end - render + it 'contains a recovery code section' do + user = User.new + allow(view).to receive(:current_user).and_return(user) + + render + + expect(rendered).to have_content t('profile.items.recovery_code') + expect(rendered). + to have_link(t('profile.links.regenerate_recovery_code'), href: manage_recovery_code_path) + end + end + + context 'when has_password_reset_profile is true' do + before do + assign(:has_password_reset_profile, true) + end - expect(rendered).to have_content t('profile.items.recovery_code') - expect(rendered). - to have_link(t('profile.links.regenerate_recovery_code'), href: manage_recovery_code_path) + it 'lacks a recovery code section' do + user = User.new + allow(view).to receive(:current_user).and_return(user) + + render + + expect(rendered).to_not have_content t('profile.items.recovery_code') + expect(rendered).to_not have_link( + t('profile.links.regenerate_recovery_code'), href: manage_recovery_code_path + ) + end end it 'contains account history' do