Fix #20447: URL names given to contrib.auth.views are now resolved.

This commit also adds tests for the redirect feature of most auth views.
It also cleans up the tests, most notably using @override_settings instead
of ad-hoc setUp/tearDown methods.

Thanks to caumons for the report.

Conflicts:
	docs/releases/1.6.txt

django/contrib/auth/tests/test_views.py

@@ -1,6 +1,10 @@
 import itertools
 import os
 import re
+try:
+    from urllib.parse import urlparse, ParseResult
+except ImportError:     # Python 2
+    from urlparse import urlparse, ParseResult
 
 from django.conf import global_settings, settings
 from django.contrib.sites.models import Site, RequestSite
@@ -46,15 +50,26 @@ def login(self, password='password'):
             'username': 'testclient',
             'password': password,
             })
-        self.assertEqual(response.status_code, 302)
-        self.assertTrue(response.url.endswith(settings.LOGIN_REDIRECT_URL))
         self.assertTrue(SESSION_KEY in self.client.session)
+        return response
 
     def assertFormError(self, response, error):
         """Assert that error is found in response.context['form'] errors"""
         form_errors = list(itertools.chain(*response.context['form'].errors.values()))
         self.assertIn(force_text(error), form_errors)
 
+    def assertURLEqual(self, url, expected):
+        """
+        Given two URLs, make sure all their components (the ones given by
+        urlparse) are equal, only comparing components that are present in both
+        URLs.
+        """
+        fields = ParseResult._fields
+
+        for attr, x, y in zip(fields, urlparse(url), urlparse(expected)):
+            if x and y and x != y:
+                self.fail("%r != %r (%s doesn't match)" % (url, expected, attr))
+
 
 @skipIfCustomUser
 class AuthViewNamedURLTests(AuthViewsTestCase):
@@ -153,7 +168,6 @@ def test_poisoned_http_host_admin_site(self):
     def _test_confirm_start(self):
         # Start by creating the email
         response = self.client.post('/password_reset/', {'email': 'staffmember@example.com'})
-        self.assertEqual(response.status_code, 302)
         self.assertEqual(len(mail.outbox), 1)
         return self._read_signup_email(mail.outbox[0])
 
@@ -205,8 +219,6 @@ def test_confirm_complete(self):
         url, path = self._test_confirm_start()
         response = self.client.post(path, {'new_password1': 'anewpassword',
                                            'new_password2': 'anewpassword'})
-        # It redirects us to a 'complete' page:
-        self.assertEqual(response.status_code, 302)
         # Check the password has been changed
         u = User.objects.get(email='staffmember@example.com')
         self.assertTrue(u.check_password("anewpassword"))
@@ -221,6 +233,47 @@ def test_confirm_different_passwords(self):
                                            'new_password2': 'x'})
         self.assertFormError(response, SetPasswordForm.error_messages['password_mismatch'])
 
+    def test_reset_redirect_default(self):
+        response = self.client.post('/password_reset/',
+            {'email': 'staffmember@example.com'})
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/password_reset/done/')
+
+    def test_reset_custom_redirect(self):
+        response = self.client.post('/password_reset/custom_redirect/',
+            {'email': 'staffmember@example.com'})
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/custom/')
+
+    def test_reset_custom_redirect_named(self):
+        response = self.client.post('/password_reset/custom_redirect/named/',
+            {'email': 'staffmember@example.com'})
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/password_reset/')
+
+    def test_confirm_redirect_default(self):
+        url, path = self._test_confirm_start()
+        response = self.client.post(path, {'new_password1': 'anewpassword',
+                                           'new_password2': 'anewpassword'})
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/reset/done/')
+
+    def test_confirm_redirect_custom(self):
+        url, path = self._test_confirm_start()
+        path = path.replace('/reset/', '/reset/custom/')
+        response = self.client.post(path, {'new_password1': 'anewpassword',
+                                           'new_password2': 'anewpassword'})
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/custom/')
+
+    def test_confirm_redirect_custom_named(self):
+        url, path = self._test_confirm_start()
+        path = path.replace('/reset/', '/reset/custom/named/')
+        response = self.client.post(path, {'new_password1': 'anewpassword',
+                                           'new_password2': 'anewpassword'})
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/password_reset/')
+
 
 @override_settings(AUTH_USER_MODEL='auth.CustomUser')
 class CustomUserPasswordResetTest(AuthViewsTestCase):
@@ -285,8 +338,6 @@ def test_password_change_succeeds(self):
             'new_password1': 'password1',
             'new_password2': 'password1',
         })
-        self.assertEqual(response.status_code, 302)
-        self.assertTrue(response.url.endswith('/password_change/done/'))
         self.fail_login()
         self.login(password='password1')
 
@@ -298,13 +349,43 @@ def test_password_change_done_succeeds(self):
             'new_password2': 'password1',
         })
         self.assertEqual(response.status_code, 302)
-        self.assertTrue(response.url.endswith('/password_change/done/'))
+        self.assertURLEqual(response.url, '/password_change/done/')
 
+    @override_settings(LOGIN_URL='/login/')
     def test_password_change_done_fails(self):
