[Django] #33648: Unnecessary redirect in LogoutView when ?next=... contains "unsafe" URL

14 views
Skip to first unread message

Django

unread,
Apr 16, 2022, 1:16:57 PM4/16/22
to django-...@googlegroups.com
#33648: Unnecessary redirect in LogoutView when ?next=... contains "unsafe" URL
-------------------------------------+-------------------------------------
Reporter: Aymeric | Owner: Aymeric Augustin
Augustin |
Type: | Status: assigned
Cleanup/optimization |
Component: | Version: 4.0
contrib.auth |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
**Reproduction instructions**

* Set `LOGOUT_REDIRECT_URL`
* Wire `LogoutView.as_view()` at `/logout/` in the URLconf
* Add this form to any template: `<form
action="/logout/?next=http://evil/" method="POST"><input type="submit"
value="Logout">{% csrf_token %}</form>`
* Log in, then use the form to log out

**Expected result**

You are logged out; the `next` parameter is ignored; you are redirected to
`LOGOUT_REDIRECT_URL`

**Actual result**

There is an intermediary, useless redirect; see the logs of the
development server:

{{{
[16/Apr/2022 19:05:38] "POST /logout/?next=http://evil/ HTTP/1.1" 302 0
[16/Apr/2022 19:05:38] "GET /logout/ HTTP/1.1" 302 0
[16/Apr/2022 19:05:38] "GET /en/ HTTP/1.1" 200 13918
}}}

I noticed this via code inspection. The implementation of
LogoutView.get_next_page seemed a bit weird to me.

This stems from
https://github.com/django/django/blame/e12670016bbcebcc0d89c2ac4a0121951181fbae/django/contrib/auth/views.py#L178
which predates the introduction of `LOGOUT_REDIRECT_URL`.

From the user's perspective, the behavior is correct. There's just an
extra round-trip and needlessly complicated code.

--
Ticket URL: <https://code.djangoproject.com/ticket/33648>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 16, 2022, 1:31:48 PM4/16/22
to django-...@googlegroups.com
#33648: Unnecessary redirect in LogoutView when ?next=... contains "unsafe" URL
-------------------------------------+-------------------------------------
Reporter: Aymeric Augustin | Owner: Aymeric
Type: | Augustin
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: 4.0
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 Aymeric Augustin):

* has_patch: 0 => 1


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

Django

unread,
Apr 16, 2022, 2:35:12 PM4/16/22
to django-...@googlegroups.com
#33648: Unnecessary redirect in LogoutView when ?next=... contains "unsafe" URL
-------------------------------------+-------------------------------------
Reporter: Aymeric Augustin | Owner: Aymeric
Type: | Augustin
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: 4.0
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 Mariusz Felisiak):

* stage: Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/33648#comment:2>

Django

unread,
Apr 18, 2022, 10:33:32 AM4/18/22
to django-...@googlegroups.com
#33648: Unnecessary redirect in LogoutView when ?next=... contains "unsafe" URL
-------------------------------------+-------------------------------------
Reporter: Aymeric Augustin | Owner: Aymeric
Type: | Augustin
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: 4.0
Severity: Normal | Resolution: fixed

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 GitHub <noreply@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"5591a72571b8a07c5e3d87dcfe08310bb7611d15" 5591a725]:
{{{
#!CommitTicketReference repository=""
revision="5591a72571b8a07c5e3d87dcfe08310bb7611d15"
Fixed #33648 -- Prevented extra redirect in LogoutView on invalid next
page when LOGOUT_REDIRECT_URL is set.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33648#comment:3>

Reply all
Reply to author
Forward
0 new messages