Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Closed
wants to merge 1 commit into from

2 participants

@bmispelon
Collaborator

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.

@bmispelon bmispelon 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.
38845c9
@mjtamlyn
Collaborator

Fixed in 5a318d2f02fcf03f3d5485b284dac579d5e34e02. Thanks!

@mjtamlyn mjtamlyn closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 19, 2013
  1. @bmispelon

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

    bmispelon authored
    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.
This page is out of date. Refresh to see the latest.
View
214 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,7 +556,7 @@ 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):
@@ -478,7 +564,7 @@ def test_logout_with_redirect_argument(self):
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):
@@ -486,7 +572,15 @@ def test_logout_with_custom_redirect_argument(self):
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'):
View
11 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/')),
View
9 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():
View
6 docs/releases/1.6.txt
@@ -241,6 +241,12 @@ Minor features
* The ``choices`` argument to model fields now accepts an iterable of iterables
instead of requiring an iterable of lists or tuples.
+* 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
=====================================
Something went wrong with that request. Please try again.