Fix double personal key encryption when using personal key as second factor

app/controllers/concerns/personal_key_concern.rb

@@ -14,7 +14,7 @@ def create_new_code
       Pii::ReEncryptor.new(user: current_user, user_session: user_session).perform
       active_profile.personal_key
     else
-      PersonalKeyGenerator.new(current_user).create
+      PersonalKeyGenerator.new(current_user).generate!
     end
   end
 

app/controllers/two_factor_authentication/personal_key_verification_controller.rb

@@ -66,8 +66,7 @@ def alert_user_about_personal_key_sign_in(disavowal_token)
 
     def remove_personal_key
       # for now we will regenerate a key and not show it to them so retire personal key page shows
-      current_user.personal_key = PersonalKeyGenerator.new(current_user).create
-      current_user.save!
+      PersonalKeyGenerator.new(current_user).generate!
       user_session.delete(:personal_key)
     end
 

app/models/profile.rb

@@ -279,7 +279,7 @@ def encrypt_pii(pii, password)
 
   # @param [Pii::Attributes] pii
   def encrypt_recovery_pii(pii, personal_key: nil)
-    personal_key ||= personal_key_generator.create
+    personal_key ||= personal_key_generator.generate!
     encryptor = Encryption::Encryptors::PiiEncryptor.new(
       personal_key_generator.normalize(personal_key),
     )

app/services/personal_key_generator.rb

@@ -10,7 +10,7 @@ def initialize(user, length: 4)
     @length = length
   end
 
-  def create
+  def generate!
     user.personal_key = raw_personal_key
     user.save!
     raw_personal_key.tr(' ', '-')

spec/controllers/accounts/personal_keys_controller_spec.rb

@@ -31,7 +31,7 @@
       allow(PersonalKeyGenerator).to receive(:new).
         with(subject.current_user).and_return(generator)
 
-      expect(generator).to receive(:create)
+      expect(generator).to receive(:generate!)
 
       post :create
 

spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb

@@ -31,7 +31,7 @@
     it 'redirects to the two_factor_options page if user is IAL2' do
       profile = create(:profile, :active, :verified, pii: { ssn: '1234' })
       user = profile.user
-      PersonalKeyGenerator.new(user).create
+      PersonalKeyGenerator.new(user).generate!
       stub_sign_in_before_2fa(user)
       get :show
 
@@ -43,7 +43,7 @@
   describe '#create' do
     context 'when the user enters a valid personal key' do
       let(:user) { create(:user, :with_phone) }
-      let(:personal_key) { { personal_key: PersonalKeyGenerator.new(user).create } }
+      let(:personal_key) { { personal_key: PersonalKeyGenerator.new(user).generate! } }
       let(:payload) { { personal_key_form: personal_key } }
       it 'tracks the valid authentication event' do
         personal_key
@@ -134,7 +134,7 @@
 
     it 'does generate a new personal key after the user signs in with their old one' do
       user = create(:user)
-      raw_key = PersonalKeyGenerator.new(user).create
+      raw_key = PersonalKeyGenerator.new(user).generate!
       old_key = user.reload.encrypted_recovery_code_digest
       stub_sign_in_before_2fa(user)
       post :create, params: { personal_key_form: { personal_key: raw_key } }
@@ -147,7 +147,7 @@
     it 'redirects to the two_factor_options page if user is IAL2' do
       profile = create(:profile, :active, :verified, pii: { ssn: '1234' })
       user = profile.user
-      raw_key = PersonalKeyGenerator.new(user).create
+      raw_key = PersonalKeyGenerator.new(user).generate!
       stub_sign_in_before_2fa(user)
       post :create, params: { personal_key_form: { personal_key: raw_key } }
 

spec/controllers/users/personal_keys_controller_spec.rb

@@ -48,7 +48,7 @@
       allow(PersonalKeyGenerator).to receive(:new).
         with(subject.current_user).and_return(generator)
 
-      expect(generator).to_not receive(:create)
+      expect(generator).to_not receive(:generate!)
 
       get :show
     end

spec/features/two_factor_authentication/sign_in_via_personal_key_spec.rb

@@ -6,7 +6,7 @@
       :user, :fully_registered, :with_phone, :with_personal_key,
       with: { phone: '+1 (202) 345-6789' }
     )
-    raw_key = PersonalKeyGenerator.new(user).create
+    raw_key = PersonalKeyGenerator.new(user).generate!
     old_key = user.reload.encrypted_recovery_code_digest
 
     sign_in_before_2fa(user)
