Support two factor auth (2FA)

agjohnson commented 6 years ago

We should enable 2fa for dashboard users. I keep wanting to add site admin features to the dashboard, but then think about the security aspects of adding these features and find myself also wanting 2fa. There are some libraries that do handle a 2fa workflow for standard django logins, but i don't know if this extends to django + allauth or django + mamacas.

I'm sure we're probably in agreement of this being an important feature, but I'm not sure we can gauge the importance of 2fa for users. I'm sure community users would use a feature like this, and site admins would use this feature -- I doubt this is in high demand for commercial hosting customers though.

The following thoughts come to mind:

Is this too hard? Does allauth play well with a 2fa workflow?
Is there any reason besides complicating login that we shouldn't?

ericholscher commented 6 years ago

I'm +0 on adding it. I don't think RTD is so sensitive that we are a common attack vector. I'm much more worried about building authoring features before building something like this, unless it's simple to do with a pluggable Django app. Unless users are specifically asking for this, I don't see it as a high priority (sadly).

agjohnson commented 6 years ago

Yeah, i agree on priority here. This is a feature that i consider more important for commercial hosting, but I also haven't had any requests for this feature though.

agjohnson commented 6 years ago

Also, I think a lot of what I want to add would probably be more applicable as a django admin action instead of an on site admin only feature.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ericholscher commented 5 years ago

Accepted :+1:

dojutsu-user commented 5 years ago

@agjohnson

more applicable as a django admin action instead of an on site admin only feature

I am a little confused on this line on what this means? Also the Needed: design decision tag is removed. Can it be made clear on what needs to be done?

MatteoGheza commented 3 years ago

Any update on this?

ericholscher commented 3 years ago

Sadly not -- we'd love to support it, but it isn't on our short term roadmap. If there is a good way to handle this via Django, we'd love to know, but I haven't found one.

humitos commented 3 years ago

Today I did quick search and I found this one https://django-allauth-2fa.readthedocs.io/en/latest/, which looks like a good candidate since it should integrate directly with our current auth system: django-allauth.

humitos commented 1 year ago

There is another Python package that could be useful for this https://github.com/justinmayer/kagi

ericholscher commented 1 year ago

I do wonder about basically just punting on this, and saying to use one of our SSO options if you want 2FA. I think more and more users are going to be defaulting to logging in with those options to enable our VCS SSO anyway.

humitos commented 7 months ago

I think suggesting SSO options is the way to go here. However, is possible to "remove the password" from an existing user after connecting GitHub for example? I did a quick check and I wasn't able to find it.

stsewd commented 6 months ago

Django allauth now has built-in support for 2FA https://docs.allauth.org/en/latest/mfa/introduction.html

humitos commented 3 months ago

We talk about supporting this during the offsite if it wasn't super complicated. I'm adding this issue to 2024Q3 to see if we can prioritize it.

ericholscher commented 3 months ago

Yea, now that allauth supports it, would be great to add 👍

ericholscher commented 1 month ago

Putting this on our next sprint, as this is now supported in AllAuth, hopefully this will be pretty straightforward 🙏

stsewd commented 1 month ago

Well, it was easy to integrate. Just install a new dependency django-allauth[mfa], and install the app.

Things to take into consideration:

We probably want to override all the templates related to mfa, the default ones don't look great with our theme.
~~I see in the code that the current hostname is used for some things, we should check that the auth works on both domains (rtd.org/com and app.rtd.org/com), or do some overrides.~~ Edit: This is used for webauthn only.
The name of the site is taken from the Site object, we should make sure it says Read the Docs or Read the Docs Community or whatever.
The recovery codes are not encrypted by default, we need to implement that for ourselves. See encrypt/decrypt in https://docs.allauth.org/en/latest/mfa/adapter.html. I think it makes sense to encrypt the recovery codes. Update: Looks like what they give you to encrypt is a seed, the codes themselves are generated from that seed, so, it could be useful to encrypt that too.

ericholscher commented 1 month ago

It sounds like overall the main work is templating then and some QA? I assume the templates should be pretty simple, so hopefully not a ton of work to theme.

The name of the site is taken from the Site object, we should make sure it says Read the Docs or Read the Docs Community or whatever.

I confirmed these

stsewd commented 1 month ago

Yep, if we are cool with that, I can open a PR with the changes.

stsewd commented 1 week ago

Note: support for 2FA has been added, but it won't be exposed to users yet, we will be exposing this feature in the new dashboard (once it's ready).

agjohnson commented 6 days ago

@stsewd Could you give the URL for the mfa settings?

readthedocs / readthedocs.org

Support two factor auth (2FA) #3523