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 2FA to the login flow #1858

Closed
joren485 opened this issue Aug 3, 2021 · 6 comments · Fixed by #3572
Closed

Add 2FA to the login flow #1858

joren485 opened this issue Aug 3, 2021 · 6 comments · Fixed by #3572
Assignees
Labels
feature Issues regarding a complete new feature priority: high Must be dealt with before the next release is deployed. security Security related issues
Milestone

Comments

@joren485
Copy link
Contributor

joren485 commented Aug 3, 2021

Describe the solution you'd like

It would be great if there was an option to add a TOTP to user accounts.

Motivation

2FA adds an extra layer of security for the users. As many people use weak passwords or reuse passwords, having the option 2FA is considered best practice.

This is security is especially important for admin and moderator users. If one of their accounts get compromised, an attacker might be able to leak/alter sensitive data. If you implement this, you might want to consider to enforce 2FA for privileged accounts.

Additional context

As a study association that (partly) focuses on digital security, I feel that it is important to follow these types of best practices.

@joren485 joren485 added feature Issues regarding a complete new feature priority: low Should be dealt with when nothing else remains. labels Aug 3, 2021
@se-bastiaan
Copy link
Contributor

Good option probably: https://django-two-factor-auth.readthedocs.io/

@JobDoesburg
Copy link
Collaborator

If we were to do this, I think we should directly go for FIDO(2) https://pypi.org/project/django-mfa2/

@se-bastiaan
Copy link
Contributor

I do not particularly like that package. There are no tests, the code has no codestyle applied. (Note after I wrote this: other people agree)
Not that the library I linked is perfect, since PRs do not seem to be accepted. As is also the case for django-otp which seems to be in some kind of sleep mode?

I do agree that webauthn/fido2 is a good thing, but it doesn't look like there is an easy and good way to get it in.

@JobDoesburg
Copy link
Collaborator

I do agree that webauthn/fido2 is a good thing

My main point :)

@se-bastiaan
Copy link
Contributor

se-bastiaan commented Aug 15, 2021

https://github.com/oliwarner/django-multifactor/ another option, basically rewritten django-mfa2. Like it more, but it requires quite some custom templates, plus we need to find a way on how to force 2FA in places.

That library wants 2FA for specific views. I'd rather have it 1 time once you login, because that won't bother the API consumers since that can be part of the OAuth process.

So:

  • 2FA check immediately on login
  • Have support for backup options
  • have support to force users to setup 2FA

@pingiun pingiun added priority: maybe someday This is not really relevant, but if we have nothing else to do, then we can think about this. and removed priority: low Should be dealt with when nothing else remains. labels Sep 13, 2021
@JobDoesburg JobDoesburg linked a pull request Apr 15, 2022 that will close this issue
@DeD1rk DeD1rk added priority: medium A new feature or a bugfix that is non-critical. security Security related issues and removed priority: maybe someday This is not really relevant, but if we have nothing else to do, then we can think about this. labels Sep 2, 2023
@DeD1rk
Copy link
Member

DeD1rk commented Sep 2, 2023

I think it would be nice to do this soon. https://django-two-factor-auth.readthedocs.io/ is popular and stable now. It also supports webauthn.

Should be simple to set up, and after that we can look into enforcing 2fa, for example (perhaps gradually) for superusers, board, active members, people with thalia pay set up.

@DeD1rk DeD1rk added priority: high Must be dealt with before the next release is deployed. and removed priority: medium A new feature or a bugfix that is non-critical. labels Dec 13, 2023
@DeD1rk DeD1rk added this to the Release 52.0 milestone Dec 13, 2023
@Noah-marc Noah-marc self-assigned this Dec 14, 2023
@Noah-marc Noah-marc mentioned this issue Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Issues regarding a complete new feature priority: high Must be dealt with before the next release is deployed. security Security related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants