From 66e1ebbffc2742deb9b2051c6de89c0ac58fcc89 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Tue, 26 Jul 2016 16:10:08 -0700 Subject: [PATCH] Fixed #26956 -- Added success_url_allowed_hosts to LoginView and LogoutView. Allows specifying additional hosts to redirect after login and log out. --- django/contrib/auth/views.py | 20 +++++++++---- docs/releases/1.11.txt | 5 ++++ docs/topics/auth/default.txt | 8 +++++ tests/auth_tests/test_views.py | 53 ++++++++++++++++++++++++++++++++++ tests/auth_tests/urls.py | 3 ++ 5 files changed, 84 insertions(+), 5 deletions(-) diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index 8912c87aee6..b84fae88599 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -53,7 +53,16 @@ def deprecate_current_app(func): return inner -class LoginView(FormView): +class SuccessURLAllowedHostsMixin(object): + success_url_allowed_hosts = set() + + def get_success_url_allowed_hosts(self): + allowed_hosts = {self.request.get_host()} + allowed_hosts.update(self.success_url_allowed_hosts) + return allowed_hosts + + +class LoginView(SuccessURLAllowedHostsMixin, FormView): """ Displays the login form and handles the login action. """ @@ -86,7 +95,7 @@ class LoginView(FormView): ) url_is_safe = is_safe_url( url=redirect_to, - allowed_hosts={self.request.get_host()}, + allowed_hosts=self.get_success_url_allowed_hosts(), require_https=self.request.is_secure(), ) if not url_is_safe: @@ -123,7 +132,7 @@ def login(request, *args, **kwargs): return LoginView.as_view(**kwargs)(request, *args, **kwargs) -class LogoutView(TemplateView): +class LogoutView(SuccessURLAllowedHostsMixin, TemplateView): """ Logs out the user and displays 'You are logged out' message. """ @@ -157,10 +166,11 @@ class LogoutView(TemplateView): ) url_is_safe = is_safe_url( url=next_page, - allowed_hosts={self.request.get_host()}, + allowed_hosts=self.get_success_url_allowed_hosts(), require_https=self.request.is_secure(), ) - # Security check -- don't allow redirection to a different host. + # Security check -- Ensure the user-originating redirection URL is + # safe. 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 a202232cb66..0c1e387b662 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -101,6 +101,11 @@ Minor features * :func:`~django.contrib.auth.update_session_auth_hash` now rotates the session key to allow a password change to invalidate stolen session cookies. +* The new ``success_url_allowed_hosts`` attribute for + :class:`~django.contrib.auth.views.LoginView` and + :class:`~django.contrib.auth.views.LogoutView` allows specifying a set of + hosts that are safe for redirecting after login and logout. + :mod:`django.contrib.contenttypes` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index f8a1a50169a..a484a89d6df 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -999,6 +999,10 @@ implementation details see :ref:`using-the-views`. authenticated users accessing the login page will be redirected as if they had just successfully logged in. Defaults to ``False``. + * ``success_url_allowed_hosts``: A :class:`set` of hosts, in addition to + :meth:`request.get_host() `, that are + safe for redirecting after login. Defaults to an empty :class:`set`. + Here's what ``LoginView`` does: * If called via ``GET``, it displays a login form that POSTs to the @@ -1138,6 +1142,10 @@ implementation details see :ref:`using-the-views`. * ``extra_context``: A dictionary of context data that will be added to the default context data passed to the template. + * ``success_url_allowed_hosts``: A :class:`set` of hosts, in addition to + :meth:`request.get_host() `, that are + safe for redirecting after logout. Defaults to an empty :class:`set`. + **Template context:** * ``title``: The string "Logged out", localized. diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index 3b60643a737..122fc7ea99c 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -822,6 +822,38 @@ class LoginRedirectAuthenticatedUser(AuthViewsTestCase): self.client.get(url) +class LoginSuccessURLAllowedHostsTest(AuthViewsTestCase): + def test_success_url_allowed_hosts_same_host(self): + response = self.client.post('/login/allowed_hosts/', { + 'username': 'testclient', + 'password': 'password', + 'next': 'https://testserver/home', + }) + self.assertIn(SESSION_KEY, self.client.session) + self.assertEqual(response.status_code, 302) + self.assertURLEqual(response.url, 'https://testserver/home') + + def test_success_url_allowed_hosts_safe_host(self): + response = self.client.post('/login/allowed_hosts/', { + 'username': 'testclient', + 'password': 'password', + 'next': 'https://otherserver/home', + }) + self.assertIn(SESSION_KEY, self.client.session) + self.assertEqual(response.status_code, 302) + self.assertURLEqual(response.url, 'https://otherserver/home') + + def test_success_url_allowed_hosts_unsafe_host(self): + response = self.client.post('/login/allowed_hosts/', { + 'username': 'testclient', + 'password': 'password', + 'next': 'https://evil/home', + }) + self.assertIn(SESSION_KEY, self.client.session) + self.assertEqual(response.status_code, 302) + self.assertURLEqual(response.url, '/accounts/profile/') + + class LogoutTest(AuthViewsTestCase): def confirm_logged_out(self): @@ -893,6 +925,27 @@ class LogoutTest(AuthViewsTestCase): self.assertURLEqual(response.url, '/password_reset/') self.confirm_logged_out() + def test_success_url_allowed_hosts_same_host(self): + self.login() + response = self.client.get('/logout/allowed_hosts/?next=https://testserver/') + self.assertEqual(response.status_code, 302) + self.assertURLEqual(response.url, 'https://testserver/') + self.confirm_logged_out() + + def test_success_url_allowed_hosts_safe_host(self): + self.login() + response = self.client.get('/logout/allowed_hosts/?next=https://otherserver/') + self.assertEqual(response.status_code, 302) + self.assertURLEqual(response.url, 'https://otherserver/') + self.confirm_logged_out() + + def test_success_url_allowed_hosts_unsafe_host(self): + self.login() + response = self.client.get('/logout/allowed_hosts/?next=https://evil/') + self.assertEqual(response.status_code, 302) + self.assertURLEqual(response.url, '/logout/allowed_hosts/') + self.confirm_logged_out() + def test_security_check(self): logout_url = reverse('logout') diff --git a/tests/auth_tests/urls.py b/tests/auth_tests/urls.py index e8174fb5364..35bdbbb3a87 100644 --- a/tests/auth_tests/urls.py +++ b/tests/auth_tests/urls.py @@ -67,6 +67,7 @@ urlpatterns = auth_urlpatterns + [ url(r'^logout/custom_query/$', views.LogoutView.as_view(redirect_field_name='follow')), url(r'^logout/next_page/$', views.LogoutView.as_view(next_page='/somewhere/')), url(r'^logout/next_page/named/$', views.LogoutView.as_view(next_page='password_reset')), + url(r'^logout/allowed_hosts/$', views.LogoutView.as_view(success_url_allowed_hosts={'otherserver'})), url(r'^remote_user/$', remote_user_auth_view), url(r'^password_reset_from_email/$', @@ -106,6 +107,8 @@ urlpatterns = auth_urlpatterns + [ url(r'^login/redirect_authenticated_user_default/$', views.LoginView.as_view()), url(r'^login/redirect_authenticated_user/$', views.LoginView.as_view(redirect_authenticated_user=True)), + url(r'^login/allowed_hosts/$', + views.LoginView.as_view(success_url_allowed_hosts={'otherserver'})), # This line is only required to render the password reset with is_admin=True url(r'^admin/', admin.site.urls),