Webauthn support

Feature
21

This is very nice!

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):

image
image.png2848×1542 461 KB

11 Likes
22

We can feature detect using navigator.credentials I assume.

9 Likes
23

Some people have a hard time justifying the extra expense even though it’s nice :smiley:

7 Likes
24

Argh I did that feature check on navigator.credentials.get but not create sorry about that, will be a quick fix!

8 Likes
25

The most recent commit seems to be working. I was able to use 2-factor (with fingerprint!) on my android phone just now.

6 Likes
26

Hey Penar, I’ve got a fix up for this now, are you able to check it out on Safari? https://github.com/discourse/discourse/pull/8146 . I’m just not sure if the method of feature detection I’m using will work with Safari having the feature turned off.

9 Likes
27

Sam beat me to the merge but this looks good on Safari, thanks Martin!

10 Likes
28

Right but in this case it is a normal part of work at Discourse so we would cover the cost. Sorry if that was not clear, but hopefully it is clear now?

9 Likes
29

Congrats on WebAuthn support! Interesting to see you rolled your own solution instead of using the webauthn gem, if there’s any feedback for us there I’d love to hear it :smiley:

I noticed in your implementation only supports algorithm -7 (ES256), but Windows Hello platform authenticators (backed by TPM 2.0 hardware) require -257 (RS256) as per Microsoft’s documentation. TPM 2.0 is required since July 28, 2016 for new Windows 10 desktop models, so that’s not an insignificant amount of hardware.

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.

10 Likes
30

@Martin_Brennan any thoughts on :arrow_double_up:

6 Likes
31

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:

  1. 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.
  2. 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).
  3. 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.

16 Likes
34

Support for the additional RS256 algorithm is now merged https://github.com/discourse/discourse/pull/8385

14 Likes
35

It seems, iOS 13.3 and macOS Catalina 10.15.2 are now supporting FIDO2-auth as well :partying_face:

This update also includes bug fixes and other improvements. This update:

  • Adds support for NFC, USB, and Lightning FIDO2-compliant security keys in Safari
8 Likes
36

It’s a shame iPhones need a third party device for it tough. I hope iOS++ has it built in with the device auth and secure chip. Like Windows, Mac OS and Android.

9 Likes
37

I am so excited to sign into Discourse by looking at it!!

Am actually kind of surprised that built-in authenticator support isn’t there just yet… but it’s exciting news regardless - I’ll continue to hold my breath.

7 Likes
38

I’ve observed a possible UI inconsistency when indicating activation of 2FA for some users on a hosted instance of Discourse :

Listing all ‘Staff’ users does not show my account as having 2FA enabled:

Annotation 2020-02-03 135308
Annotation 2020-02-03 1353081166×466 7.4 KB

  • Account summary page implies that 2FA is on, given the button text of ‘Manage Two Factor Authentication’.

  • Two Factor Authentication shows that a Security Key is enabled, and that Second Factor could be disabled.

  • Other users on the same instance who choose to use a Token-Based Authenticator (without a Security Key) show the padlock icon on the ‘all user’ listing.

Please let me know if this is a UI bug, or if simply adding a Security Key is not sufficient for 2FA for this platform.

5 Likes
39

I will take a look into this, but my best guess is that there is just a UI bug in the users list if the actual 2FA section of the UI is showing everything correctly. Thanks for the report!

9 Likes
40

That was the problem. Fix is building here and I will merge as soon as I am able https://github.com/discourse/discourse/pull/8839

Edit: This is now fixed.

11 Likes
41

Welcome to the brave new world

Apple has joined the FIDO Alliance (AKA Fast Identity Online), an organization already including giants such as Google, Intel, Microsoft and Samsung.

12 Likes
42

This works using TouchID / FaceID on iOS 14.

11 Likes