From df90e462d91d3a77aa89b69d791bf17c2bf7ff9b Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Mon, 12 Mar 2018 15:48:46 +0000 Subject: [PATCH] Fixed #29212 -- Doc'd redirect loop if @permission_required used with redirect_authenticated_user. --- docs/topics/auth/default.txt | 8 ++++++++ tests/auth_tests/test_views.py | 28 ++++++++++++++++++++++++++++ tests/auth_tests/urls.py | 24 ++++++++++++++++++++++-- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 3d9317f8f60..9f864c64d71 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -712,6 +712,10 @@ The ``permission_required`` decorator def my_view(request): ... + This also avoids a redirect loop when :class:`.LoginView`'s + ``redirect_authenticated_user=True`` and the logged-in user doesn't have + all of the required permissions. + .. currentmodule:: django.contrib.auth.mixins The ``PermissionRequiredMixin`` mixin @@ -981,6 +985,10 @@ implementation details see :ref:`using-the-views`. `_" information leakage, host all images and your favicon on a separate domain. + Enabling ``redirect_authenticated_user`` can also result in a redirect + loop when using the :func:`.permission_required` decorator + unless the ``raise_exception`` parameter is used. + * ``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`. diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index bb5bd7a0870..723fd1142b6 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -26,6 +26,7 @@ from django.db import connection from django.http import HttpRequest, QueryDict from django.middleware.csrf import CsrfViewMiddleware, get_token from django.test import Client, TestCase, override_settings +from django.test.client import RedirectCycleError from django.test.utils import patch_logger from django.urls import NoReverseMatch, reverse, reverse_lazy from django.utils.http import urlsafe_base64_encode @@ -883,6 +884,33 @@ class LoginRedirectAuthenticatedUser(AuthViewsTestCase): with self.assertRaisesMessage(ValueError, msg): self.client.get(url) + def test_permission_required_not_logged_in(self): + # Not logged in ... + with self.settings(LOGIN_URL=self.do_redirect_url): + # redirected to login. + response = self.client.get('/permission_required_redirect/', follow=True) + self.assertEqual(response.status_code, 200) + # exception raised. + response = self.client.get('/permission_required_exception/', follow=True) + self.assertEqual(response.status_code, 403) + # redirected to login. + response = self.client.get('/login_and_permission_required_exception/', follow=True) + self.assertEqual(response.status_code, 200) + + def test_permission_required_logged_in(self): + self.login() + # Already logged in... + with self.settings(LOGIN_URL=self.do_redirect_url): + # redirect loop encountered. + with self.assertRaisesMessage(RedirectCycleError, 'Redirect loop detected.'): + self.client.get('/permission_required_redirect/', follow=True) + # exception raised. + response = self.client.get('/permission_required_exception/', follow=True) + self.assertEqual(response.status_code, 403) + # exception raised. + response = self.client.get('/login_and_permission_required_exception/', follow=True) + self.assertEqual(response.status_code, 403) + class LoginSuccessURLAllowedHostsTest(AuthViewsTestCase): def test_success_url_allowed_hosts_same_host(self): diff --git a/tests/auth_tests/urls.py b/tests/auth_tests/urls.py index b69c554bc87..9dc23cee887 100644 --- a/tests/auth_tests/urls.py +++ b/tests/auth_tests/urls.py @@ -1,14 +1,14 @@ from django.conf.urls import url from django.contrib import admin from django.contrib.auth import views -from django.contrib.auth.decorators import login_required +from django.contrib.auth.decorators import login_required, permission_required from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.urls import urlpatterns as auth_urlpatterns from django.contrib.messages.api import info from django.http import HttpRequest, HttpResponse from django.shortcuts import render from django.template import RequestContext, Template -from django.urls import reverse_lazy +from django.urls import path, reverse_lazy from django.views.decorators.cache import never_cache @@ -62,6 +62,22 @@ def userpage(request): pass +@permission_required('unknown.permission') +def permission_required_redirect(request): + pass + + +@permission_required('unknown.permission', raise_exception=True) +def permission_required_exception(request): + pass + + +@login_required +@permission_required('unknown.permission', raise_exception=True) +def login_and_permission_required_exception(request): + pass + + uid_token = r'(?P[0-9A-Za-z_\-]+)/(?P[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,20})' # special urls for auth test cases @@ -119,6 +135,10 @@ urlpatterns = auth_urlpatterns + [ url(r'^login/allowed_hosts/$', views.LoginView.as_view(success_url_allowed_hosts={'otherserver'})), + path('permission_required_redirect/', permission_required_redirect), + path('permission_required_exception/', permission_required_exception), + path('login_and_permission_required_exception/', login_and_permission_required_exception), + # This line is only required to render the password reset with is_admin=True url(r'^admin/', admin.site.urls), ]