Documentation error which can cause TFA bypass

[pjensen000]

The documentation, in the Installation section, has a warning at the end about the admin site, and then some sample code.

The sample code has an error which can result in TFA being bypassed for the admin site. The scenario is a site where some users do not have/need TFA, but all admin users do. Using the code provided, an attacker can login with a non-admin account without TFA. Then browse directly to the admin site, and are allowed to re-login there without TFA. The redirect provided in the documentation code:

admin.site.login = login_required(admin.site.login)

does not trigger the login_required wrapper because the user logged in already. They are instead taken directly to the admin site, which provides a login view that bypassess TFA.

FIX:

Instead of wrapping with login_required, wrap with staff_member_required:

`from django.contrib.admin.views.decorators import staff_member_required

admin.site.login = staff_member_required(admin.site.login, login_url='/accounts/login')`

This will the disallow showing the admin site to anyone not logged in as an admin, preventing the attack described.

[clokep]

Thanks @pjensen000! Any chance you can provide this as a PR?

[pjensen000]

Sorry, I don't have any git tools installed or knowledge on how to use them. I just wanted to share the issue report.

[dicato]

@pjensen000 / @hailkomputer do you have steps to recreate and test this? Your analysis makes sense but I would prefer to confirm the proposed fix before publishing it.

[pjensen000]

Steps to recreate:
0) assume we are using Django session authentication

1. Create any user that does not need TFA to login (user 1)
2. Create an admin user who does need TFA to login and has admin site access (user 2)
3. Login as user 1
4. browse directly to your admin site login url (htttps://mysite.com/admin/)
5. Since you are already logged in as user 1, the login_required mixin does not stop you from seeing the login page
6. login to the admin site using the default admin login screen which requires username/password but does not prompt for TFA

I can verify that I did this and was able to get in using the login_required decorator. I can also verify that I switched to staff_member_required and now only people already logged in as staff can see the login screen.

Technically, if some staff don't have/need TFA and others do, you could still bypass TFA by logging in as a staff who doesn't have TFA and then doing the same procedure as above. In my case all staff are required to have TFA. The fix only works if all staff are required to use TFA.

A more universal fix would be to write a mixin like staff_member_required except instead it's tfa_enabled_user_required. I have not written such a mixin.

[akx]

django-otp (used by this library) also sets a session key to denote a session has been OTP'd. That should be also checked by middleware/decorators...

https://github.com/django-otp/django-otp/blob/cd8429869ff580d3ae70da0cba3f615d95fcc6a6/src/django_otp/__init__.py#L25-L27