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
This commit is contained in:
Baptiste Mispelon 2013-05-19 12:57:06 +02:00 committed by Marc Tamlyn
parent 9b22baddef
commit 980ae2ab29
4 changed files with 180 additions and 60 deletions

View File

@ -1,6 +1,10 @@
import itertools import itertools
import os import os
import re 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.conf import global_settings, settings
from django.contrib.sites.models import Site, RequestSite from django.contrib.sites.models import Site, RequestSite
@ -46,15 +50,26 @@ class AuthViewsTestCase(TestCase):
'username': 'testclient', 'username': 'testclient',
'password': password, '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) self.assertTrue(SESSION_KEY in self.client.session)
return response
def assertFormError(self, response, error): def assertFormError(self, response, error):
"""Assert that error is found in response.context['form'] errors""" """Assert that error is found in response.context['form'] errors"""
form_errors = list(itertools.chain(*response.context['form'].errors.values())) form_errors = list(itertools.chain(*response.context['form'].errors.values()))
self.assertIn(force_text(error), form_errors) 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 @skipIfCustomUser
class AuthViewNamedURLTests(AuthViewsTestCase): class AuthViewNamedURLTests(AuthViewsTestCase):
@ -153,7 +168,6 @@ class PasswordResetTest(AuthViewsTestCase):
def _test_confirm_start(self): def _test_confirm_start(self):
# Start by creating the email # Start by creating the email
response = self.client.post('/password_reset/', {'email': 'staffmember@example.com'}) response = self.client.post('/password_reset/', {'email': 'staffmember@example.com'})
self.assertEqual(response.status_code, 302)
self.assertEqual(len(mail.outbox), 1) self.assertEqual(len(mail.outbox), 1)
return self._read_signup_email(mail.outbox[0]) return self._read_signup_email(mail.outbox[0])
@ -205,8 +219,6 @@ class PasswordResetTest(AuthViewsTestCase):
url, path = self._test_confirm_start() url, path = self._test_confirm_start()
response = self.client.post(path, {'new_password1': 'anewpassword', response = self.client.post(path, {'new_password1': 'anewpassword',
'new_password2': 'anewpassword'}) 'new_password2': 'anewpassword'})
# It redirects us to a 'complete' page:
self.assertEqual(response.status_code, 302)
# Check the password has been changed # Check the password has been changed
u = User.objects.get(email='staffmember@example.com') u = User.objects.get(email='staffmember@example.com')
self.assertTrue(u.check_password("anewpassword")) self.assertTrue(u.check_password("anewpassword"))
@ -221,6 +233,47 @@ class PasswordResetTest(AuthViewsTestCase):
'new_password2': 'x'}) 'new_password2': 'x'})
self.assertFormError(response, SetPasswordForm.error_messages['password_mismatch']) 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') @override_settings(AUTH_USER_MODEL='auth.CustomUser')
class CustomUserPasswordResetTest(AuthViewsTestCase): class CustomUserPasswordResetTest(AuthViewsTestCase):
@ -285,8 +338,6 @@ class ChangePasswordTest(AuthViewsTestCase):
'new_password1': 'password1', 'new_password1': 'password1',
'new_password2': 'password1', 'new_password2': 'password1',
}) })
self.assertEqual(response.status_code, 302)
self.assertTrue(response.url.endswith('/password_change/done/'))
self.fail_login() self.fail_login()
self.login(password='password1') self.login(password='password1')
@ -298,13 +349,43 @@ class ChangePasswordTest(AuthViewsTestCase):
'new_password2': 'password1', 'new_password2': 'password1',
}) })
self.assertEqual(response.status_code, 302) 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): def test_password_change_done_fails(self):
with self.settings(LOGIN_URL='/login/'):
response = self.client.get('/password_change/done/') response = self.client.get('/password_change/done/')
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertTrue(response.url.endswith('/login/?next=/password_change/done/')) 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 @skipIfCustomUser
@ -374,63 +455,68 @@ class LoginTest(AuthViewsTestCase):
# the custom authentication form used by this login asserts # the custom authentication form used by this login asserts
# that a request is passed to the form successfully. # that a request is passed to the form successfully.
@skipIfCustomUser @skipIfCustomUser
class LoginURLSettings(AuthViewsTestCase): class LoginURLSettings(AuthViewsTestCase):
"""Tests for settings.LOGIN_URL."""
def setUp(self): def assertLoginURLEquals(self, url):
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/') response = self.client.get('/login_required/')
self.assertEqual(response.status_code, 302) 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): def test_standard_login_url(self):
login_url = '/login/' self.assertLoginURLEquals('/login/?next=/login_required/')
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('/')))
@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): def test_remote_login_url(self):
login_url = 'http://remote.example.com/login' quoted_next = urlquote('http://testserver/login_required/')
login_required_url = self.get_login_required_url(login_url) expected = 'http://remote.example.com/login?next=%s' % quoted_next
querystring = QueryDict('', mutable=True) self.assertLoginURLEquals(expected)
querystring['next'] = 'http://testserver/login_required/'
self.assertEqual(login_required_url,
'%s?%s' % (login_url, querystring.urlencode('/')))
@override_settings(LOGIN_URL='https:///login/')
def test_https_login_url(self): def test_https_login_url(self):
login_url = 'https:///login/' quoted_next = urlquote('http://testserver/login_required/')
login_required_url = self.get_login_required_url(login_url) expected = 'https:///login/?next=%s' % quoted_next
querystring = QueryDict('', mutable=True) self.assertLoginURLEquals(expected)
querystring['next'] = 'http://testserver/login_required/'
self.assertEqual(login_required_url,
'%s?%s' % (login_url, querystring.urlencode('/')))
@override_settings(LOGIN_URL='/login/?pretty=1')
def test_login_url_with_querystring(self): def test_login_url_with_querystring(self):
login_url = '/login/?pretty=1' self.assertLoginURLEquals('/login/?pretty=1&next=/login_required/')
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('/'))
@override_settings(LOGIN_URL='http://remote.example.com/login/?next=/default/')
def test_remote_login_url_with_next_querystring(self): def test_remote_login_url_with_next_querystring(self):
login_url = 'http://remote.example.com/login/' quoted_next = urlquote('http://testserver/login_required/')
login_required_url = self.get_login_required_url('%s?next=/default/' % expected = 'http://remote.example.com/login/?next=%s' % quoted_next
login_url) self.assertLoginURLEquals(expected)
querystring = QueryDict('', mutable=True)
querystring['next'] = 'http://testserver/login_required/'
self.assertEqual(login_required_url, '%s?%s' % (login_url, @skipIfCustomUser
querystring.urlencode('/'))) 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 @skipIfCustomUser
@ -457,11 +543,11 @@ class LogoutTest(AuthViewsTestCase):
self.login() self.login()
response = self.client.get('/logout/next_page/') response = self.client.get('/logout/next_page/')
self.assertEqual(response.status_code, 302) 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/') response = self.client.get('/logout/next_page/?next=/login/')
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertTrue(response.url.endswith('/login/')) self.assertURLEqual(response.url, '/login/')
self.confirm_logged_out() self.confirm_logged_out()
@ -470,7 +556,7 @@ class LogoutTest(AuthViewsTestCase):
self.login() self.login()
response = self.client.get('/logout/next_page/') response = self.client.get('/logout/next_page/')
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertTrue(response.url.endswith('/somewhere/')) self.assertURLEqual(response.url, '/somewhere/')
self.confirm_logged_out() self.confirm_logged_out()
def test_logout_with_redirect_argument(self): def test_logout_with_redirect_argument(self):
@ -478,7 +564,7 @@ class LogoutTest(AuthViewsTestCase):
self.login() self.login()
response = self.client.get('/logout/?next=/login/') response = self.client.get('/logout/?next=/login/')
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertTrue(response.url.endswith('/login/')) self.assertURLEqual(response.url, '/login/')
self.confirm_logged_out() self.confirm_logged_out()
def test_logout_with_custom_redirect_argument(self): def test_logout_with_custom_redirect_argument(self):
@ -486,7 +572,15 @@ class LogoutTest(AuthViewsTestCase):
self.login() self.login()
response = self.client.get('/logout/custom_query/?follow=/somewhere/') response = self.client.get('/logout/custom_query/?follow=/somewhere/')
self.assertEqual(response.status_code, 302) 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() self.confirm_logged_out()
def test_security_check(self, password='password'): def test_security_check(self, password='password'):

View File

@ -62,8 +62,19 @@ def custom_request_auth_login(request):
urlpatterns = urlpatterns + patterns('', urlpatterns = urlpatterns + patterns('',
(r'^logout/custom_query/$', 'django.contrib.auth.views.logout', dict(redirect_field_name='follow')), (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/$', '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'^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_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'^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_required(password_reset)),
(r'^login_required_login_url/$', login_required(password_reset, login_url='/somewhere/')), (r'^login_required_login_url/$', login_required(password_reset, login_url='/somewhere/')),

View File

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

View File

@ -243,6 +243,12 @@ Minor features
* The reason phrase can be customized in HTTP responses. * 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 Backwards incompatible changes in 1.6
===================================== =====================================