Ticket #11025: 11025.diff

django/conf/global_settings.py

@@ -473 +473 @@
 
 LOGIN_REDIRECT_URL = '/accounts/profile/'
 
+LOGIN_URL_NEXT_ARG = 'next'
+
 # The number of days a password reset link is valid for
 PASSWORD_RESET_TIMEOUT_DAYS = 3
 

django/contrib/auth/__init__.py

@@ -1 +1 @@
 import datetime
 from warnings import warn
+from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.utils.importlib import import_module
 
 SESSION_KEY = '_auth_user_id'
 BACKEND_SESSION_KEY = '_auth_user_backend'
-REDIRECT_FIELD_NAME = 'next'
+REDIRECT_FIELD_NAME = settings.LOGIN_URL_NEXT_ARG 
 
 def load_backend(path):
     i = path.rfind('.')
@@ -32 +33 @@
     return cls()
 
 def get_backends():
-    from django.conf import settings
     backends = []
     for backend_path in settings.AUTHENTICATION_BACKENDS:
         backends.append(load_backend(backend_path))

django/contrib/auth/decorators.py

@@ +1 @@
+import urlparse
 try:
-    from functools import update_wrapper, wraps
+    from functools import wraps
 except ImportError:
-    from django.utils.functional import update_wrapper, wraps  # Python 2.4 fallback.
+    from django.utils.functional import wraps  # Python 2.4 fallback.
 
+from django.conf import settings
 from django.contrib.auth import REDIRECT_FIELD_NAME
-from django.http import HttpResponseRedirect
 from django.utils.decorators import available_attrs
-from django.utils.http import urlquote
 
 
 def user_passes_test(test_func, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME):
@@ -15 +15 @@
     redirecting to the log-in page if necessary. The test should be a callable
     that takes the user object and returns True if the user passes.
     """
-    if not login_url:
-        from django.conf import settings
-        login_url = settings.LOGIN_URL
 
     def decorator(view_func):
+        @wraps(view_func, assigned=available_attrs(view_func))
         def _wrapped_view(request, *args, **kwargs):
             if test_func(request.user):
                 return view_func(request, *args, **kwargs)
-            path = urlquote(request.get_full_path())
-            tup = login_url, redirect_field_name, path
-            return HttpResponseRedirect('%s?%s=%s' % tup)
-        return wraps(view_func, assigned=available_attrs(view_func))(_wrapped_view)
+            path = request.build_absolute_uri()
+            # If the login url is the same scheme and net location then just
+            # use the path as the "next" url.
+            login_scheme, login_netloc = urlparse.urlparse(login_url or
+                                                        settings.LOGIN_URL)[:2]
+            current_scheme, current_netloc = urlparse.urlparse(path)[:2]
+            if ((not login_scheme or login_scheme == current_scheme) and
+                (not login_netloc or login_netloc == current_netloc)):
+                path = request.get_full_path()
+            from django.contrib.auth.views import redirect_to_login
+            return redirect_to_login(path, login_url, redirect_field_name)
+        return _wrapped_view
     return decorator
 
 

django/contrib/auth/tests/__init__.py

@@ -6 +6 @@
         import RemoteUserTest, RemoteUserNoCreateTest, RemoteUserCustomTest
 from django.contrib.auth.tests.models import ProfileTestCase
 from django.contrib.auth.tests.tokens import TOKEN_GENERATOR_TESTS
-from django.contrib.auth.tests.views \
-        import PasswordResetTest, ChangePasswordTest, LoginTest, LogoutTest
+from django.contrib.auth.tests.views import PasswordResetTest, \
+    ChangePasswordTest, LoginTest, LogoutTest, LoginURLSettings
 
 # The password for the fixture data users is 'password'
 

django/contrib/auth/tests/decorators.py

@@ -42 +42 @@
         login_required decorator with a login_url set.
         """
         self.testLoginRequired(view_url='/login_required_login_url/',
-            login_url='/somewhere/')
- No newline at end of file
+            login_url='/somewhere/')

django/contrib/auth/tests/views.py

@@ -5 +5 @@
 from django.conf import settings
 from django.contrib.auth import SESSION_KEY, REDIRECT_FIELD_NAME
 from django.contrib.auth.forms import AuthenticationForm
-from django.contrib.sites.models import Site, RequestSite
+from django.contrib.sites.models import Site
 from django.contrib.auth.models import User
 from django.test import TestCase
 from django.core import mail
 from django.core.urlresolvers import reverse
