diff --git a/app/models/concerns/user_access_key_overrides.rb b/app/models/concerns/user_access_key_overrides.rb index cb604ad68e6..fdf0fa55e64 100644 --- a/app/models/concerns/user_access_key_overrides.rb +++ b/app/models/concerns/user_access_key_overrides.rb @@ -27,14 +27,9 @@ def valid_personal_key?(normalized_personal_key) ) end - def personal_key - @personal_key - end - def personal_key=(new_personal_key) - @personal_key = new_personal_key - return if @personal_key.blank? - self.encrypted_recovery_code_digest = Encryption::PasswordVerifier.digest(@personal_key) + return if new_personal_key.blank? + self.encrypted_recovery_code_digest = Encryption::PasswordVerifier.digest(new_personal_key) end # This is a devise method, which we are overriding. This should not be removed diff --git a/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb index 3615ee49fb1..9d1de9b92c0 100644 --- a/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb @@ -65,13 +65,14 @@ it 'generates a new personal key after the user signs in with their old one' do user = create(:user) - old_key = PersonalKeyGenerator.new(user).create + raw_key = PersonalKeyGenerator.new(user).create + old_key = user.reload.encrypted_recovery_code_digest stub_sign_in_before_2fa(user) - post :create, params: { personal_key_form: { personal_key: old_key } } + post :create, params: { personal_key_form: { personal_key: raw_key } } user.reload - expect(user.personal_key).to_not be_nil - expect(user.personal_key).to_not eq old_key + expect(user.encrypted_recovery_code_digest).to_not be_nil + expect(user.encrypted_recovery_code_digest).to_not eq old_key end context 'when the personal key field is empty' do @@ -140,12 +141,13 @@ end it 'does not generate a new personal key if the user enters an invalid key' do - user = create(:user, personal_key: 'ABCD-EFGH-IJKL-MNOP') + user = create(:user, :with_personal_key) + old_key = user.reload.encrypted_recovery_code_digest stub_sign_in_before_2fa(user) post :create, params: payload user.reload - expect(user.personal_key).to eq 'ABCD-EFGH-IJKL-MNOP' + expect(user.encrypted_recovery_code_digest).to eq old_key end end end 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 9d0d05793a0..fa68d848a1a 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 @@ -3,18 +3,16 @@ 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) - sign_in_before_2fa(user) - - personal_key = PersonalKeyGenerator.new(user).create + raw_key = PersonalKeyGenerator.new(user).create + old_key = user.reload.encrypted_recovery_code_digest + sign_in_before_2fa(user) choose_another_security_option('personal_key') - - enter_personal_key(personal_key: personal_key) - + enter_personal_key(personal_key: raw_key) click_submit_default click_acknowledge_personal_key - expect(user.reload.personal_key).to_not eq personal_key + expect(user.reload.encrypted_recovery_code_digest).to_not eq old_key expect(current_path).to eq account_path end diff --git a/spec/forms/personal_key_form_spec.rb b/spec/forms/personal_key_form_spec.rb index 6398f43997d..83120fc7bb9 100644 --- a/spec/forms/personal_key_form_spec.rb +++ b/spec/forms/personal_key_form_spec.rb @@ -6,7 +6,7 @@ it 'returns FormResponse with success: true' do user = create(:user) raw_code = PersonalKeyGenerator.new(user).create - old_key = user.reload.personal_key + old_code = user.reload.encrypted_recovery_code_digest form = PersonalKeyForm.new(user, raw_code) result = instance_double(FormResponse) @@ -15,7 +15,7 @@ expect(FormResponse).to receive(:new). with(success: true, errors: {}, extra: extra).and_return(result) expect(form.submit).to eq result - expect(user.reload.personal_key).to eq old_key + expect(user.reload.encrypted_recovery_code_digest).to eq old_code end end @@ -31,7 +31,7 @@ expect(FormResponse).to receive(:new). with(success: false, errors: errors, extra: extra).and_return(result) expect(form.submit).to eq result - expect(user.personal_key).to_not be_nil + expect(user.encrypted_recovery_code_digest).to_not be_nil expect(form.personal_key).to be_nil end end diff --git a/spec/models/profile_spec.rb b/spec/models/profile_spec.rb index a64f522c80c..35524caa210 100644 --- a/spec/models/profile_spec.rb +++ b/spec/models/profile_spec.rb @@ -30,12 +30,12 @@ it 'generates new personal key' do expect(profile.encrypted_pii_recovery).to be_nil - initial_personal_key = user.personal_key + initial_personal_key = user.encrypted_recovery_code_digest profile.encrypt_pii(pii, user.password) expect(profile.encrypted_pii_recovery).to_not be_nil - expect(user.personal_key).to_not eq initial_personal_key + expect(user.reload.encrypted_recovery_code_digest).to_not eq initial_personal_key end end @@ -43,13 +43,13 @@ it 'generates new personal key' do expect(profile.encrypted_pii_recovery).to be_nil - initial_personal_key = user.personal_key + initial_personal_key = user.encrypted_recovery_code_digest profile.encrypt_recovery_pii(pii) expect(profile.encrypted_pii_recovery).to_not be_nil - expect(user.personal_key).to_not eq initial_personal_key - expect(profile.personal_key).to_not eq user.personal_key + expect(user.reload.encrypted_recovery_code_digest).to_not eq initial_personal_key + expect(profile.personal_key).to_not eq user.encrypted_recovery_code_digest end end