Merge pull request #3810 from Shopify/rename-mfa-concern-methods

Rename UserMultifactorMethods concern methods to reference MFA [4/4]

app/controllers/email_confirmations_controller.rb

@@ -97,7 +97,7 @@ def token_params
   end
 
   def mfa_update_conditions_met?
-    @user.mfa_enabled? && @user.ui_otp_verified?(params[:otp]) && session_active?
+    @user.mfa_enabled? && @user.ui_mfa_verified?(params[:otp]) && session_active?
   end
 
   def setup_mfa_authentication

app/controllers/multifactor_auths_controller.rb

@@ -27,7 +27,7 @@ def create
   end
 
   def update
-    if current_user.ui_otp_verified?(otp_param)
+    if current_user.ui_mfa_verified?(otp_param)
       handle_new_level_param
       redirect_to session.fetch("mfa_redirect_uri", edit_settings_path)
       session.delete("mfa_redirect_uri")

app/controllers/passwords_controller.rb

@@ -79,7 +79,7 @@ def setup_mfa_authentication
   end
 
   def mfa_edit_conditions_met?
-    @user.mfa_enabled? && @user.ui_otp_verified?(params[:otp]) && session_active?
+    @user.mfa_enabled? && @user.ui_mfa_verified?(params[:otp]) && session_active?
   end
 
   def login_failure(message)

app/controllers/sessions_controller.rb

@@ -143,7 +143,7 @@ def setup_mfa_authentication
   end
 
   def login_conditions_met?
-    @user&.mfa_enabled? && @user&.ui_otp_verified?(params[:otp]) && session_active?
+    @user&.mfa_enabled? && @user&.ui_mfa_verified?(params[:otp]) && session_active?
   end
 
   def record_mfa_login_duration(mfa_type:)

app/models/api_key.rb

@@ -28,7 +28,7 @@ def enabled_scopes
 
   def mfa_authorized?(otp)
     return true unless mfa_enabled?
-    user.api_otp_verified?(otp)
+    user.api_mfa_verified?(otp)
   end
 
   def mfa_enabled?

app/models/concerns/user_multifactor_methods.rb

@@ -6,63 +6,63 @@ module UserMultifactorMethods
     include UserWebauthnMethods
 
     enum mfa_level: { disabled: 0, ui_only: 1, ui_and_api: 2, ui_and_gem_signin: 3 }, _prefix: :mfa
+  end
 
-    def mfa_enabled?
-      !mfa_disabled?
-    end
+  def mfa_enabled?
+    !mfa_disabled?
+  end
 
-    def mfa_gem_signin_authorized?(otp)
-      return true unless strong_mfa_level? || webauthn_credentials.present?
-      api_otp_verified?(otp)
-    end
+  def mfa_gem_signin_authorized?(otp)
+    return true unless strong_mfa_level? || webauthn_credentials.present?
+    api_mfa_verified?(otp)
+  end
 
-    def mfa_recommended_not_yet_enabled?
-      mfa_recommended? && mfa_disabled?
-    end
+  def mfa_recommended_not_yet_enabled?
+    mfa_recommended? && mfa_disabled?
+  end
 
-    def mfa_recommended_weak_level_enabled?
-      mfa_recommended? && mfa_ui_only?
-    end
+  def mfa_recommended_weak_level_enabled?
+    mfa_recommended? && mfa_ui_only?
+  end
 
-    def mfa_required_not_yet_enabled?
-      mfa_required? && mfa_disabled?
-    end
+  def mfa_required_not_yet_enabled?
+    mfa_required? && mfa_disabled?
+  end
 
-    def mfa_required_weak_level_enabled?
-      mfa_required? && mfa_ui_only?
-    end
+  def mfa_required_weak_level_enabled?
+    mfa_required? && mfa_ui_only?
+  end
 
-    def ui_otp_verified?(otp)
-      otp = otp.to_s
-      return true if verify_totp(mfa_seed, otp)
-      return false unless mfa_recovery_codes.include? otp
-      mfa_recovery_codes.delete(otp)
-      save!(validate: false)
-    end
+  def ui_mfa_verified?(otp)
+    otp = otp.to_s
+    return true if verify_totp(mfa_seed, otp)
+    return false unless mfa_recovery_codes.include? otp
+    mfa_recovery_codes.delete(otp)
+    save!(validate: false)
+  end
 
