WebAuthn support

[jpaniagualaconich]

Implement a new second factor method that uses WebAuthn.

Description

A new authentication method is available in the setup wizard: WebAuthn, that enables support for hardware authentication devices such as fido-u2f keys in modern browsers that implement the specification.

Motivation and Context

WebAuthn is supported in the Chrome, Firefox, and Edge browsers, for credential creation and assertion using a U2F Token, like those provided by Yubico and Feitian.

How Has This Been Tested?

New view and utils tests have been added.

Screenshots (if appropriate):

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [x] I have added tests to cover my changes.
• [x] All new and existing tests passed.

[moggers87]

By the looks of https://github.com/w3c/webauthn/issues/1255 it seems the JS is not optional, so that's a huge :-1: from me. Not enough for me to reject this PR out of hand, but that code really does need tests and the entirety of webauthn needs to be optional in this package. That JS also needs to be taken out of the templates so people who aren't using the default templates don't have to go write their own JS + tests.

[claudep]

It would be much easier to host such optional modules if we could finish and merge #331

[moggers87]

It's on my todo list!

[jpaniagualaconich]

Hi there. Shall I wait for #331 in that case? It isn't the best thing in the world that I "if cased" webauthn sprinkling it here and there especially in the SetupView. Mimicking what's been done for the yubikey case to make it optional would only go further down that road.

And no, JS code is not optional. Do you have a preference for the testing framework I should use?

[moggers87]

I have no preference on what testing framework you use, whatever you're already familiar with is probably best.

[jpaniagualaconich]

Hi there!

• the new dependency should be optional, like the yubikey dependencies are

now it's optional

• a lot of the translation file updates appear to be unrelated to this PR and make it difficult to see if there's anything other than line number changes in there

took out all the translation-related stuff

• Is the client side JS really required? I'd prefer not to have any JS involved at all if possible. Also there are no tests for this non-trivial code.

changed it so JS is sent to the client only when WebAuthn is being used. Added an E2E test that covers the whole process (registration and use). webauthn tests will be skipped if the webauthn package is not installed.

The E2E test will be skipped if:

• one of these packages is missing: webauthn, selenium, webdriver-manager. First one is an extra (pip install django-two-factor-auth[webauthn]). For the other two there's a requirements_e2e.txt file.
• the StaticLiveServerTestCase service can't be reached at https://localhost/. I added an ssl-rev-proxy make target that sets up the root CA and a localhost certificate needed to serve https. It also spins an nginx container to reverse proxy from https://localhost. It needs docker and mkcert.

The "webauthn is an extra" part is in the documentation but the "how to run the E2E test" part is not. I'd like to know what you think of the whole thing before attempting to document it.

• The TWO_FACTOR_DEVICE_PREFERENCE stuff looks interesting, but really needs its own PR

it's no longer a part of this PR.

[jpaniagualaconich]

• Did away with inline JS
• No need for docker, mkcert, nginx. django-sslserver is used instead
• Documentation is updated
• Chrome can silently stop cooperating if you forget some settings. Made it harder for this to happen.
• modified tox.ini to include webauthn testing, with E2E bits too

[brugnara]

Hi @moggers87

Could you please help us and review this PR?

Thank you for considering my request.

[moggers87]

I'd like to wait for #425 to be merged before reviewing any more PRs

[jpaniagualaconich]

There's a new version of the py_webauthn package which brings support for lots of interesting attestation statement formats. Here is the complete list: https://github.com/duo-labs/py_webauthn/releases/tag/v1.0.0

It does come with a totally new API, though, and now it requires python 3.8+. @moggers87 since we're waiting for #425 I'm tempted to update. What do you recommend?

[Frikster]

For those waiting on this who don't want to use this development branch before it is merged to main, I have tested and found both the following repos django-fido and kagi to work quite nicely to offer Webauth 2FA.

[moggers87]

@jpaniagualaconich not sure I like bumping the minimum required version of Python to 3.8 just yet. 3.7 will still be receiving updates until June 2023: https://endoflife.date/python

[jpaniagualaconich]

@moggers87 ok, I patched typing so it has Literal also on python 3.7. This is what the new version of webauthn expects and it's the only reason why it wasn't working on 3.7.

I merged latest master and it's now ready for you to review.

[claudep]

@jpaniagualaconich I'm still working on making this package more plugin-friendly, so some more "breakages" may come soon in master branch. I'm sorry for this, and will try to help as much as possible for your branch integration.

[jpaniagualaconich]

@claudep ok no problem.

@moggers87 ok, it uses django-extensions now and everything lives in plugins/webauthn.

[codecov[bot]]

Codecov Report

Merging #408 (ebe0731) into master (8d1ecfe) will decrease coverage by 0.30%. The diff coverage is 95.30%.

@@            Coverage Diff             @@
##           master     #408      +/-   ##
==========================================
- Coverage   98.18%   97.88%   -0.31%     
==========================================
  Files          62       74      +12     
  Lines        2751     3162     +411     
  Branches      288      327      +39     
==========================================
+ Hits         2701     3095     +394     
- Misses         29       39      +10     
- Partials       21       28       +7     

Impacted Files	Coverage Δ
two_factor/plugins/webauthn/apps.py	75.00% <75.00%> (ø)
two_factor/plugins/webauthn/views.py	80.00% <80.00%> (ø)
two_factor/plugins/webauthn/utils.py	85.71% <85.71%> (ø)
two_factor/plugins/webauthn/models.py	91.66% <91.66%> (ø)
two_factor/plugins/webauthn/method.py	94.44% <94.44%> (ø)
tests/settings.py	100.00% <100.00%> (ø)
two_factor/plugins/webauthn/admin.py	100.00% <100.00%> (ø)
two_factor/plugins/webauthn/forms.py	100.00% <100.00%> (ø)
two_factor/plugins/webauthn/tests/test_e2e.py	100.00% <100.00%> (ø)
two_factor/plugins/webauthn/tests/test_forms.py	100.00% <100.00%> (ø)
... and 6 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jpaniagualaconich]

@moggers87 @claudep I merged latest master and went with the pluginisation as far as the current framework allows.

[moggers87]

@claudep I'll do a review, but I wouldn't mind a second pair of eyes to take a look.

[claudep]

Sure, that's my intention, but I'd like to finish integrating the email plugin first.

[hansegucker]

Is there any progress on this topic or something one could help with?

[jpaniagualaconich]

I merged latest master in, again. It's ready for someone to review.

[claudep]

@jpaniagualaconich Do you think you would be able to extract the backup_phones replacement by get_devices/get_other_devices in a separate PR? (I may ask you other such separate PR's in the future, hope you don't mind).

[claudep]

Would it also possible to move the migration file inside plugins/webauthn dir?

[jpaniagualaconich]

@claudep I extracted the device/other devices thingy into its own PR: https://github.com/jazzband/django-two-factor-auth/pull/526

[jpaniagualaconich]

@claudep migration file moved, also webauthn tests moved to their own folder (two_factor/plugins/webauthn/tests/) It's ready for another review round.

[claudep]

I guess we are approaching the merge moment, eventually! What do you think about rebasing and squashing the commits? Or we can do it during the merge, too.

[jpaniagualaconich]

@claudep great idea. done.

[claudep]

@moggers87, I think this is ready to go. can you give a last review or your approval?

[claudep]

Thanks a LOT @jpaniagualaconich for the constant rebasing and great patience to add this new functionality.

[jpaniagualaconich]

woohoo! thanks @claudep @moggers87 and all.