Add tests for recovery codes

app/controllers/user_controller.rb

@@ -175,10 +175,9 @@ class UserController < ApplicationController
 
   def edit_security
     if current_user.otp_module_disabled?
-      current_user.otp_secret_key = User.otp_random_secret(26)
+      current_user.otp_secret_key = User.otp_random_secret(25)
       current_user.save
 
-      @provisioning_uri = current_user.provisioning_uri(nil, issuer: APP_CONFIG[:hostname])
       qr_code = RQRCode::QRCode.new(current_user.provisioning_uri("Retrospring:#{current_user.screen_name}", issuer: "Retrospring"))
 
       @qr_svg = qr_code.as_svg({offset: 4, module_size: 4, color: '000;fill:var(--primary)'}).html_safe
@@ -192,7 +191,7 @@ class UserController < ApplicationController
     current_user.otp_module = :enabled
 
     if current_user.authenticate_otp(req_params[:otp_validation], drift: APP_CONFIG.fetch(:otp_drift_period, 30).to_i)
-      @recovery_keys = TotpRecoveryCode.create!(Array.new(10) { {user: current_user, code: SecureRandom.base58(8).downcase} })
+      @recovery_keys = TotpRecoveryCode.generate_for(current_user)
       current_user.save!
 
       render 'settings/security/recovery_keys'
@@ -205,14 +204,14 @@ class UserController < ApplicationController
   def destroy_2fa
     current_user.otp_module = :disabled
     current_user.save!
-    TotpRecoveryCode.where(user_id: current_user.id).delete_all
+    TotpRecoveryCode.remove_all_for(current_user)
     flash[:success] = 'Two factor authentication has been disabled for your account.'
     redirect_to edit_user_security_path
   end
 
   def reset_user_recovery_codes
-    TotpRecoveryCode.where(user_id: current_user.id).delete_all
-    @recovery_keys = TotpRecoveryCode.create!(Array.new(10) { {user: current_user, code: SecureRandom.base58(8).downcase} })
+    TotpRecoveryCode.remove_all_for(current_user)
+    @recovery_keys = TotpRecoveryCode.generate_for(current_user)
     render 'settings/security/recovery_keys'
   end
 end

app/models/totp_recovery_code.rb

@@ -1,3 +1,17 @@
 class TotpRecoveryCode < ApplicationRecord
+  NUMBER_OF_CODES_TO_GENERATE = 16
+
   belongs_to :user
+
+  # @param user [User]
+  # @return [Array<TotpRecoveryCode>]
+  def self.generate_for(user)
+    TotpRecoveryCode.create!(Array.new(16) { {user: user, code: SecureRandom.base58(8).downcase} })
+  end
+
+  # @param user [User]
+  # @return [Integer]
+  def self.remove_all_for(user)
+    TotpRecoveryCode.where(user: user).delete_all
+  end
 end

app/models/user.rb

@@ -43,6 +43,7 @@ class User < ApplicationRecord
   has_many :list_memberships, class_name: "ListMember", foreign_key: 'user_id', dependent: :destroy
 
   has_many :subscriptions, dependent: :destroy
+  has_many :totp_recovery_codes, dependent: :destroy
 
   has_one :theme, dependent: :destroy
 

spec/controllers/user_controller_spec.rb

@@ -3,7 +3,9 @@
 require "rails_helper"
 
 describe UserController, type: :controller do
-  let(:user) { FactoryBot.create :user, otp_module: :disabled }
+  let(:user) { FactoryBot.create :user,
+                                 otp_module: :disabled,
+                                 otp_secret_key: 'EJFNIJPYXXTCQSRTQY6AG7XQLAT2IDG5H7NGLJE3'}
 
   describe "#edit" do
     subject { get :edit }
@@ -97,8 +99,7 @@ describe UserController, type: :controller do
       context "user enters the incorrect code" do
         let(:update_params) do
           {
-              user: { otp_secret_key: 'EJFNIJPYXXTCQSRTQY6AG7XQLAT2IDG5H7NGLJE3',
-                      otp_validation: 123456 }
+              user: { otp_validation: 123456 }
           }
         end
 
@@ -106,6 +107,7 @@ describe UserController, type: :controller do
           Timecop.freeze(Time.at(1603290888)) do
             subject
             expect(response).to redirect_to :edit_user_security
+            expect(flash[:error]).to eq('The code you entered was invalid.')
           end
         end
       end
@@ -113,21 +115,23 @@ describe UserController, type: :controller do
       context "user enters the correct code" do
         let(:update_params) do
           {
-              user: { otp_secret_key: 'EJFNIJPYXXTCQSRTQY6AG7XQLAT2IDG5H7NGLJE3',
-                      otp_validation: 187894 }
+              user: { otp_validation: 187894 }
           }
         end
 
-        it "enables 2FA for the logged in user" do
+        it "enables 2FA for the logged in user and generates recovery keys" do
           Timecop.freeze(Time.at(1603290888)) do
             subject
-            expect(response).to redirect_to :edit_user_security
+            expect(response).to have_rendered(:recovery_keys)
+
+            expect(user.totp_recovery_codes.count).to be(TotpRecoveryCode::NUMBER_OF_CODES_TO_GENERATE)
           end
         end
 
         it "shows an error if the user attempts to use the code once it has expired" do
           Timecop.freeze(Time.at(1603290910)) do
             subject
+            expect(response).to redirect_to :edit_user_security
             expect(flash[:error]).to eq('The code you entered was invalid.')
           end
         end
@@ -142,13 +146,31 @@ describe UserController, type: :controller do
       before(:each) do
         user.otp_module = :enabled
         user.save
-        sign_in user
+        sign_in(user)
       end
 
       it "disables 2FA for the logged in user" do
         subject
         user.reload
         expect(user.otp_module_enabled?).to be_falsey
+        expect(user.totp_recovery_codes.count).to be(0)
+      end
+    end
+  end
+
+  describe "#reset_user_recovery_codes" do
+    subject { delete :reset_user_recovery_codes }
+
+    context "user signed in" do
+      before(:each) do
+        sign_in(user)
+      end
+
+      it "regenerates codes on request" do
+        old_codes = user.totp_recovery_codes.pluck(:code)
+        subject
+        new_codes = user.totp_recovery_codes.pluck(:code)
+        expect(new_codes).not_to match_array(old_codes)
       end
     end
   end