Refactor WebAuthn Verification logic #3720
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3720 +/- ##
==========================================
+ Coverage 98.75% 98.76% +0.01%
==========================================
Files 207 208 +1
Lines 5143 5119 -24
==========================================
- Hits 5079 5056 -23
+ Misses 64 63 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
bd25bae to
a4a0cac
Compare
There was a problem hiding this comm 10000 ent.
To add coverage for an indirect change https://app.codecov.io/gh/rubygems/rubygems.org/pull/3720/indirect-changes
…o WebAuthn Verifiable concern
Extract behaviour in webauthn_credential_verified? to helper methods
7b96e4c to
22fb663
Compare
22fb663 to
e6e2d95
Compare
There was a problem hiding this comment.
@simi @segiddins do you have any concerns with adding tests for a controller concern? I noticed that none of the other controller concerns have tests and that they are tested indirectly through the controller tests.
As mentioned above, in creating a controller concern, I wanted to isolate testing the logic + gives an example of the interface.
What problem are you solving?
There's a lot of duplication of the WebAuthn verification logic between the sessions, email confirmation, password and webauthn verification controllers.
What approach did you choose and why?
Created a controller concern
WebauthnVerifiablethat extracts the behaviour ofAll behaviour should remain the same, commits would show how the all logic is consolidated into the concern.
What should reviewers focus on?
Testing
Should be able to MFA using webauthn when signing in, reset password, change email and gem signin