Ab addtl pkey formatting

app/controllers/two_factor_authentication/recovery_code_verification_controller.rb

@@ -5,8 +5,13 @@ class RecoveryCodeVerificationController < ApplicationController
     prepend_before_action :authenticate_user
     skip_before_action :handle_two_factor_authentication
 
+    def show
+      @recovery_code_form = RecoveryCodeForm.new(current_user)
+    end
+
     def create
-      result = RecoveryCodeForm.new(current_user, personal_key).submit
+      @recovery_code_form = RecoveryCodeForm.new(current_user, personal_key)
+      result = @recovery_code_form.submit
 
       analytics.track_event(Analytics::MULTI_FACTOR_AUTH, result.merge(method: 'recovery code'))
 
@@ -38,7 +43,7 @@ def pii
     end
 
     def personal_key
-      params[:code][:words].join(' ')
+      params[:recovery_code_form].require(:code).join(' ')
     end
   end
 end

app/controllers/users/reactivate_profile_controller.rb

@@ -6,7 +6,6 @@ def index
 
     def create
       @reactivate_profile_form = build_reactivate_profile_form
-
       if @reactivate_profile_form.submit(flash)
         redirect_to profile_path
       else
@@ -19,7 +18,7 @@ def create
     def build_reactivate_profile_form
       ReactivateProfileForm.new(
         current_user,
-        params[:reactivate_profile_form].permit(:recovery_code, :password)
+        params[:reactivate_profile_form].permit(:password, recovery_code: [])
       )
     end
   end

app/forms/reactivate_profile_form.rb

@@ -10,8 +10,11 @@ class ReactivateProfileForm
   attr_reader :user
 
   def initialize(user, attrs = {})
+    attrs[:recovery_code] ||= []
     @user = user
     super attrs
+
+    @recovery_code = recovery_code.join(' ')
   end
 
   def submit(flash)

app/forms/recovery_code_form.rb

@@ -1,5 +1,9 @@
 class RecoveryCodeForm
-  def initialize(user, code)
+  include ActiveModel::Model
+
+  attr_accessor :code
+
+  def initialize(user, code = [])
     @user = user
     @code = code
   end
@@ -14,7 +18,7 @@ def submit
 
   private
 
-  attr_reader :user, :code, :success
+  attr_reader :user, :success
 
   def valid_recovery_code?
     recovery_code_generator = RecoveryCodeGenerator.new(user)

app/inputs/array_input.rb

@@ -0,0 +1,34 @@
+class ArrayInput < SimpleForm::Inputs::StringInput
+  # simple_form throws a deprecation warning noting that the
+  # method signature should be changed to add this parameter
+  def input(_wrapper_options)
+    input_html_options[:type] ||= input_type
+
+    existing_values = Array(object.public_send(attribute_name)).map do
+      build
+    end
+
+    existing_values.push(build) if existing_values.empty?
+
+    safe_join(existing_values)
+  end
+
+  def input_type
+    :text
+  end
+
+  def build
+    options = {
+      value: nil,
+      class: 'block col-12 field',
+      autocomplete: 'off',
+      name: "#{object_name}[#{attribute_name}][]",
+    }
+
+    builder.text_field(nil, input_html_options.merge(options))
+  end
+
+  def builder
+    @builder ||= :builder
+  end
+end

app/views/partials/personal_key/_entry_fields.html.slim

@@ -0,0 +1,7 @@
+.mb3
+  label.block.bold#personal-key-label
+    = t('simple_form.required.html') + t('forms.two_factor.recovery_code')
+  - Figaro.env.recovery_code_length.to_i.times do
+    = f.input attribute_name, as: :array, label: false,
+      input_html: { required: true, 'aria-labelledby': 'personal-key-label' }
+  end

app/views/two_factor_authentication/recovery_code_verification/show.html.slim

