[Django] #26902: Add `secure` argument to `is_safe_url()`
Django
-----------------------------+--------------------
Reporter: suligap | Owner: nobody
Type: New feature | Status: new
Component: Utilities | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------
`django.utils.http.is_safe_url()` considers any HTTP and HTTPS url safe as
long as its hostname matches the `host` argument. Currently this is true:
`is_safe_url('http://example.com', host='example.com')`.
Let's add a `secure` argument to `is_safe_url()` so that when it's `True`,
only HTTPS is considered as a safe scheme.
The existence of that argument alone would make users aware of potential
issues that can arise from ignoring it. For example if a developer uses
`is_safe_url()` to validate user supplied urls for redirection to a target
with appended secrets as url query params.
`django.contrib.admin` uses `django.contrib.auth` login view where
`is_safe_url()` is used to validate the `next` query param. This scenario
is currently possible:
- user goes to
https://example.net/admin/login/?next=http://example.net/admin/foo
- they enter their credentials and POST to the above url
- They're successfully authenticated, they receive a response with a new
session cookie and are redirected to http://example.net/admin/foo
Of course our HTTPS site should only set `Secure` session cookies and use
HSTS, so there should be no possibility of the the cookie being sent by
the user via HTTP. But if the site doesn't set secure cookies and doesn't
use HSTS, this is a problem. If the site doesn't use secure cookies in the
first place, then the `secure` param to `is_safe_url()` won't help much..
but I would argue it still makes the validation more "complete".
--
Ticket URL: <https://code.djangoproject.com/ticket/26902>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: suligap | Owner: nobody
Type: New feature | Status: new
Component: Utilities | Version: master
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 timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26902#comment:1>
Django
Reporter: suligap | Owner: nobody
Type: New feature | Status: new
Component: Utilities | Version: master
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by berkerpeksag):
* cc: berker.peksag@… (added)
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/6923 PR #6923] looks good to me. I
just left two minor comments about the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/26902#comment:2>
Django
Reporter: suligap | Owner: nobody
Type: New feature | Status: closed
Component: Utilities | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"5e5a17028f4b9cfb5ff777d8c259e079bca0c988" 5e5a1702]:
{{{
#!CommitTicketReference repository=""
revision="5e5a17028f4b9cfb5ff777d8c259e079bca0c988"
Fixed #26902 -- Allowed is_safe_url() to require an https URL.
Thanks Andrew Nester, Berker Peksag, and Tim Graham for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26902#comment:3>
Django
Reporter: suligap | Owner: nobody
Type: New feature | Status: closed
Component: Utilities | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"549b90fab33c80d1ba6575d4837ea52d79f70fb1" 549b90fa]:
{{{
#!CommitTicketReference repository=""
revision="549b90fab33c80d1ba6575d4837ea52d79f70fb1"
Refs #26902 -- Protected against insecure redirects in Login/LogoutView.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26902#comment:4>
Django
Reporter: suligap | Owner: nobody
Type: New feature | Status: closed
Component: Utilities | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"1f68bb5683608ebe1ce47f0c9d37dad9482d1df9" 1f68bb56]:
{{{
#!CommitTicketReference repository=""
revision="1f68bb5683608ebe1ce47f0c9d37dad9482d1df9"
Refs #26902 -- Protected against insecure redirects in set_language().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26902#comment:5>