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

Allauth admin integration issues #3786

Closed
jkaeske opened this issue May 7, 2024 · 6 comments
Closed

Allauth admin integration issues #3786

jkaeske opened this issue May 7, 2024 · 6 comments

Comments

@jkaeske
Copy link

jkaeske commented May 7, 2024

Problem

To force allauth workflow in Django admin the docs propose this change:

from django.conf import settings
from django.contrib import admin
from django.contrib.admin.views.decorators import staff_member_required

admin.site.login = staff_member_required(
    admin.site.login, login_url=settings.LOGIN_URL
)

Django cookiecutter workaround

This however leads to ERR_TOO_MANY_REDIRECTS.

Django cookiecutter uses this as a workaround:

from django.contrib.auth import decorators

if settings.DJANGO_ADMIN_FORCE_ALLAUTH:
    # Force the `admin` sign in process to go through the `django-allauth` workflow:
    # https://django-allauth.readthedocs.io/en/stable/advanced.html#admin
    admin.site.login = decorators.login_required(admin.site.login)  # type: ignore[method-assign]

That however makes it possible to go around the allauth by logging in as a non-superuser.
When logging in as a normal user, the user gets redirected to the Django admin site with "You are logged in but do not have access to this page. Do you want to log in as a different user?".

Discussion

We discussed the issue and possible workarounds here:
cookiecutter/cookiecutter-django#4766

We figured out a possible way to avoid the "You are logged in but do not have access to this page. Do you want to log in as a different user?" and drop connections, that are not authenticated as superusers.

I am not sure if that is the best thing to do.
@pennersr do you have any recommendations here?
I am happy to contribute I am just not sure where to take this from here.

@joonhyungshin
Copy link
Contributor

I also faced this problem, and #686 helped. You can set ACCOUNT_AUTHENTICATED_LOGIN_REDIRECTS = False and the allauth login page won't redirect you even if you're authenticated. Probably you'll want to display a message "you don't have access" or something to non-staff users, which can be done by customizing the login page. You can also customize the admin login view so that it will only redirect non-authenticated user to the allauth login page and take non-staff users to somewhere else (e.g. index).

@bluesurfer
Copy link

Same problem here. It is pretty critical since the MFA should be meant for the admin first.

@joonhyungshin to me it is not enough setting ACCOUNT_AUTHENTICATED_LOGIN_REDIRECTS. I think the only way is (like you said) to override the Django admin site.

@joonhyungshin
Copy link
Contributor

joonhyungshin commented May 8, 2024

Yes, if that doesn't work for you you can always customize your admin site.

from functools import update_wrapper

from django.conf import settings
from django.contrib import admin
from django.http import HttpResponseRedirect
from django.urls import reverse
from django.views.decorators.cache import never_cache
from django.views.decorators.csrf import csrf_protect

class MyAdminSite(admin.AdminSite):
    def admin_view(self, view, cacheable=False):
        def inner(request, *args, **kwargs):
            if not request.user.is_authenticated:
                if request.path == reverse("admin:logout", current_app=self.name):
                    index_path = reverse("admin:index", current_app=self.name)
                    return HttpResponseRedirect(index_path)
                from django.contrib.auth.views import redirect_to_login

                return redirect_to_login(request.get_full_path())
            if not self.has_permission(request):
                return HttpResponseRedirect(settings.LOGIN_REDIRECT_URL)
            return view(request, *args, **kwargs)

        if not cacheable:
            inner = never_cache(inner)
        if not getattr(view, "csrf_exempt", False):
            inner = csrf_protect(inner)
        return update_wrapper(inner, view)

Perhaps a simpler solution is to define a custom decorator and use in lieu of staff_member_required:

from django.conf import settings
from django.contrib.admin.views.decorators import staff_member_required
from django.contrib.auth import REDIRECT_FIELD_NAME
from django.contrib.auth.decorators import login_required

def staff_member_required_or_fail(
    view_func=None,
    redirect_field_name=REDIRECT_FIELD_NAME,
    login_url=settings.LOGIN_URL,
    fail_url=settings.LOGIN_REDIRECT_URL,
):
    return login_required(
        staff_member_required(view_func, redirect_field_name=None, login_url=fail_url),
        redirect_field_name=redirect_field_name,
        login_url=login_url,
    )

This will take staff users to the view and non-staff users to fail_url. This makes the url slightly uglier, but it should work.

@jkaeske
Copy link
Author

jkaeske commented May 9, 2024

@joonhyungshin thank you very much for your effort!
I like the solution with the custom decorator, it is clean and works well

This is definitely something that should be included in the allauth docs.
Do you want to open a PR or should I do it?
Maybe we could call it staff_member_required_or_redirect instead.

@joonhyungshin
Copy link
Contributor

Yes, I do hope allauth docs makes this point more clearly. Please feel free to open a PR.

@pennersr
Copy link
Owner

There is now a decorator that is properly working out of the box: https://docs.allauth.org/en/latest/common/admin.html

b1786ed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants