From 10781b4c6ff981f581157957d221e7621e0bf4ed Mon Sep 17 00:00:00 2001 From: Olivier Le Thanh Duong Date: Thu, 12 Nov 2015 00:48:16 +0100 Subject: [PATCH] Fixed #12233 -- Allowed redirecting authenticated users away from the login view. contrib.auth.views.login() has a new parameter `redirect_authenticated_user` to automatically redirect authenticated users visiting the login page. Thanks to dmathieu and Alex Buchanan for the original code and to Carl Meyer for the help and review. --- django/contrib/auth/views.py | 31 +++++++++++-------- docs/releases/1.10.txt | 4 +++ docs/topics/auth/default.txt | 10 ++++++- tests/auth_tests/test_views.py | 54 ++++++++++++++++++++++++++++++++++ tests/auth_tests/urls.py | 2 ++ 5 files changed, 88 insertions(+), 13 deletions(-) diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index e2387fea89..ff6643b035 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -48,6 +48,13 @@ def deprecate_current_app(func): return inner +def _get_login_redirect_url(request, redirect_to): + # Ensure the user-originating redirection URL is safe. + if not is_safe_url(url=redirect_to, host=request.get_host()): + return resolve_url(settings.LOGIN_REDIRECT_URL) + return redirect_to + + @deprecate_current_app @sensitive_post_parameters() @csrf_protect @@ -55,25 +62,25 @@ def deprecate_current_app(func): def login(request, template_name='registration/login.html', redirect_field_name=REDIRECT_FIELD_NAME, authentication_form=AuthenticationForm, - extra_context=None): + extra_context=None, redirect_authenticated_user=False): """ Displays the login form and handles the login action. """ - redirect_to = request.POST.get(redirect_field_name, - request.GET.get(redirect_field_name, '')) + redirect_to = request.POST.get(redirect_field_name, request.GET.get(redirect_field_name, '')) - if request.method == "POST": + if redirect_authenticated_user and request.user.is_authenticated(): + redirect_to = _get_login_redirect_url(request, redirect_to) + if redirect_to == request.path: + raise ValueError( + "Redirection loop for authenticated user detected. Check that " + "your LOGIN_REDIRECT_URL doesn't point to a login page." + ) + return HttpResponseRedirect(redirect_to) + elif request.method == "POST": form = authentication_form(request, data=request.POST) if form.is_valid(): - - # Ensure the user-originating redirection url is safe. - if not is_safe_url(url=redirect_to, host=request.get_host()): - redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL) - - # Okay, security check complete. Log the user in. auth_login(request, form.get_user()) - - return HttpResponseRedirect(redirect_to) + return HttpResponseRedirect(_get_login_redirect_url(request, redirect_to)) else: form = authentication_form(request) diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt index a5e68759da..7ea4a5e4d5 100644 --- a/docs/releases/1.10.txt +++ b/docs/releases/1.10.txt @@ -86,6 +86,10 @@ Minor features :func:`~django.contrib.auth.views.logout` view, if the view doesn't get a ``next_page`` argument. +* The new ``redirect_authenticated_user`` parameter for the + :func:`~django.contrib.auth.views.login` view allows redirecting + authenticated users visiting the login page. + :mod:`django.contrib.contenttypes` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 0d571a743c..64fcce49af 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -965,7 +965,7 @@ All authentication views This is a list with all the views ``django.contrib.auth`` provides. For implementation details see :ref:`using-the-views`. -.. function:: login(request, template_name=`registration/login.html`, redirect_field_name='next', authentication_form=AuthenticationForm, current_app=None, extra_context=None) +.. function:: login(request, template_name=`registration/login.html`, redirect_field_name='next', authentication_form=AuthenticationForm, current_app=None, extra_context=None, redirect_authenticated_user=False) **URL name:** ``login`` @@ -991,11 +991,19 @@ 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. + * ``redirect_authenticated_user``: A boolean that controls whether or not + authenticated users accessing the login page will be redirected as if + they had just successfully logged in. Defaults to ``False``. + .. deprecated:: 1.9 The ``current_app`` parameter is deprecated and will be removed in Django 2.0. Callers should set ``request.current_app`` instead. + .. versionadded:: 1.10 + + The ``redirect_authenticated_user`` parameter was added. + Here's what ``django.contrib.auth.views.login`` does: * If called via ``GET``, it displays a login form that POSTs to the diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index d8325d8b93..2df2c75122 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -715,6 +715,60 @@ class RedirectToLoginTests(AuthViewsTestCase): self.assertEqual(expected, login_redirect_response.url) +class LoginRedirectAuthenticatedUser(AuthViewsTestCase): + dont_redirect_url = '/login/redirect_authenticated_user_default/' + do_redirect_url = '/login/redirect_authenticated_user/' + + def test_default(self): + """Stay on the login page by default.""" + self.login() + response = self.client.get(self.dont_redirect_url) + self.assertEqual(response.status_code, 200) + + def test_guest(self): + """If not logged in, stay on the same page.""" + response = self.client.get(self.do_redirect_url) + self.assertEqual(response.status_code, 200) + + def test_redirect(self): + """If logged in, go to default redirected URL.""" + self.login() + response = self.client.get(self.do_redirect_url) + self.assertRedirects(response, '/accounts/profile/', fetch_redirect_response=False) + + @override_settings(LOGIN_REDIRECT_URL='/custom/') + def test_redirect_url(self): + """If logged in, go to custom redirected URL.""" + self.login() + response = self.client.get(self.do_redirect_url) + self.assertRedirects(response, '/custom/', fetch_redirect_response=False) + + def test_redirect_param(self): + """If next is specified as a GET parameter, go there.""" + self.login() + url = self.do_redirect_url + '?next=/custom_next/' + response = self.client.get(url) + self.assertRedirects(response, '/custom_next/', fetch_redirect_response=False) + + def test_redirect_loop(self): + """ + Detect a redirect loop if LOGIN_REDIRECT_URL is not correctly set, + with and without custom parameters. + """ + self.login() + msg = ( + "Redirection loop for authenticated user detected. Check that " + "your LOGIN_REDIRECT_URL doesn't point to a login page" + ) + with self.settings(LOGIN_REDIRECT_URL=self.do_redirect_url): + with self.assertRaisesMessage(ValueError, msg): + self.client.get(self.do_redirect_url) + + url = self.do_redirect_url + '?bla=2' + with self.assertRaisesMessage(ValueError, msg): + self.client.get(url) + + class LogoutTest(AuthViewsTestCase): def confirm_logged_out(self): diff --git a/tests/auth_tests/urls.py b/tests/auth_tests/urls.py index bc68c3f216..2b70c59a51 100644 --- a/tests/auth_tests/urls.py +++ b/tests/auth_tests/urls.py @@ -96,6 +96,8 @@ urlpatterns = auth_urlpatterns + [ url(r'^auth_processor_messages/$', auth_processor_messages), url(r'^custom_request_auth_login/$', custom_request_auth_login), url(r'^userpage/(.+)/$', userpage, name="userpage"), + url(r'^login/redirect_authenticated_user_default/$', views.login), + url(r'^login/redirect_authenticated_user/$', views.login, dict(redirect_authenticated_user=True)), # This line is only required to render the password reset with is_admin=True url(r'^admin/', admin.site.urls),