-    def api_otp_verified?(otp)
-      return true if verify_webauthn_otp(otp)
-      return true if ui_otp_verified?(otp)
-      false
-    end
+  def api_mfa_verified?(otp)
+    return true if verify_webauthn_otp(otp)
+    return true if ui_mfa_verified?(otp)
+    false
+  end
 
-    private
+  private
 
-    def strong_mfa_level?
-      mfa_ui_and_gem_signin? || mfa_ui_and_api?
-    end
+  def strong_mfa_level?
+    mfa_ui_and_gem_signin? || mfa_ui_and_api?
+  end
 
-    def mfa_recommended?
-      return false if strong_mfa_level? || mfa_required?
+  def mfa_recommended?
+    return false if strong_mfa_level? || mfa_required?
 
-      rubygems.mfa_recommended.any?
-    end
+    rubygems.mfa_recommended.any?
+  end
 
-    def mfa_required?
-      return false if strong_mfa_level?
+  def mfa_required?
+    return false if strong_mfa_level?
 
-      rubygems.mfa_required.any?
-    end
+    rubygems.mfa_required.any?
   end
 
   class_methods do

test/models/concerns/user_multifactor_methods_test.rb

@@ -214,100 +214,100 @@ class UserMultifactorMethodsTest < ActiveSupport::TestCase
     end
   end
 
-  context "#ui_otp_verified?" do
+  context "#ui_mfa_verified?" do
     setup do
       @user.enable_totp!(ROTP::Base32.random_base32, :ui_and_api)
     end
 
     context "with totp" do
       should "return true when correct" do
-        assert @user.ui_otp_verified?(ROTP::TOTP.new(@user.mfa_seed).now)
+        assert @user.ui_mfa_verified?(ROTP::TOTP.new(@user.mfa_seed).now)
       end
 
       should "return true when correct in last interval" do
         last_otp = ROTP::TOTP.new(@user.mfa_seed).at(Time.current - 30)
 
-        assert @user.ui_otp_verified?(last_otp)
+        assert @user.ui_mfa_verified?(last_otp)
       end
 
       should "return true when correct in next interval" do
         next_otp = ROTP::TOTP.new(@user.mfa_seed).at(Time.current + 30)
 
-        assert @user.ui_otp_verified?(next_otp)
+        assert @user.ui_mfa_verified?(next_otp)
       end
 
       should "return false when incorrect" do
-        refute @user.ui_otp_verified?(ROTP::TOTP.new(ROTP::Base32.random_base32).now)
+        refute @user.ui_mfa_verified?(ROTP::TOTP.new(ROTP::Base32.random_base32).now)
       end
 
       should "return false if the mfa_seed is blank" do
         @user.update!(mfa_seed: nil)
 
-        refute @user.ui_otp_verified?(ROTP::TOTP.new(ROTP::Base32.random_base32).now)
+        refute @user.ui_mfa_verified?(ROTP::TOTP.new(ROTP::Base32.random_base32).now)
       end
     end
 
     context "with webauthn otp" do
       should "return false" do
         webauthn_verification = create(:webauthn_verification, user: @user)
 
-        refute @user.ui_otp_verified?(webauthn_verification.otp)
+        refute @user.ui_mfa_verified?(webauthn_verification.otp)
       end
     end
 
     should "return true if recovery code is correct" do
       recovery_code = @user.mfa_recovery_codes.first
 
-      assert @user.ui_otp_verified?(recovery_code)
+      assert @user.ui_mfa_verified?(recovery_code)
       refute_includes @user.mfa_recovery_codes, recovery_code
     end
   end
 
-  context "#api_otp_verified?" do
+  context "#api_mfa_verified?" do
     setup do
       @user.enable_totp!(ROTP::Base32.random_base32, :ui_and_api)
     end
 
     context "with totp" do
       should "return true when correct" do
-        assert @user.api_otp_verified?(ROTP::TOTP.new(@user.mfa_seed).now)
+        assert @user.api_mfa_verified?(ROTP::TOTP.new(@user.mfa_seed).now)
       end
 
       should "return true when correct in last interval" do
         last_otp = ROTP::TOTP.new(@user.mfa_seed).at(Time.current - 30)
 
