Codecov Report

Merging #3905 (c255e8b) into master (642b650) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head c255e8b differs from pull request most recent head 3486573. Consider uploading reports for the commit 3486573 to get more accurate results

@@           Coverage Diff           @@
##           master    #3905   +/-   ##
=======================================
  Coverage   98.79%   98.79%           
=======================================
  Files         217      217           
  Lines        5406     5414    +8     
=======================================
+ Hits         5341     5349    +8     
  Misses         65       65           

@ericherscovich I did some initial checks.

Specifically, we should check if there exists any user that has mfa_level somehow set while not having any MFA devices.

User.where.not(mfa_level: 'disabled').each_with_object(0) {|u,m| m=m+1 if u.webauthn_credentials.none?}
#=> 0 # yes, I was lazy to write JOIN logic in here

We should also check that there isn't any user that has mfa_level set to disabled, but has an MFA device present.

User.where(mfa_level: 'disabled').joins(:webauthn_credentials).count
#=> 0

@ericherscovich I did some initial checks.

Specifically, we should check if there exists any user that has mfa_level somehow set while not having any MFA devices.

User.where.not(mfa_level: 'disabled').each_with_object(0) {|u,m| m=m+1 if u.webauthn_credentials.none?}
#=> 0 # yes, I was lazy to write JOIN logic in here

We should also check that there isn't any user that has mfa_level set to disabled, but has an MFA device present.

User.where(mfa_level: 'disabled').joins(:webauthn_credentials).count
#=> 0

Thank you! Could you do a more generic check that would check for TOTP as well? You should be able to check something along the lines of

1. User.where.not(mfa_level: 'disabled') and check u.no_mfa_devices?
2. User.where(mfa_level: 'disabled') and u.!no_mfa_devices?

@ericherscovich like this?

User.where.not(mfa_level: 'disabled') and check u.no_mfa_devices?

User.where.not(mfa_level: 'disabled').each_with_object(0) {|u,m| m=m+1 if u.no_mfa_devices?}
=> 0

User.where(mfa_level: 'disabled') and u.!no_mfa_devices?

User.where(mfa_level: 'disabled').each_with_object(0) {|u,m| m=m+1 if !u.no_mfa_devices?}
=> 0

@ericherscovich like this?

User.where.not(mfa_level: 'disabled') and check u.no_mfa_devices?

User.where.not(mfa_level: 'disabled').each_with_object(0) {|u,m| m=m+1 if u.no_mfa_devices?}
=> 0

User.where(mfa_level: 'disabled') and u.!no_mfa_devices?

User.where(mfa_level: 'disabled').each_with_object(0) {|u,m| m=m+1 if !u.no_mfa_devices?}
=> 0

Yes that looks like what I was looking for! Thank you for doing this check :D