Thanks @Falco and @sam for your feedback. I didn’t realise there was a different route for mobile login either! I will start work on these fixes including the password labelling/button changes tonight, hopefully even open a new PR to fix!
One minor niggle though: on Safari/Mac, Web Authentication is a developer-only feature, off by default. When enabled, it works well. But we should likely show a message or a warning when Web Authentication is not enabled. Currently, on default Safari, nothing in the UI indicates that the registration process won’t work (the console has an error):
One suggestion for the “Login Flow” mockup - WebAuthn has an official logo that could be used instead of a generic fingerprint image. Besides fingerprint, facial recognition, swipe pattern or PIN are common user verification options as well.
Thank you for the feedback Rafe, we will need to add the additional algorithm here then. And thank you for the link to the official logo! My thought when I set only one algorithm was to just add the minimum number of supported algorithms to get things working for V1, as I wasn’t sure of nuances between all the algorithms, and ES256 was used in all the examples I could get my hands on.
As for why at the time I opted not to use the gem, I agonized over this quite a bit, and I knew I would be asked about this decision at some point. I certainly read through a lot of the code in the gem to get a better understanding of the implementation. The main reasons were:
Not wanting to add an additional dependency to Discourse. Every gem dependency adds additional overhead, no matter how excellent the code in that gem may be.
Wanting to have a good understanding of how such a critical security piece of Discourse operates. I thought that having the code within the core of Discourse would make things clearer and easier to extend, without having a “black box” so to speak. (Not an indictment of the gem itself or its complexity, I realize people could just go look at the code there, and I found it quite easy to follow what was going on).
Not wanting to add more than the minimum required code to get Webauthn running, for example I left out attestation from our initial implementation. I didn’t think there was much point adding a whole toolshed when all we needed was a spanner.
That being said, this could have been misguided on my part, and if we get into V2, V3 etc. of webauthn support with attestations, more supported algorithms, and what have you, it may become too laborious for us to become “Webauthn experts” so to speak, and at that point I think we could re-evaluate use of the gem.