Implement 2FA auth using webauthn #248
Conversation
jnugh
force-pushed
the
webauthn
branch
3 times, most recently
from
2a981fd
to
0325421
Compare
There was a problem hiding this comment.
This works (with my Nitrokey FIDO U2F. I'll test again with my FIDO2 key which should arrive today or tomorrow)! Awesome. Thanks @jnugh :-)
There was a problem hiding this comment.
Awesome work, thank you 🎉
I didn't have time to do a proper review of the authentication process yet, but since you requested many comments I already had a first look at the changes and tried to be super picky on the code style 😋
Apart from that, we have a very strict pylint config with which your code does not comply yet. You can run dev-tools/pylint.sh which will give you a full report on everything not passing our code conventions...
|
|
Add a settings view, allowing a user to register multiple keys and remove those. Add a new login step in case there is a key to verify
|
I noticed that the date columns in the key table were filled with dummy values only. Also fixed that :) |
There was a problem hiding this comment.
Thanks for your changes, apart from the two fuzzy translations the code looks very clean now! 🙂
Unfortunately, I couldn't get a FIDO2-emulation running on my machine, but as @svenseeberg already tested the functionality, I think once the translations are fixed, this PR is ready to merge! 🎉
Add a settings view, allowing a user to register multiple keys and remove those.
Changing MFA settings is only possible if the user (re-)verified his identity (using a password) within the last 5 minutes.
Question is: Should 2FA be checked before changes are allowed to be made?
I don't see a thread model where this would help. The user already proved his identity by logging in in the first place and the password check is only there if someone gains access to the already logged in session. Asking for the password again makes sure that the attacker could not gain persistent access. If the password was leaked he would need the key to log in in the first place.
Asking for 2FA to edit 2FA settings would only help against an attacker that has the valid password and got access to a valid session somehow.
Add a new login step in case there is a key to verify.
Fixes #131
This is my first PR, so there is most likely a lot of things that don't align with your standards and as this is a quite large PR as well I expect a lot of comments on this :P
I did not fix lining errors yet, as pylint_runner shows me a ton on unrelated files as well