-        with self.settings(LOGIN_URL='/login/'):
-            response = self.client.get('/password_change/done/')
-            self.assertEqual(response.status_code, 302)
-            self.assertTrue(response.url.endswith('/login/?next=/password_change/done/'))
+        response = self.client.get('/password_change/done/')
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/login/?next=/password_change/done/')
+
+    def test_password_change_redirect_default(self):
+        self.login()
+        response = self.client.post('/password_change/', {
+            'old_password': 'password',
+            'new_password1': 'password1',
+            'new_password2': 'password1',
+        })
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/password_change/done/')
+
+    def test_password_change_redirect_custom(self):
+        self.login()
+        response = self.client.post('/password_change/custom/', {
+            'old_password': 'password',
+            'new_password1': 'password1',
+            'new_password2': 'password1',
+        })
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/custom/')
+
+    def test_password_change_redirect_custom_named(self):
+        self.login()
+        response = self.client.post('/password_change/custom/named/', {
+            'old_password': 'password',
+            'new_password1': 'password1',
+            'new_password2': 'password1',
+        })
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/password_reset/')
 
 
 @skipIfCustomUser
@@ -374,63 +455,68 @@ def test_login_form_contains_request(self):
         # the custom authentication form used by this login asserts
         # that a request is passed to the form successfully.
 
+
 @skipIfCustomUser
 class LoginURLSettings(AuthViewsTestCase):
-
-    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
+    """Tests for settings.LOGIN_URL."""
+    def assertLoginURLEquals(self, url):
         response = self.client.get('/login_required/')
         self.assertEqual(response.status_code, 302)
-        return response.url
+        self.assertURLEqual(response.url, url)
 
+    @override_settings(LOGIN_URL='/login/')
     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('/')))
+        self.assertLoginURLEquals('/login/?next=/login_required/')
+
+    @override_settings(LOGIN_URL='login')
+    def test_named_login_url(self):
+        self.assertLoginURLEquals('/login/?next=/login_required/')
 
+    @override_settings(LOGIN_URL='http://remote.example.com/login')
     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('/')))
+        quoted_next = urlquote('http://testserver/login_required/')
+        expected = 'http://remote.example.com/login?next=%s' % quoted_next
+        self.assertLoginURLEquals(expected)
 
+    @override_settings(LOGIN_URL='https:///login/')
     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('/')))
+        quoted_next = urlquote('http://testserver/login_required/')
+        expected = 'https:///login/?next=%s' % quoted_next
+        self.assertLoginURLEquals(expected)
 
+    @override_settings(LOGIN_URL='/login/?pretty=1')
     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('/'))
+        self.assertLoginURLEquals('/login/?pretty=1&next=/login_required/')
 
+    @override_settings(LOGIN_URL='http://remote.example.com/login/?next=/default/')
     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('/')))
+        quoted_next = urlquote('http://testserver/login_required/')
+        expected = 'http://remote.example.com/login/?next=%s' % quoted_next
+        self.assertLoginURLEquals(expected)
+
+
+@skipIfCustomUser
+class LoginRedirectUrlTest(AuthViewsTestCase):
+    """Tests for settings.LOGIN_REDIRECT_URL."""
+    def assertLoginRedirectURLEqual(self, url):
+        response = self.login()
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, url)
+
+    def test_default(self):
+        self.assertLoginRedirectURLEqual('/accounts/profile/')
+
+    @override_settings(LOGIN_REDIRECT_URL='/custom/')
+    def test_custom(self):
+        self.assertLoginRedirectURLEqual('/custom/')
+
+    @override_settings(LOGIN_REDIRECT_URL='password_reset')
+    def test_named(self):
+        self.assertLoginRedirectURLEqual('/password_reset/')
+
+    @override_settings(LOGIN_REDIRECT_URL='http://remote.example.com/welcome/')
+    def test_remote(self):
+        self.assertLoginRedirectURLEqual('http://remote.example.com/welcome/')
 
 
 @skipIfCustomUser
@@ -457,11 +543,11 @@ def test_logout_with_overridden_redirect_url(self):
         self.login()
         response = self.client.get('/logout/next_page/')
         self.assertEqual(response.status_code, 302)
-        self.assertTrue(response.url.endswith('/somewhere/'))
+        self.assertURLEqual(response.url, '/somewhere/')
 
         response = self.client.get('/logout/next_page/?next=/login/')
         self.assertEqual(response.status_code, 302)
-        self.assertTrue(response.url.endswith('/login/'))
+        self.assertURLEqual(response.url, '/login/')
 
         self.confirm_logged_out()
 
@@ -470,23 +556,31 @@ def test_logout_with_next_page_specified(self):
         self.login()
         response = self.client.get('/logout/next_page/')
         self.assertEqual(response.status_code, 302)
-        self.assertTrue(response.url.endswith('/somewhere/'))
+        self.assertURLEqual(response.url, '/somewhere/')
         self.confirm_logged_out()
 
     def test_logout_with_redirect_argument(self):
         "Logout with query string redirects to specified resource"
         self.login()
         response = self.client.get('/logout/?next=/login/')
         self.assertEqual(response.status_code, 302)
