Admin site does not redirect to two factor setup if admin user does not have second factor (was: Explain 2FA requirement if not configured in admin site)

[nicomak]

Hi, I followed the example site and managed to get the "secret" page working with 2FA setup on my website.

However, to protect the admin site, I found this ReadTheDocs article, but it doesn't seem to work. Is this procedure still the current way to make 2FA work on the admin site ?
Using this, when I go to the admin site there is directly a username/password form (instead of asking to activate 2FA like in the "secret" page). And the login form doesn't work, it keeps re-showing the same form even if the credentials are correct.

This is my url config:

from two_factor.gateways.twilio.urls import urlpatterns as tf_twilio_urls
from two_factor.urls import urlpatterns as tf_urls

admin.site.__class__ = AdminSiteOTPRequired
admin.autodiscover()

urlpatterns = [    
    url(r'^secret/$', ExampleSecretView.as_view(), name="secret"),
    url(r"^admin/", include(admin.site.urls)),
    url(r'', include(tf_urls + tf_twilio_urls, "two_factor")),
]

Did I do something wrong ? Any help would be greatly appreciated, thanks !

[Bouke]

The admin configuration is two-fold;

1. the login view is automatically monkey patched to use this package's login view
2. AdminSiteOTPRequired requires that all users accessing the page have been authenticated using two factors

And the login form doesn't work, it keeps re-showing the same form even if the credentials are correct.

I'm assuming your user doesn't have a second factor enabled. At the moment I don't think an error is shown. It would be nice to inform you that although your credentials are correct, you cannot proceed as you don't have a second factor. Instead it is redirecting you to the login page, making you think that the login page is broken.

[skyl]

Even better, if you're trying to go to a otp-requiring page, wouldn't it make sense to redirect to the wizard to setup 2fa?

[JVanloofsvelt]

Agreed @Bouke , this confused me a lot too! I feel like it should allow the login to succeed, and then allow 2FA to be set up once you're logged in.

[rjskene]

i must admit to being stumped by this. How do you set 2FA on the admin users if you can't login to them?

[GaramNick]

If you update the login in the AdminSiteOTPRequiredMixin to this, it redirects to the setup:

def login(self, request, extra_context=None):
    redirect_to = request.POST.get(REDIRECT_FIELD_NAME, request.GET.get(REDIRECT_FIELD_NAME))
    if request.method == "GET" and super(AdminSiteOTPRequiredMixinRedirSetup, self).has_permission(request):
            # Already logged-in
            if request.user.is_verified():
                # User has permission
                index_path = reverse("admin:index", current_app=self.name)
            else:
                # User has permission but no OTP set:
                index_path = reverse("two_factor:setup", current_app=self.name)
            return HttpResponseRedirect(index_path)

        if not redirect_to or not is_safe_url(url=redirect_to, allowed_hosts=[request.get_host()]):
            redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL)

        return redirect_to_login(redirect_to)

[saschwarz]

Thanks @GaramNick for your snippet! I ended up putting your idea into a subclass and had to adjust the super() call to has_permission(request) so the default admin version would be called:

class AdminSiteOTPRequiredMixinRedirSetup(AdminSiteOTPRequired):
    def login(self, request, extra_context=None):
        redirect_to = request.POST.get(
            REDIRECT_FIELD_NAME, request.GET.get(REDIRECT_FIELD_NAME)
        )
        # For users not yet verified the AdminSiteOTPRequired.has_permission
        # will fail. So use the standard admin has_permission check:
        # (is_active and is_staff) and then check for verification.
        # Go to index if they pass, otherwise make them setup OTP device.
        if request.method == "GET" and super(
            AdminSiteOTPRequiredMixin, self
        ).has_permission(request):
            # Already logged-in and verified by OTP
            if request.user.is_verified():
                # User has permission
                index_path = reverse("admin:index", current_app=self.name)
            else:
                # User has permission but no OTP set:
                index_path = reverse("two_factor:setup", current_app=self.name)
            return HttpResponseRedirect(index_path)

        if not redirect_to or not is_safe_url(
            url=redirect_to, allowed_hosts=[request.get_host()]
        ):
            redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL)

        return redirect_to_login(redirect_to)

Then:

admin.site.__class__ = AdminSiteOTPRequiredMixinRedirSetup

[aseem-hegshetye]

Any plans on including this in django-two-factor-auth package so we dont have manually do this in each project ?

[moggers87]

Pull requests welcome 😺

[aseem-hegshetye]

Pull request

[limolitz]

Any chance to get this pull request merged?

[RyanHope]

Yes this seems like the way things should be by default.

[rob32]

thanks @saschwarz and @GaramNick! That helped enormously.

Here is my snippet with imports etc. (is_safe_url is now called url_has_allowed_host_and_scheme)

from django.contrib import admin
from django.contrib.auth import REDIRECT_FIELD_NAME
from django.contrib.auth.views import redirect_to_login
from django.http import HttpResponseRedirect
from django.shortcuts import resolve_url
from django.urls import include, path, reverse
from django.utils.http import url_has_allowed_host_and_scheme

from two_factor.admin import AdminSiteOTPRequired, AdminSiteOTPRequiredMixin
from two_factor.urls import urlpatterns as tf_urls

class CustomAdminSiteOTPRequired(AdminSiteOTPRequired):
    def login(self, request, extra_context=None):
        redirect_to = request.POST.get(
            REDIRECT_FIELD_NAME, request.GET.get(REDIRECT_FIELD_NAME)
        )
        if request.method == "GET" and super(
            AdminSiteOTPRequiredMixin, self
        ).has_permission(request):
            if request.user.is_verified():
                index_path = reverse("admin:index", current_app=self.name)
            else:
                index_path = reverse("two_factor:setup", current_app=self.name)
            return HttpResponseRedirect(index_path)

        if not redirect_to or not url_has_allowed_host_and_scheme(
            url=redirect_to, allowed_hosts=[request.get_host()]
        ):
            redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL)

        return redirect_to_login(redirect_to)


admin.site.__class__ = CustomAdminSiteOTPRequired

urlpatterns = [
    # ...
    path("", include(tf_urls)),
    path("admin/", admin.site.urls),
]

[andrelccorrea-blinctek]

The previous comment has the solution.
The way the lib currently works is extremely confusing.
What is missing for this solution to be implemented as standard?

[moggers87]

Changed the title so this issue is easier to search for.