Move TOTP related methods to UserTotpMethods concern

app/models/concerns/user_multifactor_methods.rb

@@ -2,38 +2,14 @@ module UserMultifactorMethods
   extend ActiveSupport::Concern
 
   included do
+    include UserTotpMethods
+
     enum mfa_level: { disabled: 0, ui_only: 1, ui_and_api: 2, ui_and_gem_signin: 3 }, _prefix: :mfa
 
     def mfa_enabled?
       !mfa_disabled?
     end
 
-    def disable_mfa!
-      mfa_disabled!
-      self.mfa_seed = ""
-      self.mfa_recovery_codes = []
-      save!(validate: false)
-      Mailer.mfa_disabled(id, Time.now.utc).deliver_later
-    end
-
-    def verify_and_enable_mfa!(seed, level, otp, expiry)
-      if expiry < Time.now.utc
-        errors.add(:base, I18n.t("multifactor_auths.create.qrcode_expired"))
-      elsif verify_digit_otp(seed, otp)
-        enable_mfa!(seed, level)
-      else
-        errors.add(:base, I18n.t("multifactor_auths.incorrect_otp"))
-      end
-    end
-
-    def enable_mfa!(seed, level)
-      self.mfa_level = level
-      self.mfa_seed = seed
-      self.mfa_recovery_codes = Array.new(10).map { SecureRandom.hex(6) }
-      save!(validate: false)
-      Mailer.mfa_enabled(id, Time.now.utc).deliver_later
-    end
-
     def mfa_gem_signin_authorized?(otp)
       return true unless strong_mfa_level? || webauthn_credentials.present?
       api_otp_verified?(otp)
@@ -87,15 +63,6 @@ def mfa_required?
       rubygems.mfa_required.any?
     end
 
-    def verify_digit_otp(seed, otp)
-      return false if seed.blank?
-
-      totp = ROTP::TOTP.new(seed)
-      return false unless totp.verify(otp, drift_behind: 30, drift_ahead: 30)
-
-      save!(validate: false)
-    end
-
     def verify_webauthn_otp(otp)
       webauthn_verification&.verify_otp(otp)
     end

app/models/concerns/user_totp_methods.rb

@@ -0,0 +1,40 @@
+module UserTotpMethods
+  extend ActiveSupport::Concern
+
+  def disable_totp!
+    mfa_disabled!
+    self.mfa_seed = ""
+    self.mfa_recovery_codes = []
+    save!(validate: false)
+    Mailer.mfa_disabled(id, Time.now.utc).deliver_later
+  end
+
+  def verify_and_enable_totp!(seed, level, otp, expiry)
+    if expiry < Time.now.utc
+      errors.add(:base, I18n.t("multifactor_auths.create.qrcode_expired"))
+    elsif verify_digit_otp(seed, otp)
+      enable_totp!(seed, level)
+    else
+      errors.add(:base, I18n.t("multifactor_auths.incorrect_otp"))
+    end
+  end
+
+  def enable_totp!(seed, level)
+    self.mfa_level = level
+    self.mfa_seed = seed
+    self.mfa_recovery_codes = Array.new(10).map { SecureRandom.hex(6) }
+    save!(validate: false)
+    Mailer.mfa_enabled(id, Time.now.utc).deliver_later
+  end
+
+  private
+
+  def verify_digit_otp(seed, otp)
+    return false if seed.blank?
+
+    totp = ROTP::TOTP.new(seed)
+    return false unless totp.verify(otp, drift_behind: 30, drift_ahead: 30)
+
+    save!(validate: false)
+  end
+end

test/models/concerns/user_multifactor_methods_test.rb

@@ -30,87 +30,6 @@ class UserMultifactorMethodsTest < ActiveSupport::TestCase
     end
   end
 