-        self.assertTrue(response.url.endswith('/login/'))
+        self.assertURLEqual(response.url, '/login/')
         self.confirm_logged_out()
 
     def test_logout_with_custom_redirect_argument(self):
         "Logout with custom query string redirects to specified resource"
         self.login()
         response = self.client.get('/logout/custom_query/?follow=/somewhere/')
         self.assertEqual(response.status_code, 302)
-        self.assertTrue(response.url.endswith('/somewhere/'))
+        self.assertURLEqual(response.url, '/somewhere/')
+        self.confirm_logged_out()
+
+    def test_logout_with_named_redirect(self):
+        "Logout resolves names or URLs passed as next_page."
+        self.login()
+        response = self.client.get('/logout/next_page/named/')
+        self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, '/password_reset/')
         self.confirm_logged_out()
 
     def test_security_check(self, password='password'):

django/contrib/auth/tests/urls.py

@@ -62,8 +62,19 @@ def custom_request_auth_login(request):
 urlpatterns = urlpatterns + patterns('',
     (r'^logout/custom_query/$', 'django.contrib.auth.views.logout', dict(redirect_field_name='follow')),
     (r'^logout/next_page/$', 'django.contrib.auth.views.logout', dict(next_page='/somewhere/')),
+    (r'^logout/next_page/named/$', 'django.contrib.auth.views.logout', dict(next_page='password_reset')),
     (r'^remote_user/$', remote_user_auth_view),
     (r'^password_reset_from_email/$', 'django.contrib.auth.views.password_reset', dict(from_email='staffmember@example.com')),
+    (r'^password_reset/custom_redirect/$', 'django.contrib.auth.views.password_reset', dict(post_reset_redirect='/custom/')),
+    (r'^password_reset/custom_redirect/named/$', 'django.contrib.auth.views.password_reset', dict(post_reset_redirect='password_reset')),
+    (r'^reset/custom/(?P<uidb36>[0-9A-Za-z]{1,13})-(?P<token>[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,20})/$',
+        'django.contrib.auth.views.password_reset_confirm',
+        dict(post_reset_redirect='/custom/')),
+    (r'^reset/custom/named/(?P<uidb36>[0-9A-Za-z]{1,13})-(?P<token>[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,20})/$',
+        'django.contrib.auth.views.password_reset_confirm',
+        dict(post_reset_redirect='password_reset')),
+    (r'^password_change/custom/$', 'django.contrib.auth.views.password_change', dict(post_change_redirect='/custom/')),
+    (r'^password_change/custom/named/$', 'django.contrib.auth.views.password_change', dict(post_change_redirect='password_reset')),
     (r'^admin_password_reset/$', 'django.contrib.auth.views.password_reset', dict(is_admin_site=True)),
     (r'^login_required/$', login_required(password_reset)),
     (r'^login_required_login_url/$', login_required(password_reset, login_url='/somewhere/')),

django/contrib/auth/views.py

@@ -72,6 +72,9 @@ def logout(request, next_page=None,
     """
     auth_logout(request)
 
+    if next_page is not None:
+        next_page = resolve_url(next_page)
+
     if redirect_field_name in request.REQUEST:
         next_page = request.REQUEST[redirect_field_name]
         # Security check -- don't allow redirection to a different host.
@@ -140,6 +143,8 @@ def password_reset(request, is_admin_site=False,
                    extra_context=None):
     if post_reset_redirect is None:
         post_reset_redirect = reverse('django.contrib.auth.views.password_reset_done')
+    else:
+        post_reset_redirect = resolve_url(post_reset_redirect)
     if request.method == "POST":
         form = password_reset_form(request.POST)
         if form.is_valid():
@@ -193,6 +198,8 @@ def password_reset_confirm(request, uidb36=None, token=None,
     assert uidb36 is not None and token is not None  # checked by URLconf
     if post_reset_redirect is None:
         post_reset_redirect = reverse('django.contrib.auth.views.password_reset_complete')
+    else:
+        post_reset_redirect = resolve_url(post_reset_redirect)
     try:
         uid_int = base36_to_int(uidb36)
         user = UserModel._default_manager.get(pk=uid_int)
@@ -243,6 +250,8 @@ def password_change(request,
                     current_app=None, extra_context=None):
     if post_change_redirect is None:
         post_change_redirect = reverse('django.contrib.auth.views.password_change_done')
+    else:
+        post_change_redirect = resolve_url(post_change_redirect)
     if request.method == "POST":
         form = password_change_form(user=request.user, data=request.POST)
         if form.is_valid():

docs/releases/1.6.txt

@@ -243,6 +243,12 @@ Minor features
 
 * The reason phrase can be customized in HTTP responses.
 
+* When giving the URL of the next page for :func:`~django.contrib.auth.views.logout`,
+  :func:`~django.contrib.auth.views.password_reset`,
+  :func:`~django.contrib.auth.views.password_reset_confirm`,
+  and :func:`~django.contrib.auth.views.password_change`, you can now pass
+  URL names and they will be resolved.
+
 Backwards incompatible changes in 1.6
 =====================================