From 549b90fab33c80d1ba6575d4837ea52d79f70fb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Suliga?= Date: Fri, 19 Aug 2016 13:40:21 +0200 Subject: [PATCH] Refs #26902 -- Protected against insecure redirects in Login/LogoutView. --- django/contrib/auth/views.py | 14 ++++++++++++-- docs/releases/1.11.txt | 7 +++++++ tests/auth_tests/test_views.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index fd5508f3bf..cf256d1639 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -84,7 +84,12 @@ class LoginView(FormView): 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 @@ -150,8 +155,13 @@ class LogoutView(TemplateView): 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 diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index d62ea80e8c..64afe7a841 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -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 ------------- diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index e1371f5a35..3b60643a73 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -551,6 +551,23 @@ class LoginTest(AuthViewsTestCase): 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/', { @@ -919,6 +936,21 @@ class LogoutTest(AuthViewsTestCase): 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