Add FIDO2 passwordless login support with security keys #16
Conversation
petschekr
commented
Aug 24, 2019
petschekr
commented
- Adds support for logging in with FIDO2 security keys without a password 🎉
- Test using a compatible security that can perform User Verification in addition to User Presence (i.e. YubiKey 5 series vs YubiKey 4 series)
- Minor unrelated bug fixes
- Logging in with [email protected] for the CAS now provides a better error message with log out link
- Missing MIME types for static content have been added
| public readonly passportStrategy: Strategy; | ||
| private readonly timeout = 60000; // 60 seconds | ||
| private readonly fidoInstance = new Fido2Lib({ | ||
| cryptoParams: [-7], // ECC only |
There was a problem hiding this comment.
Can you document why this is -7
There was a problem hiding this comment.
Ok I find this weird too but it's a spec thing:
The alg is a number described in the COSE registry; for example, -7 indicates that the server accepts Elliptic Curve public keys using a SHA-256 signature algorithm
| done(null, false, { "message": "Registration timed out. Please try again." }); | ||
| return; | ||
| } | ||
| let email = session.email.trim().toLowerCase(); |
There was a problem hiding this comment.
can we add a standard email sanitizer for all strategies?
| origin: createLink(request, "").slice(0, -1), // Removes trailing slash from origin | ||
| factor: "first", | ||
| publicKey: user.services.fido2.publicKey || "", | ||
| prevCounter: user.services.fido2.prevCounter || Number.MAX_SAFE_INTEGER, |
There was a problem hiding this comment.
why Number.MAX_SAFE_INTEGER?
There was a problem hiding this comment.
The idea is that the counter prevents replay attacks because it must always be increasing. If we stored that the last auth was with counter set to 100, a replay of 100 or anything less than that will be rejected. If the prevCounter value is undefined or null for some reason, we set it to the maximum allowed number in JS so that the auth attempt will be rejected.
| done(null, false, { "message": "Invalid session values" }); | ||
| return; | ||
| } | ||
| if (Date.now() >= (session.fidoChallengeTime || 0) + this.timeout) { |
There was a problem hiding this comment.
move this logic to the top level of the passport callback since it's repeated?
|
Fun fact: Chrome on Android allegedly implements FIDO2 but cannot unlock the YubiKey due to a bug in Google Play Services (tracking issue I filed: https://bugs.chromium.org/p/chromium/issues/detail?id=997538) causing our site to reject the login attempt. As I see it, we have two options:
|
