New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reference UserWebauthnMethods into UserMultifactorMethods [3/4] #3808
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3808 +/- ##
=======================================
Coverage 98.81% 98.81%
=======================================
Files 214 214
Lines 5243 5244 +1
=======================================
+ Hits 5181 5182 +1
Misses 62 62
|
d0105c2
to
4cd3913
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhat related to my comment in #3806 but is there any benefits in moving this from User
into UserMultifactorMethods
? My thought is it may be better to keep it at the higher level, in the User, so these can be included individually if needed, instead of including all multifactor methods to get just the OTP ones, as an example.
That's a good point. However if you just include |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice clean up 🧹 💯 . LGTM 👍 .
4cd3913
to
4bfbf57
Compare
What problem are you solving?
This PR is based off of a larger PR #3805
As part of #3800, there are methods in
UserMultifactorMethods
that are specific to Webauthn.What approach did you choose and why?
This PR moves
verify_webauthn_otp
method to UserWebauthnMethods. I also moved the inclusion ofUserWebauthnMethods
intoUserMultifactorMethods
as methods inUserMultifactorMethods
relies on the methods inUserWebauthnMethods
.