django-two-factor-auth icon indicating copy to clipboard operation
django-two-factor-auth copied to clipboard

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)

Open nicomak opened this issue 8 years ago • 14 comments

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 !

nicomak avatar Aug 02 '17 16:08 nicomak

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.

Bouke avatar Aug 10 '17 20:08 Bouke

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?

skyl avatar Oct 03 '17 06:10 skyl

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.

JVanloofsvelt avatar Dec 15 '17 11:12 JVanloofsvelt

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

rjskene avatar Feb 03 '18 18:02 rjskene

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)

GaramNick avatar Nov 15 '18 14:11 GaramNick

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

saschwarz avatar May 21 '19 13:05 saschwarz

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

aseem-hegshetye avatar Aug 05 '20 15:08 aseem-hegshetye

Pull requests welcome :smiley_cat:

moggers87 avatar Aug 05 '20 17:08 moggers87

Any chance to get this pull request merged?

limolitz avatar Jul 13 '22 07:07 limolitz

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

RyanHope avatar Mar 22 '23 21:03 RyanHope

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),
]

rob32 avatar Dec 21 '23 12:12 rob32

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?

andrelccorrea-blinctek avatar Mar 25 '24 13:03 andrelccorrea-blinctek