Skip to content

Commit

Permalink
Merge pull request #3893 from Shopify/eric/webauthn_only_mfa_level
Browse files Browse the repository at this point in the history
Remove temporary webauthn only logic
  • Loading branch information
jenshenny committed Jun 30, 2023
2 parents 511fa06 + 931fb7f commit 5ec5af4
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 16 deletions.
7 changes: 1 addition & 6 deletions app/models/concerns/user_multifactor_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ module UserMultifactorMethods
end

def mfa_enabled?
if webauthn_enabled? && mfa_disabled?
self.mfa_level = :ui_and_gem_signin
save!(validate: false)
end

!mfa_disabled?
end

Expand All @@ -26,7 +21,7 @@ def no_mfa_devices?
end

def mfa_gem_signin_authorized?(otp)
return true unless strong_mfa_level? || webauthn_enabled?
return true unless strong_mfa_level?
api_mfa_verified?(otp)
end

Expand Down
25 changes: 15 additions & 10 deletions test/models/concerns/user_multifactor_methods_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,31 @@ class UserMultifactorMethodsTest < ActiveSupport::TestCase
end

context "#mfa_enabled" do
should "return true if multifactor auth is not disabled" do
should "return true if multifactor auth is not disabled using TOTP" do
@user.enable_totp!(ROTP::Base32.random_base32, :ui_only)

assert_predicate @user, :mfa_enabled?
end

should "return true if multifactor auth is disabled" do
should "return false if multifactor auth is disabled using TOTP" do
@user.disable_totp!

refute_predicate @user, :mfa_enabled?
end

should "return true if multifactor auth is not disabled using WebAuthn" do
create(:webauthn_credential, user: @user)

assert_predicate @user, :mfa_enabled?
end

should "return true if multifactor auth is disabled using WebAuthn" do
create(:webauthn_credential, user: @user)
@user.webauthn_credentials.first.destroy

refute_predicate @user, :mfa_enabled?
end

should "send mfa enabled email" do
assert_emails 1 do
@user.enable_totp!(ROTP::Base32.random_base32, :ui_and_gem_signin)
Expand All @@ -28,14 +41,6 @@ class UserMultifactorMethodsTest < ActiveSupport::TestCase
assert_equal "Multi-factor authentication enabled on RubyGems.org", last_email.subject
assert_equal [@user.email], last_email.to
end

should "update mfa level and return true with mfa disabled webauthn only users" do
@credential = create(:webauthn_credential, user: @user)
@user.update!(mfa_level: :disabled)

assert_predicate @user, :mfa_enabled?
assert_predicate @user, :mfa_ui_and_gem_signin?
end
end

context "#mfa_device_count_one?" do
Expand Down

0 comments on commit 5ec5af4

Please sign in to comment.