django-allauth icon indicating copy to clipboard operation
django-allauth copied to clipboard

Django-allauth views not compatible with Django 5.1 LoginRequiredMiddleware

Open danjac opened this issue 1 year ago • 7 comments

As of Django 5.1, the LoginRequiredMiddleware will be part of the Django core. This will be included by default with new Django projects in the MIDDLEWARE setting. All Django views will redirect to LOGIN_URL by default if the user is not authenticated.

There is a decorator login_not_required which should be added to any views you do not wish to apply the redirect.

Currently django-allauth login, signup and other views do not use this decorator, so will cause an infinite loop when trying to access them.

As Django 5.1 is due for production release in a few weeks, is there a branch for handling 5.1 compatibility?

danjac avatar Aug 01 '24 08:08 danjac

This will be included by default with new Django projects in the MIDDLEWARE setting.

Where did you read that? It's not documented here:

https://docs.djangoproject.com/en/dev/topics/http/middleware/#activating-middleware

AFAICT, LoginRequiredMiddleware is not enabled by default, meaning, out of the box, django-allauth is compatible with Django 5.1

pennersr avatar Aug 01 '24 08:08 pennersr

That is perhaps my misunderstanding. Nevertheless, it is likely this middleware will be commonly used in projects that want to provide a default authentication check for all views, so is there a plan for adding the login_not_required middleware to all-auth views?

danjac avatar Aug 01 '24 08:08 danjac

it is likely this middleware will be commonly used

Let's see how the eco system around this change evolves first, switching to LoginRequiredMiddleware would break a lot of upstream projects, so it will definitely take some time for that to become the default, if we ever reach that point.

Having said that, if anybody wants to propose a solution for gracefully handling this within django-allauth, feel free to go ahead.

pennersr avatar Aug 01 '24 08:08 pennersr

There may be limited if any need for allauth to switch to using the new LoginRequiredMiddleware, as the middleware can just be subclassed by a custom middleware and adapted for a given project's needs. For example, the below middleware uses the new LoginRequiredMiddleware to require authentication for all urls except for the login url and any other urls in OPEN_URLS:

class LoginRequiredMiddleware(LoginRequiredMiddleware):
    def __init__(self, get_response):
        self.login_url = reverse(settings.LOGIN_URL)
        self.open_urls = [self.login_url, *getattr(settings, "OPEN_URLS", [])]
        super().__init__(get_response)

    def process_view(self, request, view_func, view_args, view_kwargs):
        if request.path_info in self.open_urls:
            return None
        return super().process_view(request, view_func, view_args, view_kwargs)

mcastle avatar Aug 08 '24 21:08 mcastle

There may be limited if any need for allauth to switch to using the new LoginRequiredMiddleware, as the middleware can just be subclassed by a custom middleware and adapted for a given project's needs. For example, the below middleware uses the new LoginRequiredMiddleware to require authentication for all urls except for the login url and any other urls in OPEN_URLS:

class LoginRequiredMiddleware(LoginRequiredMiddleware):
    def __init__(self, get_response):
        self.login_url = reverse(settings.LOGIN_URL)
        self.open_urls = [self.login_url, *getattr(settings, "OPEN_URLS", [])]
        super().__init__(get_response)

    def process_view(self, request, view_func, view_args, view_kwargs):
        if request.path_info in self.open_urls:
            return None
        return super().process_view(request, view_func, view_args, view_kwargs)

Given that there is a decorator login_not_required it would probably be easier to wrap all the allauth URLs (or at least the ones you are using) e.g.

from allauth.accounts import views
from django.contrib.auth.decorators import login_not_required

login = login_not_required(views.login)

danjac avatar Aug 09 '24 08:08 danjac

There may be limited if any need for allauth to switch to using the new LoginRequiredMiddleware, as the middleware can just be subclassed by a custom middleware and adapted for a given project's needs. For example, the below middleware uses the new LoginRequiredMiddleware to require authentication for all urls except for the login url and any other urls in OPEN_URLS:

class LoginRequiredMiddleware(LoginRequiredMiddleware):
    def __init__(self, get_response):
        self.login_url = reverse(settings.LOGIN_URL)
        self.open_urls = [self.login_url, *getattr(settings, "OPEN_URLS", [])]
        super().__init__(get_response)

    def process_view(self, request, view_func, view_args, view_kwargs):
        if request.path_info in self.open_urls:
            return None
        return super().process_view(request, view_func, view_args, view_kwargs)

Given that there is a decorator login_not_required it would probably be easier to wrap all the allauth URLs (or at least the ones you are using) e.g.

from allauth.accounts import views
from django.contrib.auth.decorators import login_not_required

login = login_not_required(views.login)

That's also a good approach. It would need to be done at the url level:

from allauth.account import views
from django.contrib.auth.decorators import login_not_required

path("accounts/login/", login_not_required(views.login), name="account_login"),

mcastle avatar Aug 09 '24 17:08 mcastle

For django-allauth to support projects that use LoginRequiredMiddleware, I'm pretty sure it only needs to apply the login_not_required decorator to views that accept unauthenticated users. That decorator is very simple, here is in its entire implementation:

def login_not_required(view_func):
    """
    Decorator for views that allows access to unauthenticated requests.
    """
    view_func.login_required = False
    return view_func

Flimm avatar Aug 20 '24 13:08 Flimm

Moved to https://codeberg.org/allauth/django-allauth/issues/4001

pennersr avatar Sep 10 '24 10:09 pennersr

FYI: https://codeberg.org/allauth/django-allauth/pulls/4104

pennersr avatar Sep 13 '24 13:09 pennersr

Supported via d096a0b6

pennersr avatar Sep 13 '24 15:09 pennersr