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

Implement multi-factor authentication for user accounts #314

Open
sea-kelp opened this issue May 10, 2023 · 2 comments
Open

Implement multi-factor authentication for user accounts #314

sea-kelp opened this issue May 10, 2023 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@sea-kelp
Copy link
Collaborator

sea-kelp commented May 10, 2023

Background

Multi-factor authentication is a widely used mechanism for defending against several types of user account attacks. Implementing MFA would go a long way toward strengthening user account security in OpenOversight.

There are currently two main algorithms for generating the MFA one-time passwords (OTP) used to log in:

  • Time-based One Time Password (TOTP): a time-based algorithm often used in a phone app like Google Authenticator. This is the more popular of the two, especially among non-technical users.
  • HMAC-based One Time Password (HOTP): an algorithm that is more often implemented in hardware tokens like Yubikeys.

We should implement TOTP support but should also either support HOTP upfront or be extensible enough to support HOTP in the future.

User Stories

As an OpenOversight administrator:

  • I want OpenOversight to require MFA on my account.
  • I want to be able to reset MFA for all users on my instance.
  • I want another admin to be able to reset MFA on my account.
  • I want OpenOversight to require MFA when I log in or update my account settings, such as my email address and password.

As an OpenOversight area coordinator:

  • I want OpenOversight to require MFA on my account.
  • I want to be able to reset MFA for users assigned to my department.
  • I want an admin or another area coordinator in my department to be able to reset MFA on my account.
  • I want OpenOversight to require MFA when I log in or update my account settings, such as my email address and password.

As an OpenOversight user:

  • I want the option to add MFA to my OpenOversight account.
  • I want to be able to remove MFA from my account.
  • I want my admin or area coordinator to be able to reset MFA on my account.
  • I want OpenOversight to require MFA when I log in or update my account settings, such as my email address and password. if MFA is set up on my account.

Anticipated Changes

  1. In models.py, create an OTP model and add a 1-1 relationship with the User model. The model should include at least the following fields:
  • otp_secret: the secret used to generate OTP tokens
  • last_token: the last token used, to prevent token re-use
  • otp_type: whether the secret is TOTP or HOTP
  1. Update the LoginForm to take an MFA token and update the login route to validate the OTP, if provided.
  • SecureDrop uses the cryptography library to implement OTP (see Related Links below), pyotp is also a solid choice
  1. Add a page (route and template) for users to add/update/remove MFA and link this page from the account dropdown
  2. Update EditUserForm and edit_user route to remove user MFA.
  3. Update admin_required/ac_or_admin_required and login route to redirect to add MFA page if it is required for user but has not been added (Is this intrusive enough or should all pages redirect?)
  4. Update change password/change email/edit user(?)/etc pages to require MFA code before committing update
  • A future task could be to hide admin functions behind a "sudo"-type MFA check

Open Questions

Blocking questions in bold:

  • Should we require all users to add MFA instead of only admins? (Or should this be configurable at the instance/department level?)

    Configurable probably would be best here, for openoversight.com I don't think we would ever require users to have MFA enabled.

  • Should users who have not logged in in X time and who have not added MFA be automatically deactivated?

    For instances that don't allow anyone to create an account this might make sense. I would probably implement this as a separate feature however.

  • Should there be other mechanisms for resetting MFA beside reaching out to ACs/admins?

    I don't think other mechanisms are necessary for our use case. The option I see github using are having a recovery code or following a time-delayed automatic process. I can't really imagine these would actually be used (in our case) and they also increase the overall attack surface so I would be against adding other mechanisms

  • Should MFA be turned on in test/local dev? (Leaning toward yes, although having to input an MFA code while testing may get annoying)

    If it can be turned on in dev, that would be great, but personally I would prefer it be turned off by default

  • If an admin without MFA setup logs in, should there be additional checks (ex: email verification) before being allowed to set MFA?

    Good question. Email verification would make sense to me. But I think that functionality wouldn't need to exist right away.

  • How does MFA interact with the Reset Password workflow?

    I don't think there is a specific interaction required. If you only forgot your password, you can reset it as before and use the existing MFA to log in after resetting the password.

  • What if there is only one admin? Is there any way for them to reset their MFA through the website?

    No, but I think having a cli command to reset the MFA makes sense

Related Links

@sea-kelp sea-kelp added enhancement New feature or request good first issue Good for newcomers labels May 10, 2023
@sea-kelp
Copy link
Collaborator Author

Tagging @abandoned-prototype and @dismantl in case you had any thoughts before we start working on this

@abandoned-prototype
Copy link

A pretty exciting and important addition!
I don't have much experience with implementing MFA, but I will offer my thoughts on the questions. Feel free to disagree or ignore :)

Should we require all users to add MFA instead of only admins? (Or should this be configurable at the instance/department level?)

Configurable probably would be best here, for openoversight.com I don't think we would ever require users to have MFA enabled.

Should users who have not logged in in X time and who have not added MFA be automatically deactivated?

For instances that don't allow anyone to create an account this might make sense. I would probably implement this as a separate feature however.

Should there be other mechanisms for resetting MFA beside reaching out to ACs/admins?

I don't think other mechanisms are necessary for our use case. The option I see github using are having a recovery code or following a time-delayed automatic process. I can't really imagine these would actually be used (in our case) and they also increase the overall attack surface so I would be against adding other mechanisms

Should MFA be turned on in test/local dev? (Leaning toward yes, although having to input an MFA code while testing may get annoying)

If it can be turned on in dev, that would be great, but personally I would prefer it be turned off by default

If an admin without MFA setup logs in, should there be additional checks (ex: email verification) before being allowed to set MFA?

Good question. Email verification would make sense to me. But I think that functionality wouldn't need to exist right away.

How does MFA interact with the Reset Password workflow?

I don't think there is a specific interaction required. If you only forgot your password, you can reset it as before and use the existing MFA to log in after resetting the password.

What if there is only one admin? Is there any way for them to reset their MFA through the website?

No, but I think having a cli command to reset the MFA makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants