Browse Source

Fixed #29212 -- Doc'd redirect loop if @permission_required used with redirect_authenticated_user.

Nick Pope 7 years ago
parent
commit
df90e462d9
3 changed files with 58 additions and 2 deletions
  1. 8 0
      docs/topics/auth/default.txt
  2. 28 0
      tests/auth_tests/test_views.py
  3. 22 2
      tests/auth_tests/urls.py

+ 8 - 0
docs/topics/auth/default.txt

@@ -712,6 +712,10 @@ The ``permission_required`` decorator
         def my_view(request):
         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
 .. currentmodule:: django.contrib.auth.mixins
 
 
 The ``PermissionRequiredMixin`` mixin
 The ``PermissionRequiredMixin`` mixin
@@ -981,6 +985,10 @@ implementation details see :ref:`using-the-views`.
         <https://robinlinus.github.io/socialmedia-leak/>`_" information
         <https://robinlinus.github.io/socialmedia-leak/>`_" information
         leakage, host all images and your favicon on a separate domain.
         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
     * ``success_url_allowed_hosts``: A :class:`set` of hosts, in addition to
       :meth:`request.get_host() <django.http.HttpRequest.get_host>`, that are
       :meth:`request.get_host() <django.http.HttpRequest.get_host>`, that are
       safe for redirecting after login. Defaults to an empty :class:`set`.
       safe for redirecting after login. Defaults to an empty :class:`set`.

+ 28 - 0
tests/auth_tests/test_views.py

@@ -26,6 +26,7 @@ from django.db import connection
 from django.http import HttpRequest, QueryDict
 from django.http import HttpRequest, QueryDict
 from django.middleware.csrf import CsrfViewMiddleware, get_token
 from django.middleware.csrf import CsrfViewMiddleware, get_token
 from django.test import Client, TestCase, override_settings
 from django.test import Client, TestCase, override_settings
+from django.test.client import RedirectCycleError
 from django.test.utils import patch_logger
 from django.test.utils import patch_logger
 from django.urls import NoReverseMatch, reverse, reverse_lazy
 from django.urls import NoReverseMatch, reverse, reverse_lazy
 from django.utils.http import urlsafe_base64_encode
 from django.utils.http import urlsafe_base64_encode
@@ -883,6 +884,33 @@ class LoginRedirectAuthenticatedUser(AuthViewsTestCase):
             with self.assertRaisesMessage(ValueError, msg):
             with self.assertRaisesMessage(ValueError, msg):
                 self.client.get(url)
                 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):
 class LoginSuccessURLAllowedHostsTest(AuthViewsTestCase):
     def test_success_url_allowed_hosts_same_host(self):
     def test_success_url_allowed_hosts_same_host(self):

+ 22 - 2
tests/auth_tests/urls.py

@@ -1,14 +1,14 @@
 from django.conf.urls import url
 from django.conf.urls import url
 from django.contrib import admin
 from django.contrib import admin
 from django.contrib.auth import views
 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.forms import AuthenticationForm
 from django.contrib.auth.urls import urlpatterns as auth_urlpatterns
 from django.contrib.auth.urls import urlpatterns as auth_urlpatterns
 from django.contrib.messages.api import info
 from django.contrib.messages.api import info
 from django.http import HttpRequest, HttpResponse
 from django.http import HttpRequest, HttpResponse
 from django.shortcuts import render
 from django.shortcuts import render
 from django.template import RequestContext, Template
 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
 from django.views.decorators.cache import never_cache
 
 
 
 
@@ -62,6 +62,22 @@ def userpage(request):
     pass
     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<uidb64>[0-9A-Za-z_\-]+)/(?P<token>[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,20})'
 uid_token = r'(?P<uidb64>[0-9A-Za-z_\-]+)/(?P<token>[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,20})'
 
 
 # special urls for auth test cases
 # special urls for auth test cases
@@ -119,6 +135,10 @@ urlpatterns = auth_urlpatterns + [
     url(r'^login/allowed_hosts/$',
     url(r'^login/allowed_hosts/$',
         views.LoginView.as_view(success_url_allowed_hosts={'otherserver'})),
         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
     # This line is only required to render the password reset with is_admin=True
     url(r'^admin/', admin.site.urls),
     url(r'^admin/', admin.site.urls),
 ]
 ]