Skip to content

Commit

Permalink
Refs #26902 -- Protected against insecure redirects in Login/LogoutView.
Browse files Browse the repository at this point in the history
  • Loading branch information
suligap authored and timgraham committed Aug 19, 2016
1 parent 5e5a170 commit 549b90f
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
14 changes: 12 additions & 2 deletions django/contrib/auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ def get_success_url(self):
self.redirect_field_name,
self.request.GET.get(self.redirect_field_name, '')
)
if not is_safe_url(url=redirect_to, host=self.request.get_host()):
url_is_safe = is_safe_url(
url=redirect_to,
host=self.request.get_host(),
require_https=self.request.is_secure(),
)
if not url_is_safe:
return resolve_url(settings.LOGIN_REDIRECT_URL)
return redirect_to

Expand Down Expand Up @@ -150,8 +155,13 @@ def get_next_page(self):
self.redirect_field_name,
self.request.GET.get(self.redirect_field_name)
)
url_is_safe = is_safe_url(
url=next_page,
host=self.request.get_host(),
require_https=self.request.is_secure(),
)
# Security check -- don't allow redirection to a different host.
if not is_safe_url(url=next_page, host=self.request.get_host()):
if not url_is_safe:
next_page = self.request.path
return next_page

Expand Down
7 changes: 7 additions & 0 deletions docs/releases/1.11.txt
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,13 @@ to assign a free port. The ``DJANGO_LIVE_TEST_SERVER_ADDRESS`` environment
variable is no longer used, and as it's also no longer used, the
``manage.py test --liveserver`` option is removed.

Protection against insecure redirects in :mod:`django.contrib.auth` views
-------------------------------------------------------------------------

``LoginView`` and ``LogoutView`` (and the deprecated function-based equivalents)
protect users from being redirected to non-HTTPS ``next`` URLs when the app
is running over HTTPS.

Miscellaneous
-------------

Expand Down
32 changes: 32 additions & 0 deletions tests/auth_tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,23 @@ def test_security_check(self):
self.assertEqual(response.status_code, 302)
self.assertIn(good_url, response.url, "%s should be allowed" % good_url)

def test_security_check_https(self):
login_url = reverse('login')
non_https_next_url = 'http://testserver/path'
not_secured_url = '%(url)s?%(next)s=%(next_url)s' % {
'url': login_url,
'next': REDIRECT_FIELD_NAME,
'next_url': urlquote(non_https_next_url),
}
post_data = {
'username': 'testclient',
'password': 'password',
}
response = self.client.post(not_secured_url, post_data, secure=True)
self.assertEqual(response.status_code, 302)
self.assertNotEqual(response.url, non_https_next_url)
self.assertEqual(response.url, settings.LOGIN_REDIRECT_URL)

def test_login_form_contains_request(self):
# 15198
self.client.post('/custom_requestauth_login/', {
Expand Down Expand Up @@ -919,6 +936,21 @@ def test_security_check(self):
self.assertIn(good_url, response.url, "%s should be allowed" % good_url)
self.confirm_logged_out()

def test_security_check_https(self):
logout_url = reverse('logout')
non_https_next_url = 'http://testserver/'
url = '%(url)s?%(next)s=%(next_url)s' % {
'url': logout_url,
'next': REDIRECT_FIELD_NAME,
'next_url': urlquote(non_https_next_url),
}
self.login()
response = self.client.get(url, secure=True)
self.assertEqual(response.status_code, 302)
self.assertNotEqual(response.url, non_https_next_url)
self.assertEqual(response.url, logout_url)
self.confirm_logged_out()

def test_logout_preserve_language(self):
"""Check that language stored in session is preserved after logout"""
# Create a new session with language
Expand Down

0 comments on commit 549b90f

Please sign in to comment.