@@ -39,7 +39,7 @@
         second_factor_attempts_count: IdentityConfig.store.login_otp_confirmation_max_attempts - 1,
       )
       sign_in_before_2fa(user)
-      personal_key = PersonalKeyGenerator.new(user).create
+      personal_key = PersonalKeyGenerator.new(user).generate!
       wrong_personal_key = personal_key.split('-').reverse.join
 
       choose_another_security_option('personal_key')

spec/features/users/sign_in_spec.rb

@@ -665,7 +665,7 @@
     # this can happen if you submit the personal key form multiple times quickly
     it 'does not redirect to the personal key page' do
       user = create(:user, :fully_registered)
-      old_personal_key = PersonalKeyGenerator.new(user).create
+      old_personal_key = PersonalKeyGenerator.new(user).generate!
       signin(user.email, user.password)
       choose_another_security_option('personal_key')
       enter_personal_key(personal_key: old_personal_key)

spec/forms/personal_key_form_spec.rb

@@ -5,7 +5,7 @@
     context 'when the form is valid' do
       it 'returns FormResponse with success: true' do
         user = create(:user)
-        raw_code = PersonalKeyGenerator.new(user).create
+        raw_code = PersonalKeyGenerator.new(user).generate!
         old_code = user.reload.encrypted_recovery_code_digest
 
         form = PersonalKeyForm.new(user, raw_code)

spec/services/personal_key_generator_spec.rb

@@ -12,11 +12,11 @@ def stub_random_phrase
     allow(RandomPhrase).to receive(:new).and_return(random_phrase)
   end
 
-  describe '#create' do
+  describe '#generate!' do
     it 'returns the raw personal key' do
       stub_random_phrase
 
-      expect(generator.create).to eq personal_key.tr(' ', '-')
+      expect(generator.generate!).to eq personal_key.tr(' ', '-')
     end
 
     it 'hashes the raw personal key' do
@@ -25,26 +25,26 @@ def stub_random_phrase
 
       stub_random_phrase
 
-      generator.create
+      generator.generate!
 
       expect(user.encrypted_recovery_code_digest).to_not eq personal_key
     end
 
     it 'generates a phrase of 4 words by default' do
-      expect(generator.create).to match(/\A\w\w\w\w-\w\w\w\w-\w\w\w\w-\w\w\w\w\z/)
+      expect(generator.generate!).to match(/\A\w\w\w\w-\w\w\w\w-\w\w\w\w-\w\w\w\w\z/)
     end
 
     it 'allows length to be configured via ENV var' do
       allow(IdentityConfig.store).to receive(:recovery_code_length).and_return(14)
 
       fourteen_letters_and_spaces_start_end_with_letter = /\A(\w+-){13}\w+\z/
-      expect(generator.create).to match(fourteen_letters_and_spaces_start_end_with_letter)
+      expect(generator.generate!).to match(fourteen_letters_and_spaces_start_end_with_letter)
     end
 
     it 'sets the encrypted recovery code digest' do
       user = create(:user)
       generator = PersonalKeyGenerator.new(user)
-      key = generator.create
+      key = generator.generate!
 
       expect(user.encrypted_recovery_code_digest).to_not be_empty
       expect(generator.verify(key)).to eq(true)
@@ -54,7 +54,7 @@ def stub_random_phrase
   describe '#verify' do
     before do
       stub_random_phrase
-      generator.create
+      generator.generate!
     end
 
     it 'returns false for the wrong code' do

spec/support/shared_examples/sign_in.rb

@@ -128,7 +128,7 @@
 
     set_new_browser_session
 
-    old_personal_key = PersonalKeyGenerator.new(user).create
+    old_personal_key = PersonalKeyGenerator.new(user).generate!
     visit_idp_from_sp_with_ial1(sp)
     trigger_reset_password_and_click_email_link(user.confirmed_email_addresses.first.email)
     fill_in t('forms.passwords.edit.labels.password'), with: new_password
@@ -318,7 +318,7 @@ def user_with_broken_personal_key(scenario)
 
 def ial1_sign_in_with_personal_key_goes_to_sp(sp)
   user = create_ial1_account_go_back_to_sp_and_sign_out(sp)
-  old_personal_key = PersonalKeyGenerator.new(user).create
+  old_personal_key = PersonalKeyGenerator.new(user).generate!
 
   Capybara.reset_sessions!