Opened 9 years ago

Closed 9 years ago

#25032 closed Cleanup/optimization (fixed)

When /admin/login/ is accessed directly, there is 302 /admin/login/ after POST, and only then 302 /admin/

Reported by: Jan Pazdziora Owned by: nobody
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

While investigating the behaviour for https://code.djangoproject.com/ticket/25030 in more detail, I've noticed that when I access (unauthenticated, via GET) /admin/login/ directly, the access_log shows

GET /admin/login/ HTTP/1.1" 200 1716
POST /admin/login/ HTTP/1.1" 302 - 
GET /admin/login/ HTTP/1.1" 302 - 
GET /admin/ HTTP/1.1" 200 2826

The result of the form submission (the POST) is redirect to /admin/login/ again. It seems to be caused by

        if (REDIRECT_FIELD_NAME not in request.GET and
                REDIRECT_FIELD_NAME not in request.POST):
            context[REDIRECT_FIELD_NAME] = request.get_full_path()

When REDIRECT_FIELD_NAME is missing, why is the redirect going to /admin/login/ again? Wouldn't /admin/ be a better target?

Of course, the

        if request.method == 'GET' and self.has_permission(request):
            # Already logged-in, redirect to admin index
            index_path = reverse('admin:index', current_app=self.name)
            return HttpResponseRedirect(index_path)

will eventually throw it back to /admin/.

This might seem like nitpicking but figuring out correct intended behaviour seems essential when I'm attempting to make changes to it to better support external authentication.

Change History (4)

comment:1 by Jan Pazdziora, 9 years ago

Filed ​https://github.com/django/django/pull/4927 to use the same default redirect target as the authenticated GET access.

comment:2 by Tim Graham, 9 years ago

Has patch: set
Needs tests: set
Triage Stage: Unreviewed → Accepted
Type: Uncategorized → Cleanup/optimization

comment:3 by Jan Pazdziora, 9 years ago

Needs tests: unset

comment:4 by Tim Graham <timograham@…>, 9 years ago

Resolution: → fixed
Status: new → closed

In 3353684:

Fixed #25032 -- Removed double redirect in admin login.

Note: See TracTickets for help on using tickets.
Back to Top