@@ -3,16 +3,10 @@
 h1.h3.my0 = t('devise.two_factor_authentication.recovery_code_header_text')
 p.mt-tiny.mb0 = t('devise.two_factor_authentication.recovery_code_prompt')
 
-= form_tag(:login_two_factor_recovery_code, method: :post, role: 'form', class: 'mt3 sm-mt4') do
-  label.block.bold#personal-key-label
-    = t('simple_form.required.html') + t('forms.two_factor.recovery_code')
-
-  - Figaro.env.recovery_code_length.to_i.times do |_|
-      .mb2
-        = block_text_field_tag 'code[words][]', '', required: true, autocomplete: 'off',
-          'aria-labelledby': 'personal-key-label'
-  end
-
-  = submit_tag t('forms.buttons.submit.default'), class: 'btn btn-primary mt2'
+= simple_form_for(@recovery_code_form, url: login_two_factor_recovery_code_path,
+  html: { autocomplete: 'off', method: :post, role: 'form' }) do |f|
+  = render 'partials/personal_key/entry_fields', f: f, attribute_name: :code
+  = f.button :submit, t('forms.buttons.submit.default'), class: 'btn btn-primary mt2'
+end
 
 = render 'shared/cancel', link: destroy_user_session_path

app/views/users/reactivate_profile/index.html.slim

@@ -5,6 +5,6 @@ p.mt-tiny.mb0 = t('forms.reactivate_profile.instructions')
 = simple_form_for(@reactivate_profile_form, url: reactivate_profile_path,
   html: { autocomplete: 'off', method: :post, role: 'form' }) do |f|
   = f.error :base
-  = f.input :recovery_code, required: true
+  = render 'partials/personal_key/entry_fields', f: f, attribute_name: :recovery_code
   = f.input :password, required: true
   = f.button :submit, t('forms.reactivate_profile.submit'), class: 'mb1'

spec/controllers/two_factor_authentication/recovery_code_verification_controller_spec.rb

@@ -1,7 +1,9 @@
 require 'rails_helper'
 
 describe TwoFactorAuthentication::RecoveryCodeVerificationController do
-  let(:code) { { words: ['foo'] } }
+  let(:code) { { code: ['foo'] } }
+  let(:payload) { { recovery_code_form: code } }
+
   describe '#show' do
     context 'when there is no session (signed out or locked out), and the user reloads the page' do
       it 'redirects to the home page' do
@@ -25,15 +27,15 @@
       end
 
       it 'redirects to the profile' do
-        post :create, code: code
+        post :create, payload
 
         expect(response).to redirect_to profile_path
       end
 
       it 'calls handle_valid_otp' do
         expect(subject).to receive(:handle_valid_otp).and_call_original
 
-        post :create, code: code
+        post :create, payload
       end
 
       it 'tracks the valid authentication event' do
@@ -43,7 +45,7 @@
         expect(@analytics).to receive(:track_event).
           with(Analytics::MULTI_FACTOR_AUTH, analytics_hash)
 
-        post :create, code: code
+        post :create, payload
       end
     end
 
@@ -59,11 +61,11 @@
       it 'calls handle_invalid_otp' do
         expect(subject).to receive(:handle_invalid_otp).and_call_original
 
-        post :create, code: code
+        post :create, payload
       end
 
       it 're-renders the recovery code entry screen' do
-        post :create, code: code
+        post :create, payload
 
         expect(response).to render_template(:show)
         expect(flash[:error]).to eq t('devise.two_factor_authentication.invalid_recovery_code')
@@ -81,7 +83,7 @@
           with(Analytics::MULTI_FACTOR_AUTH, properties)
         expect(@analytics).to receive(:track_event).with(Analytics::MULTI_FACTOR_AUTH_MAX_ATTEMPTS)
 
-        3.times { post :create, code: code }
+        3.times { post :create, payload }
       end
     end
   end

spec/features/two_factor_authentication/sign_in_spec.rb

@@ -183,14 +183,10 @@ def submit_prefilled_otp_code
       sign_in_before_2fa(user)
 
       code = RecoveryCodeGenerator.new(user).create