-        assert @user.api_otp_verified?(last_otp)
+        assert @user.api_mfa_verified?(last_otp)
       end
 
       should "return true when correct in next interval" do
         next_otp = ROTP::TOTP.new(@user.mfa_seed).at(Time.current + 30)
 
-        assert @user.api_otp_verified?(next_otp)
+        assert @user.api_mfa_verified?(next_otp)
       end
 
       should "return false if otp is incorrect" do
-        refute @user.api_otp_verified?(ROTP::TOTP.new(ROTP::Base32.random_base32).now)
+        refute @user.api_mfa_verified?(ROTP::TOTP.new(ROTP::Base32.random_base32).now)
       end
     end
 
     context "with webauthn otp" do
       should "return true when correct" do
         webauthn_verification = create(:webauthn_verification, user: @user)
 
-        assert @user.api_otp_verified?(webauthn_verification.otp)
+        assert @user.api_mfa_verified?(webauthn_verification.otp)
       end
 
       should "return false when incorrect" do
         create(:webauthn_verification, user: @user, otp: "jiEm2mm2sJtRqAVx")
         incorrect_otp = "Yxf57d1wEUSWyXrr"
 
-        refute @user.api_otp_verified?(incorrect_otp)
+        refute @user.api_mfa_verified?(incorrect_otp)
       end
 
       should "return false when expired" do
         webauthn_verification = create(:webauthn_verification, user: @user, otp_expires_at: 2.minutes.ago)
 
-        refute @user.api_otp_verified?(webauthn_verification.otp)
+        refute @user.api_mfa_verified?(webauthn_verification.otp)
       end
 
       context "when webauthn otp has not been generated" do
@@ -316,19 +316,19 @@ class UserMultifactorMethodsTest < ActiveSupport::TestCase
         end
 
         should "return false for an otp" do
-          refute @user.api_otp_verified?("Yxf57d1wEUSWyXrr")
+          refute @user.api_mfa_verified?("Yxf57d1wEUSWyXrr")
         end
 
         should "return false if otp is nil" do
-          refute @user.api_otp_verified?(nil)
+          refute @user.api_mfa_verified?(nil)
         end
       end
     end
 
     should "return true if recovery code is correct" do
       recovery_code = @user.mfa_recovery_codes.first
 
-      assert @user.api_otp_verified?(recovery_code)
+      assert @user.api_mfa_verified?(recovery_code)
       refute_includes @user.mfa_recovery_codes, recovery_code
     end
   end

test/models/user_test.rb

@@ -360,12 +360,12 @@ class UserTest < ActiveSupport::TestCase
         should "be able to use a recovery code only once" do
           code = @user.mfa_recovery_codes.first
 
-          assert @user.ui_otp_verified?(code)
-          refute @user.ui_otp_verified?(code)
+          assert @user.ui_mfa_verified?(code)
+          refute @user.ui_mfa_verified?(code)
         end
 
         should "be able to verify correct OTP" do
-          assert @user.ui_otp_verified?(ROTP::TOTP.new(@user.mfa_seed).now)
+          assert @user.ui_mfa_verified?(ROTP::TOTP.new(@user.mfa_seed).now)
         end
 
         should "return true for mfa status check" do
@@ -376,13 +376,13 @@ class UserTest < ActiveSupport::TestCase
         should "return true for otp in last interval" do
           last_otp = ROTP::TOTP.new(@user.mfa_seed).at(Time.current - 30)
 
-          assert @user.ui_otp_verified?(last_otp)
+          assert @user.ui_mfa_verified?(last_otp)
         end
 
         should "return true for otp in next interval" do
           next_otp = ROTP::TOTP.new(@user.mfa_seed).at(Time.current + 30)
 
-          assert @user.ui_otp_verified?(next_otp)
+          assert @user.ui_mfa_verified?(next_otp)
         end
 
         context "blocking user with api key" do
@@ -414,7 +414,7 @@ class UserTest < ActiveSupport::TestCase
         end
 
         should "return false for verifying OTP" do
-          refute @user.ui_otp_verified?("")
+          refute @user.ui_mfa_verified?("")
         end
 
         should "return false for mfa status check" do