admin:logout fails to log out non-staff users

[Jan Pazdziora]

The Django 4.1 release notes at ​https://docs.djangoproject.com/en/4.1/releases/4.1/ show that the expected way to log out users is using admin:logout:

<form id="logout-form" method="post" action="{% url 'admin:logout' %}">
  {% csrf_token %}
  <button type="submit">{% translate "Log out" %}</button>
</form>

However, when the logged-in user does not have the is_staff attribute because it used a custom non-admin login, using such approach leads to a redirect back to /admin/ and /admin/login/?next=/admin/ and message

You are authenticated as bob, but are not authorized to access this page.
Would you like to login to a different account?

Since the user tries to log out, meaning strip themselves of any permissions, the authorization check that is currently in place for logout is likely wrong.

The behaviour change happened in ​https://github.com/django/django/commit/1f84630c87f8032b0167e6db41acaf50ab710879. The original code did

        # The 'logout' view doesn't require that the person is logged in.
        if url == 'logout':
            return self.logout(request)

        # Check permission to continue or display login form.
        if not self.has_permission(request):
            return self.login(request)

which the new code changed to

            url(r'^logout/$',
                wrap(self.logout),
                name='%sadmin_logout'),

with that wrap() around self.logout. So that refactoring change also changed the semantics of the logout behaviour

[Jan Pazdziora]

I've filed ​https://github.com/django/django/pull/16073 with proposed fix.

[Mariusz Felisiak]

Thanks for the report, however the proposed change is backward incompatible and introduces a regression (see #159 and 03eeb020a00dedba2594326bd606d8b41d51e80f). You should be able to use a custom AdminSite and overwrite AdminSite.has_permission(), e.g.

class CustomAdminSite(AdminSite):
    def has_permission(self, request):
        return request.user.is_active

[Jan Pazdziora]

Well, the change is backward incompatible in the sense that it changes behaviour, yes. But in my mind it fixes a problem which was introduced by some refactoring which changed behaviour for non-staff users by mistake.

In the ​https://github.com/django/django/pull/16073#issuecomment-1250690063 comment I've now shown an alternative approach which would only cater to the non-staff case and as shown by the unmodified test_client_logout_url_can_be_used_to_login (just renamed to test_logout_if_not_authenticated to make its purpose more explicit), the behaviour for the unauthenticated case does not change.

Let me reopen this ticket to have some more discussion about the non-staff case which as far as I can see is currently not captured by any tests so that behaviour seems to be undefined.

[Mariusz Felisiak]

I appreciate you'd like to reopen the ticket, but please ​follow the triaging guidelines with regards to wontfix tickets and take this to DevelopersMailingList.