-      code_words = code.split(' ')
-      click_link t('devise.two_factor_authentication.recovery_code_fallback.link')
 
-      fields = page.all('input[type="text"]')
+      click_link t('devise.two_factor_authentication.recovery_code_fallback.link')
 
-      fields.size.times do |index|
-        fields[index].set(code_words[index])
-      end
+      enter_recovery_code(code: code)
 
       click_submit_default
       click_acknowledge_recovery_code
@@ -241,17 +237,4 @@ def submit_prefilled_otp_code
       expect(current_path).to eq root_path
     end
   end
-
-  describe 'signing in and visiting endpoint that requires user to be signed out' do
-    it 'does not result in infinite redirect loop after submitting OTP code' do
-      allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
-      user = create(:user, :signed_up)
-      sign_in_user(user)
-
-      visit sign_up_email_path
-      click_submit_default
-
-      expect(current_path).to eq profile_path
-    end
-  end
 end

spec/features/visitors/password_recovery_spec.rb

@@ -35,7 +35,7 @@ def scrape_recovery_code
 
   def reactivate_profile(password, recovery_code)
     fill_in 'Password', with: password
-    fill_in 'Recovery code', with: recovery_code
+    enter_recovery_code(code: recovery_code)
     click_button t('forms.reactivate_profile.submit')
   end
 
@@ -292,7 +292,6 @@ def reactivate_profile(password, recovery_code)
 
     scenario 'resets password, uses recovery code as 2fa', email: true do
       recovery_code = recovery_code_from_pii(user, pii)
-      code_words = recovery_code.split(' ')
 
       trigger_reset_password_and_click_email_link(user.email)
 
@@ -301,11 +300,7 @@ def reactivate_profile(password, recovery_code)
 
       click_link t('devise.two_factor_authentication.recovery_code_fallback.link')
 
-      fields = page.all('input[type="text"]')
-
-      fields.size.times do |index|
-        fields[index].set(code_words[index])
-      end
+      enter_recovery_code(code: recovery_code)
 
       click_submit_default
 

spec/forms/reactivate_profile_form_spec.rb

@@ -3,7 +3,6 @@
 describe ReactivateProfileForm do
   subject(:form) do
     ReactivateProfileForm.new(user,
-                              recovery_code: recovery_code,
                               password: password)
   end
 
@@ -13,7 +12,7 @@
 
   describe '#valid?' do
     let(:password) { 'asd' }
-    let(:recovery_code) { '123' }
+    let(:recovery_code) { %w(123 abc) }
     let(:valid_recovery_code?) { true }
     let(:valid_password?) { true }
     let(:recovery_code_decrypts?) { true }
@@ -45,6 +44,12 @@
     end
 
     context 'when recovery code does not match' do
+      subject(:form) do
+        ReactivateProfileForm.new(user,
+                                  recovery_code: recovery_code,
+                                  password: password)
+      end
+
       let(:valid_recovery_code?) { false }
 
       it 'is invalid' do

spec/support/features/session_helper.rb

@@ -171,6 +171,15 @@ def click_acknowledge_recovery_code
       click_on t('forms.buttons.continue'), class: 'recovery-code-continue'
     end
 
+    def enter_recovery_code(code:, selector: 'input[type="text"]')
+      code_words = code.split(' ')
+      fields = page.all(selector)
+
+      fields.zip(code_words) do |field, word|
+        field.set(word)
+      end
+    end
+
     def loa3_sp_session
       Warden.on_next_request do |proxy|
         session = proxy.env['rack.session']

spec/views/two_factor_authentication/recovery_code_verification/show.html.slim_spec.rb

@@ -4,6 +4,7 @@
   let(:user) { build_stubbed(:user, :signed_up) }
 
   before do
+    @recovery_code_form = RecoveryCodeForm.new(user)
     allow(view).to receive(:current_user).and_return(user)
   end