Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MFA #503

Merged
merged 16 commits into from
May 11, 2023
Merged

Add MFA #503

merged 16 commits into from
May 11, 2023

Conversation

cguess
Copy link
Collaborator

@cguess cguess commented Jan 25, 2023

This adds webauthn authentication as a required second factor
authentication.

To do so the following are here:

  • After setting the password when setting up an account the user is
    prompted to use MFA to validate their account.
  • To do the previous we also add authenticate_user_and_setup! to
    replace authenticate_user! specfically so that it validates MFA is
    set up as well everywhere.
  • At login the user is not logged in until after MFA is validated
  • Added Lottie for animations

Before this is merged into master there will be branches off of it to do
the following:

  • Add recovery codes
  • Add TOTP for Firefox users since Firefox only implements Webauthn for
    hardware keys, not the passkey standard. This is absurd, but oh well.
    https://github.com/mdp/rotp
  • Add the ability to register multiple keys

To Test:

  • Go through the entire account request, confirm, authorize system and
    then create a user, set a password and make sure you can register your
    MFA. Do this on vault, not insights, to facilitate later testing
  • Log into your account with MFA
  • Log out, swtich to insights, try to log in again. Should work just
    fine
  • Try it in a few different browsers with a hardware key, that should
    work. Passkey is going to be browser locked.

This adds webauthn authentication as a required second factor
authentication.

To do so the following are here:
- After setting the password when setting up an account the user is
  prompted to use MFA to validate their account.
- To do the previous we also add `authenticate_user_and_setup!` to
  replace `authenticate_user!` specfically so that it validates MFA is
set up as well everywhere.
- At login the user is not logged in until after MFA is validated

Before this is merged into master there will be branches off of it to do
the following:
1. Add recovery codes
2. Add TOTP for Firefox users since Firefox only implements Webauthn for
   hardware keys, not the passkey standard. This is absurd, but oh well.
3. Add the ability to register multiple keys

To Test:
- Go through the entire account request, confirm, authorize system and
  then create a user, set a password and make sure you can register your
MFA. Do this on vault, not insights, to facilitate later testing
- Log into your account with MFA
- Log out, swtich to insights, try to log in again. Should work just
  fine
- Try it in a few different browsers with a hardware key, that should
  work. Passkey is going to be browser locked.
@cguess cguess changed the title 8 add mfa Add MFA Jan 25, 2023
@cguess cguess self-assigned this Jan 25, 2023
Enable recovery codes for if the MFA token is long. This just creates
them and allows checking, but doesn't actually include the UI to check
yet or the actual resetting of the credentials yet.
@cguess
Copy link
Collaborator Author

cguess commented May 3, 2023

If we register a built-in key in one browser we lock them out of other browsers. Hardware keys are immune to this and work just fine across browsers. However, for instance, TouchID only works on Safari.

This is a problem because, for instance, logging in on an iPhone and then on chrome on the computer will send the user into a very frustrating system.

However, we can't just let them add a new key for every new device because... well, an attacker could just grab a different browser and create credentials.

The answer for this is probably a magic email link (super insecure...) or a temp password generated and then moved to a new device, but that's for another step. Before closing this create an issue to address this.

Edit: The answer is Passkeys https://www.passkeys.io which our feature should already work with, but testing is required

@cguess
Copy link
Collaborator Author

cguess commented May 3, 2023

Ok, our implementation supports cross browser passkeys already apparently, some unexpected good news!

@cguess cguess had a problem deploying to zenodotus-staging May 11, 2023 00:14 Failure
@cguess cguess temporarily deployed to zenodotus-staging May 11, 2023 03:42 Inactive
@cguess cguess marked this pull request as ready for review May 11, 2023 22:56
@cguess
Copy link
Collaborator Author

cguess commented May 11, 2023

Close #517 #516 #8

@cguess cguess merged commit d275dfd into master May 11, 2023
@cguess cguess deleted the 8-add-mfa branch May 11, 2023 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant