[Django] #36717: Admin login should redirect already logged-in users to page specified in next parameter

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
------------------------------------------+--------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+--------------------------------
The admin login page at `/admin/login/` ignores the `?next=` parameter
when an already logged-in user visits the page. This differs from the
behaviour when the user logs in on the admin login page itself, where they
are redirected to the page indicated in the parameter.

== Background

When visiting the contrib.admin, the `AdminSite.login` method redirects
logged-out users to a URL that looks like this:
`/admin/login/?next=/admin/my_app/my_model/`. The `next` field (which is
the value of `contrib.auth.REDIRECT_FIELD_NAME`) is then used to populate
a hidden field login form (`contrib.auth.LoginView`).
After signing in, the `contrib.auth.RedirectURLMixin` of the `LoginView`
redirects the user to the initially requested URL.

However, sometimes may re-visit
`/admin/login/?next=/admin/my_app/my_model/` when they're already signed
in. For example:
- by signing in to the site in another tab
- by using the browser's back button
- by following a bookmark
- after custom authenticating overriding `admin.site.login` (a good
example is django-allauth's `secure_django_login`
https://docs.allauth.org/en/dev/common/admin.html that redirects visitors
to `/admin/login/` to a custom login and then back to `/admin/login/`.
Even though it keeps `next` intact, Django will always redirect the -
already logged-in - user back to the admin index)

Currently, `AdminSite.login` method disregards any `next` param in the URL
for logged-in staff users and always redirects these to `admin:index`.
This is slightly annoying because the user could be redirected these to
the requested URL instead.

== To reproduce:
1. Sign out of your Django instance
2. Manually open an admin deeplink such as `/admin/my_app/my_model/`,
which should redirect to `/admin/login/?next=/my_app/my_model/`
3. In another tab, sign in to your Django instance as staff
4. Back in the original tab, refresh the page. Observe how you are sent to
`/admin/`, instead of `/admin/my_app/my_model/`

== Possible fix:
`AdminSite.login` could respect the redirect URL, and should probably run
checks like `contrib.auth.views.RedirectURLMixin` (specifically
`get_redirect_url`), to ensure that the URL is safe. I don't think there's
a security issue here where somebody malicious links an admin to
`/admin/login/?next=/attacker-provider`, because the attacker could
already link a user directly to that page (or to the login form that then
redirects the admin to the page).

I'm assigning this to myself for now to "claim" implementing this. I
already have a working prototype locally.
--
Ticket URL: <https://code.djangoproject.com/ticket/36717>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter

--------------------------------+------------------------------------------

Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2

Severity: Normal | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------+------------------------------------------
Changes (by Benedict Etzel):

* cc: Benedict Etzel (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/36717#comment:1>

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------
Changes (by Benedict Etzel):

* Attachment "test.diff" added.

--
Ticket URL: <https://code.djangoproject.com/ticket/36717>

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------

Comment (by Benedict Etzel):

I've attached the failing test case. I'll be working on this over in
https://github.com/beheh/django/tree/ticket_36717.
--
Ticket URL: <https://code.djangoproject.com/ticket/36717#comment:2>

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------

Changes (by Benedict Etzel):

* has_patch: 0 => 1

Comment:

I have a patch now in the linked branch.

The `url_if_safe check` comes from `contrib.auth.views.RedirectURLMixin`
(https://github.com/django/django/blob/77d455ae73b177a32102f0b248828b5d63c0aa24/django/contrib/auth/views.py#L48-L56),
with the exception of the overridable `success_url_allowed_hosts` - I'm
not sure how we could do that here (except for adding to
`success_url_allowed_hosts` to `AdminSite`).
--
Ticket URL: <https://code.djangoproject.com/ticket/36717#comment:3>

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------
Changes (by Benedict Etzel):

* Attachment "test.2.diff" added.

--
Ticket URL: <https://code.djangoproject.com/ticket/36717>

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------
Changes (by Benedict Etzel):

* Attachment "test.2.diff" removed.

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------
Changes (by Benedict Etzel):

* Attachment "test.diff" added.

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------
Changes (by Benedict Etzel):

* Attachment "test.diff" removed.

[Django]

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------
Changes (by Benedict Etzel):

* Attachment "test.diff" added.

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter
--------------------------------+------------------------------------------
Reporter: Benedict Etzel | Owner: Benedict Etzel
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------------

Comment (by Benedict Etzel):

There's now a PR at https://github.com/django/django/pull/20076
--
Ticket URL: <https://code.djangoproject.com/ticket/36717#comment:4>

[Django]

#36717: Admin login should redirect already logged-in users to page specified in
next parameter

-------------------------------------+-------------------------------------

Reporter: Benedict Etzel | Owner: Benedict

Type: | Etzel
Cleanup/optimization | Status: assigned

Component: contrib.admin | Version: 5.2
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* stage: Unreviewed => Accepted
* type: Bug => Cleanup/optimization

Comment:

Hello Benedict, thank you for your report! I have reproduced as you
described and I have also noticed this behavior in the past. I agree is
fairly unexpected and that we can do better. Accepting on that basis, I'll
try to review the branch soon.
--
Ticket URL: <https://code.djangoproject.com/ticket/36717#comment:5>