Reset webauthn devices in reset 2FA action

[jenshenny]

A support request came in to reset their 2FA. I noticed that the admin action doesn't reset Webauthn devices so if a user has a passkey set up, the action won't completely reset their MFA.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.24%. Comparing base (164a40e) to head (2c6ad2d).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6125   +/-   ##
=======================================
  Coverage   97.24%   97.24%           
=======================================
  Files         476      476           
  Lines        9785     9787    +2     
=======================================
+ Hits         9515     9517    +2     
  Misses        270      270           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jenshenny]

For some reason, when calling destroy_all, the webauthn credential still persists in the association so destroying the last credential would not result in disabling mfa.

I called reset_mfa_attributes in the admin action itself, even though it is repetitive, it ensures the action actually correctly removed all MFA methods.

[jenshenny]

Fix for CI: #6126

[colby-swandale]

Using destroy_all is going to trigger the disable_user_mfa & send_deletion_email callback for each webauthn device. Considering this is an admin action, should we switch this to be delete_all to skip the callbacks? (I suspect this will also resolve your comment below)

[colby-swandale]

Should we consider moving the idea of resetting MFA (OTP + WebAuthN) into a User#reset_all_mfa method to better capture everything and test for it?

[colby-swandale]

Should we also consider wrapping this all in a transaction?

[jenshenny]

The avo action is wrapped in a transaction so I think we're fine on that part.

I tried to move everything into reset_all_mfa , have a delete_all, and only send one email instead of one per device. However, the audits are subscribed to active_record.sql events and won't capture the delete_all webauthn cred records in the audit. It's going to take me a bit longer to implement getting these deleted records in the audit so I'm going to ship this as is to unblock the support request and come back to this after.