-  context "#disable_mfa!" do
-    setup do
-      @user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
-
-      perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
-        @user.disable_mfa!
-      end
-    end
-
-    should "disable mfa" do
-      assert_predicate @user, :mfa_disabled?
-      assert_empty @user.mfa_seed
-      assert_empty @user.mfa_recovery_codes
-    end
-
-    should "send mfa disabled email" do
-      assert_emails 1
-
-      assert_equal "Multi-factor authentication disabled on RubyGems.org", last_email.subject
-      assert_equal [@user.email], last_email.to
-    end
-  end
-
-  context "#verify_and_enable_mfa!" do
-    setup do
-      @seed = ROTP::Base32.random_base32
-      @expiry = 30.minutes.from_now
-    end
-
-    should "enable mfa" do
-      @user.verify_and_enable_mfa!(
-        @seed,
-        :ui_and_api,
-        ROTP::TOTP.new(@seed).now,
-        @expiry
-      )
-
-      assert_predicate @user, :mfa_enabled?
-    end
-
-    should "add error if qr code expired" do
-      @user.verify_and_enable_mfa!(
-        @seed,
-        :ui_and_api,
-        ROTP::TOTP.new(@seed).now,
-        5.minutes.ago
-      )
-
-      refute_predicate @user, :mfa_enabled?
-      expected_error = "The QR-code and key is expired. Please try registering a new device again."
-
-      assert_contains @user.errors[:base], expected_error
-    end
-
-    should "add error if otp code is incorrect" do
-      @user.verify_and_enable_mfa!(
-        @seed,
-        :ui_and_api,
-        ROTP::TOTP.new(ROTP::Base32.random_base32).now,
-        @expiry
-      )
-
-      refute_predicate @user, :mfa_enabled?
-      assert_contains @user.errors[:base], "Your OTP code is incorrect."
-    end
-  end
-
-  context "#enable_mfa!" do
-    setup do
-      @seed = ROTP::Base32.random_base32
-      @level = :ui_and_api
-      @user.enable_mfa!(@seed, @level)
-    end
-
-    should "enable mfa" do
-      assert_equal @seed, @user.mfa_seed
-      assert_predicate @user, :mfa_ui_and_api?
-      assert_equal 10, @user.mfa_recovery_codes.length
-    end
-  end
-
   context "#mfa_gem_signin_authorized?" do
     setup do
       @seed = ROTP::Base32.random_base32

test/models/concerns/user_totp_methods_test.rb

@@ -0,0 +1,90 @@
+require "test_helper"
+
+class UserTotpMethodsTest < ActiveSupport::TestCase
+  include ActionMailer::TestHelper
+
+  setup do
+    @user = create(:user)
+  end
+
+  context "#disable_mfa!" do
+    setup do
+      @user.enable_mfa!(ROTP::Base32.random_base32, :ui_only)
+
+      perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
+        @user.disable_mfa!
+      end
+    end
+
+    should "disable mfa" do
+      assert_predicate @user, :mfa_disabled?
+      assert_empty @user.mfa_seed
+      assert_empty @user.mfa_recovery_codes
+    end
+
+    should "send mfa disabled email" do
+      assert_emails 1
+
+      assert_equal "Multi-factor authentication disabled on RubyGems.org", last_email.subject
+      assert_equal [@user.email], last_email.to
+    end
+  end
+
+  context "#verify_and_enable_mfa!" do
+    setup do
+      @seed = ROTP::Base32.random_base32
+      @expiry = 30.minutes.from_now
+    end
+
+    should "enable mfa" do
+      @user.verify_and_enable_mfa!(
+        @seed,
+        :ui_and_api,
+        ROTP::TOTP.new(@seed).now,
+        @expiry
+      )
+
+      assert_predicate @user, :mfa_enabled?
+    end
+
+    should "add error if qr code expired" do
+      @user.verify_and_enable_mfa!(
+        @seed,
+        :ui_and_api,
+        ROTP::TOTP.new(@seed).now,
+        5.minutes.ago
+      )
+
+      refute_predicate @user, :mfa_enabled?
+      expected_error = "The QR-code and key is expired. Please try registering a new device again."
+
+      assert_contains @user.errors[:base], expected_error
+    end
+
+    should "add error if otp code is incorrect" do
+      @user.verify_and_enable_mfa!(
+        @seed,
+        :ui_and_api,
+        ROTP::TOTP.new(ROTP::Base32.random_base32).now,
+        @expiry
+      )
+
+      refute_predicate @user, :mfa_enabled?
+      assert_contains @user.errors[:base], "Your OTP code is incorrect."
+    end
+  end
+
+  context "#enable_mfa!" do
+    setup do
+      @seed = ROTP::Base32.random_base32
+      @level = :ui_and_api
+      @user.enable_mfa!(@seed, @level)
+    end
+
+    should "enable mfa" do
+      assert_equal @seed, @user.mfa_seed
+      assert_predicate @user, :mfa_ui_and_api?
+      assert_equal 10, @user.mfa_recovery_codes.length
+    end
+  end
+end