+from django.http import QueryDict
 
 class AuthViewsTestCase(TestCase):
     """
@@ -25 +26 @@
         settings.LANGUAGE_CODE = 'en'
         self.old_TEMPLATE_DIRS = settings.TEMPLATE_DIRS
         settings.TEMPLATE_DIRS = (
-            os.path.join(
-                os.path.dirname(__file__),
-                'templates'
-            )
-        ,)
+            os.path.join(os.path.dirname(__file__), 'templates'),
+        )
+        self.old_LOGIN_URL_NEXT_ARG = settings.LOGIN_URL_NEXT_ARG
+        settings.LOGIN_URL_NEXT_ARG = 'next'
 
     def tearDown(self):
         settings.LANGUAGES = self.old_LANGUAGES
         settings.LANGUAGE_CODE = self.old_LANGUAGE_CODE
         settings.TEMPLATE_DIRS = self.old_TEMPLATE_DIRS
+        settings.LOGIN_URL_NEXT_ARG = self.old_LOGIN_URL_NEXT_ARG
 
     def login(self, password='password'):
         response = self.client.post('/login/', {
@@ -214 +215 @@
                 }
             )
             self.assertEquals(response.status_code, 302)
-            self.assertFalse(bad_url in response['Location'], "%s should be blocked" % bad_url)
-
-        # Now, these URLs have an other URL as a GET parameter and therefore
-        # should be allowed
-        for url_ in ('http://example.com', 'https://example.com',
-                    'ftp://exampel.com',  '//example.com'):
-            safe_url = '%(url)s?%(next)s=/view/?param=%(safe_param)s' % {
+            self.assertFalse(bad_url in response['Location'],
+                             "%s should be blocked" % bad_url)
+
+        # These URLs *should* still pass the security check
+        for good_url in ('/view/?param=http://example.com',
+                         '/view/?param=https://example.com',
+                         '/view?param=ftp://exampel.com',
+                         'view/?param=//example.com',
+                         'https:///',
+                         '//testserver/'):
+            safe_url = '%(url)s?%(next)s=%(good_url)s' % {
                 'url': login_url,
                 'next': REDIRECT_FIELD_NAME,
-                'safe_param': urllib.quote(url_)
+                'good_url': urllib.quote(good_url)
             }
             response = self.client.post(safe_url, {
                     'username': 'testclient',
@@ -231 +236 @@
                 }
             )
             self.assertEquals(response.status_code, 302)
-            self.assertTrue('/view/?param=%s' % url_ in response['Location'], "/view/?param=%s should be allowed" % url_)
+            self.assertTrue(good_url in response['Location'],
+                            "%s should be allowed" % good_url)
 
+class LoginURLSettings(AuthViewsTestCase):
+    urls = 'django.contrib.auth.tests.urls'
+    
+    def setUp(self):
+        super(LoginURLSettings, self).setUp()
+        self.old_LOGIN_URL = settings.LOGIN_URL
+
+    def tearDown(self):
+        super(LoginURLSettings, self).tearDown()
+        settings.LOGIN_URL = self.old_LOGIN_URL
+
+    def get_login_required_url(self, login_url):
+        settings.LOGIN_URL = login_url
+        response = self.client.get('/login_required/')
+        self.assertEquals(response.status_code, 302)
+        return response['Location']
+
+    def test_standard_login_url(self):
+        login_url = '/login/'
+        login_required_url = self.get_login_required_url(login_url)
+        querystring = QueryDict('', mutable=True)
+        querystring['next'] = '/login_required/'
+        self.assertEqual(login_required_url,
+             'http://testserver%s?%s' % (login_url, querystring.urlencode()))
+
+    def test_remote_login_url(self):
+        login_url = 'http://remote.example.com/login'
+        login_required_url = self.get_login_required_url(login_url)
+        querystring = QueryDict('', mutable=True)
+        querystring['next'] = 'http://testserver/login_required/'
+        self.assertEqual(login_required_url,
+                         '%s?%s' % (login_url, querystring.urlencode()))
+
+    def test_https_login_url(self):
+        login_url = 'https:///login/'
+        login_required_url = self.get_login_required_url(login_url)
+        querystring = QueryDict('', mutable=True)
+        querystring['next'] = 'http://testserver/login_required/'
+        self.assertEqual(login_required_url,
+                         '%s?%s' % (login_url, querystring.urlencode()))
+
+    def test_login_url_with_querystring(self):
+        login_url = '/login/?pretty=1'
+        login_required_url = self.get_login_required_url(login_url)
+        querystring = QueryDict('pretty=1', mutable=True)
+        querystring['next'] = '/login_required/'
+        self.assertEqual(login_required_url, 'http://testserver/login/?%s' %
+                         querystring.urlencode())
+
+    def test_remote_login_url_with_next_querystring(self):
+        login_url = 'http://remote.example.com/login/'
+        login_required_url = self.get_login_required_url('%s?next=/default/' %
+                                                         login_url)
+        querystring = QueryDict('', mutable=True)
+        querystring['next'] = 'http://testserver/login_required/'
+        self.assertEqual(login_required_url, '%s?%s' % (login_url,
+                                                    querystring.urlencode()))
         
 class LogoutTest(AuthViewsTestCase):
     urls = 'django.contrib.auth.tests.urls'

django/contrib/auth/views.py

@@ -1 +1 @@
 import re
+import urlparse
 from django.conf import settings
 from django.contrib.auth import REDIRECT_FIELD_NAME
 # Avoid shadowing the login() view below.
@@ -11 +12 @@
 from django.core.urlresolvers import reverse
 from django.shortcuts import render_to_response, get_object_or_404
 from django.contrib.sites.models import get_current_site
-from django.http import HttpResponseRedirect, Http404
+from django.http import HttpResponseRedirect, Http404, QueryDict
 from django.template import RequestContext
-from django.utils.http import urlquote, base36_to_int
+from django.utils.http import base36_to_int
 from django.utils.translation import ugettext as _
 from django.contrib.auth.models import User
 from django.views.decorators.cache import never_cache
@@ -30 +31 @@
     if request.method == "POST":
         form = authentication_form(data=request.POST)
         if form.is_valid():
+            netloc = urlparse.urlparse(redirect_to)[1]
+
             # Light security check -- make sure redirect_to isn't garbage.
             if not redirect_to or ' ' in redirect_to:
                 redirect_to = settings.LOGIN_REDIRECT_URL
 
-            # Heavier security check -- redirects to http://example.com should
-            # not be allowed, but things like /view/?param=http://example.com
-            # should be allowed. This regex checks if there is a '//' *before* a
-            # question mark.
-            elif '//' in redirect_to and re.match(r'[^\?]*//', redirect_to):
-                    redirect_to = settings.LOGIN_REDIRECT_URL
+            # Heavier security check -- don't allow redirection to a different
+            # host.
+            elif netloc and netloc != request.get_host():
+                redirect_to = settings.LOGIN_REDIRECT_URL
 
             # Okay, security checks complete. Log the user in.
             auth_login(request, form.get_user())
@@ -88 +89 @@
         login_url = settings.LOGIN_URL
     return logout(request, login_url)
 
-def redirect_to_login(next, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME):
+def redirect_to_login(next, login_url=None,
+                      redirect_field_name=REDIRECT_FIELD_NAME):
     "Redirects the user to the login page, passing the given 'next' page"
     if not login_url:
         login_url = settings.LOGIN_URL
-    return HttpResponseRedirect('%s?%s=%s' % (login_url, urlquote(redirect_field_name), urlquote(next)))
+
+    login_url_parts = list(urlparse.urlparse(login_url))
+    if redirect_field_name:
+        querystring = QueryDict(login_url_parts[4], mutable=True)
+        querystring[redirect_field_name] = next
+        login_url_parts[4] = querystring.urlencode()
+
+    return HttpResponseRedirect(urlparse.urlunparse(login_url_parts))
 
 # 4 views for password reset:
 # - password_reset sends the mail

docs/ref/settings.txt

@@ -1067 +1067 @@
 The URL where requests are redirected for login, especially when using the
 :func:`~django.contrib.auth.decorators.login_required` decorator.
 
+.. setting:: LOGIN_URL_NEXT_ARG
+
+LOGIN_URL_NEXT_ARG
+------------------
+
+.. versionadded:: 1.3
+
+Default: ``'next'``
+
+The argument used when building the "next url" querystring for the login URL.
+For example, with the default settings an anonymous user accessing a URL of
+``/secure_page/`` which maps to a view decorated with
+:func:`~django.contrib.auth.decorators.login_required` will be redirected to:
+``/accounts/login/?next=/secure_page/``.
+
 .. setting:: LOGOUT_URL
 
 LOGOUT_URL