From a78dd109e6c81c49e90e36e9b793bad67c46c23c Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Sat, 8 Sep 2012 16:55:29 -0600 Subject: [PATCH] Fixed #15552 -- LOGIN_URL and LOGIN_REDIRECT_URL can take URLpattern names. Thanks UloPe and Eric Florenzano for the patch, and Malcolm Tredinnick for review. --- django/contrib/auth/decorators.py | 6 +- django/contrib/auth/tests/decorators.py | 2 +- django/contrib/auth/views.py | 18 ++--- django/shortcuts/__init__.py | 49 ++++++++----- docs/ref/settings.txt | 33 ++++----- docs/releases/1.5.txt | 6 ++ docs/topics/auth.txt | 7 ++ .../comment_tests/tests/__init__.py | 2 +- .../comment_tests/urls_default.py | 9 +++ tests/regressiontests/resolve_url/__init__.py | 0 tests/regressiontests/resolve_url/models.py | 12 ++++ tests/regressiontests/resolve_url/tests.py | 68 +++++++++++++++++++ tests/runtests.py | 2 +- 13 files changed, 162 insertions(+), 52 deletions(-) create mode 100644 tests/regressiontests/comment_tests/urls_default.py create mode 100644 tests/regressiontests/resolve_url/__init__.py create mode 100644 tests/regressiontests/resolve_url/models.py create mode 100644 tests/regressiontests/resolve_url/tests.py diff --git a/django/contrib/auth/decorators.py b/django/contrib/auth/decorators.py index beeb284998..0fc9f3754a 100644 --- a/django/contrib/auth/decorators.py +++ b/django/contrib/auth/decorators.py @@ -8,6 +8,7 @@ from django.contrib.auth import REDIRECT_FIELD_NAME from django.core.exceptions import PermissionDenied from django.utils.decorators import available_attrs from django.utils.encoding import force_str +from django.shortcuts import resolve_url def user_passes_test(test_func, login_url=None, redirect_field_name=REDIRECT_FIELD_NAME): @@ -23,11 +24,10 @@ def user_passes_test(test_func, login_url=None, redirect_field_name=REDIRECT_FIE if test_func(request.user): return view_func(request, *args, **kwargs) path = request.build_absolute_uri() - # urlparse chokes on lazy objects in Python 3 - login_url_as_str = force_str(login_url or settings.LOGIN_URL) + resolved_login_url = resolve_url(login_url or settings.LOGIN_URL) # 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(login_url_as_str)[:2] + login_scheme, login_netloc = urlparse(resolved_login_url)[:2] current_scheme, current_netloc = urlparse(path)[:2] if ((not login_scheme or login_scheme == current_scheme) and (not login_netloc or login_netloc == current_netloc)): diff --git a/django/contrib/auth/tests/decorators.py b/django/contrib/auth/tests/decorators.py index bd3f0115f5..cefc310e40 100644 --- a/django/contrib/auth/tests/decorators.py +++ b/django/contrib/auth/tests/decorators.py @@ -25,7 +25,7 @@ class LoginRequiredTestCase(AuthViewsTestCase): pass login_required(normal_view) - def testLoginRequired(self, view_url='/login_required/', login_url=settings.LOGIN_URL): + def testLoginRequired(self, view_url='/login_required/', login_url='/login/'): """ Check that login_required works on a simple view wrapped in a login_required decorator. diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index f93541b4bf..024be5e46d 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -7,9 +7,9 @@ from django.conf import settings from django.core.urlresolvers import reverse from django.http import HttpResponseRedirect, QueryDict from django.template.response import TemplateResponse -from django.utils.encoding import force_str from django.utils.http import base36_to_int from django.utils.translation import ugettext as _ +from django.shortcuts import resolve_url from django.views.decorators.debug import sensitive_post_parameters from django.views.decorators.cache import never_cache from django.views.decorators.csrf import csrf_protect @@ -38,16 +38,16 @@ def login(request, template_name='registration/login.html', if request.method == "POST": form = authentication_form(data=request.POST) if form.is_valid(): - netloc = urlparse(redirect_to)[1] - # Use default setting if redirect_to is empty if not redirect_to: redirect_to = settings.LOGIN_REDIRECT_URL + redirect_to = resolve_url(redirect_to) + netloc = urlparse(redirect_to)[1] # Heavier security check -- don't allow redirection to a different # host. - elif netloc and netloc != request.get_host(): - redirect_to = settings.LOGIN_REDIRECT_URL + if netloc and netloc != request.get_host(): + redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL) # Okay, security checks complete. Log the user in. auth_login(request, form.get_user()) @@ -110,6 +110,7 @@ def logout_then_login(request, login_url=None, current_app=None, extra_context=N """ if not login_url: login_url = settings.LOGIN_URL + login_url = resolve_url(login_url) return logout(request, login_url, current_app=current_app, extra_context=extra_context) def redirect_to_login(next, login_url=None, @@ -117,10 +118,9 @@ def redirect_to_login(next, login_url=None, """ Redirects the user to the login page, passing the given 'next' page """ - # urlparse chokes on lazy objects in Python 3 - login_url_as_str = force_str(login_url or settings.LOGIN_URL) + resolved_url = resolve_url(login_url or settings.LOGIN_URL) - login_url_parts = list(urlparse(login_url_as_str)) + login_url_parts = list(urlparse(resolved_url)) if redirect_field_name: querystring = QueryDict(login_url_parts[4], mutable=True) querystring[redirect_field_name] = next @@ -229,7 +229,7 @@ def password_reset_complete(request, template_name='registration/password_reset_complete.html', current_app=None, extra_context=None): context = { - 'login_url': settings.LOGIN_URL + 'login_url': resolve_url(settings.LOGIN_URL) } if extra_context is not None: context.update(extra_context) diff --git a/django/shortcuts/__init__.py b/django/shortcuts/__init__.py index 154f224671..a824446b7e 100644 --- a/django/shortcuts/__init__.py +++ b/django/shortcuts/__init__.py @@ -66,23 +66,7 @@ def redirect(to, *args, **kwargs): else: redirect_class = HttpResponseRedirect - # If it's a model, use get_absolute_url() - if hasattr(to, 'get_absolute_url'): - return redirect_class(to.get_absolute_url()) - - # Next try a reverse URL resolution. - try: - return redirect_class(urlresolvers.reverse(to, args=args, kwargs=kwargs)) - except urlresolvers.NoReverseMatch: - # If this is a callable, re-raise. - if callable(to): - raise - # If this doesn't "feel" like a URL, re-raise. - if '/' not in to and '.' not in to: - raise - - # Finally, fall back and assume it's a URL - return redirect_class(to) + return redirect_class(resolve_url(to, *args, **kwargs)) def _get_queryset(klass): """ @@ -128,3 +112,34 @@ def get_list_or_404(klass, *args, **kwargs): raise Http404('No %s matches the given query.' % queryset.model._meta.object_name) return obj_list +def resolve_url(to, *args, **kwargs): + """ + Return a URL appropriate for the arguments passed. + + The arguments could be: + + * A model: the model's `get_absolute_url()` function will be called. + + * A view name, possibly with arguments: `urlresolvers.reverse()` will + be used to reverse-resolve the name. + + * A URL, which will be returned as-is. + + """ + # If it's a model, use get_absolute_url() + if hasattr(to, 'get_absolute_url'): + return to.get_absolute_url() + + # Next try a reverse URL resolution. + try: + return urlresolvers.reverse(to, args=args, kwargs=kwargs) + except urlresolvers.NoReverseMatch: + # If this is a callable, re-raise. + if callable(to): + raise + # If this doesn't "feel" like a URL, re-raise. + if '/' not in to and '.' not in to: + raise + + # Finally, fall back and assume it's a URL + return to diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index 4729a2b6f1..f443138569 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -1304,25 +1304,13 @@ The URL where requests are redirected after login when the This is used by the :func:`~django.contrib.auth.decorators.login_required` decorator, for example. -.. _`note on LOGIN_REDIRECT_URL setting`: +.. versionchanged:: 1.5 -.. note:: - You can use :func:`~django.core.urlresolvers.reverse_lazy` to reference - URLs by their name instead of providing a hardcoded value. Assuming a - ``urls.py`` with an URLpattern named ``home``:: - - urlpatterns = patterns('', - url('^welcome/$', 'test_app.views.home', name='home'), - ) - - You can use :func:`~django.core.urlresolvers.reverse_lazy` like this:: - - from django.core.urlresolvers import reverse_lazy - - LOGIN_REDIRECT_URL = reverse_lazy('home') - - This also works fine with localized URLs using - :func:`~django.conf.urls.i18n.i18n_patterns`. +This setting now also accepts view function names and +:ref:`named URL patterns ` which can be used to reduce +configuration duplication since you no longer have to define the URL in two +places (``settings`` and URLconf). +For backward compatibility reasons the default remains unchanged. .. setting:: LOGIN_URL @@ -1334,8 +1322,13 @@ Default: ``'/accounts/login/'`` The URL where requests are redirected for login, especially when using the :func:`~django.contrib.auth.decorators.login_required` decorator. -.. note:: - See the `note on LOGIN_REDIRECT_URL setting`_ +.. versionchanged:: 1.5 + +This setting now also accepts view function names and +:ref:`named URL patterns ` which can be used to reduce +configuration duplication since you no longer have to define the URL in two +places (``settings`` and URLconf). +For backward compatibility reasons the default remains unchanged. .. setting:: LOGOUT_URL diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 5578e8efcb..6420239f47 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -121,6 +121,12 @@ Django 1.5 also includes several smaller improvements worth noting: argument. By default the batch_size is unlimited except for SQLite where single batch is limited so that 999 parameters per query isn't exceeded. +* The :setting:`LOGIN_URL` and :setting:`LOGIN_REDIRECT_URL` settings now also + accept view function names and + :ref:`named URL patterns `. This allows you to reduce + configuration duplication. More information can be found in the + :func:`~django.contrib.auth.decorators.login_required` documentation. + Backwards incompatible changes in 1.5 ===================================== diff --git a/docs/topics/auth.txt b/docs/topics/auth.txt index f8dbbb65a6..c45e4bbaf7 100644 --- a/docs/topics/auth.txt +++ b/docs/topics/auth.txt @@ -947,6 +947,13 @@ The login_required decorator (r'^accounts/login/$', 'django.contrib.auth.views.login'), + .. versionchanged:: 1.5 + + As of version 1.5 :setting:`settings.LOGIN_URL ` now also accepts + view function names and :ref:`named URL patterns `. + This allows you to freely remap your login view within your URLconf + without having to update the setting. + .. function:: views.login(request, [template_name, redirect_field_name, authentication_form]) **URL name:** ``login`` diff --git a/tests/regressiontests/comment_tests/tests/__init__.py b/tests/regressiontests/comment_tests/tests/__init__.py index 40c0de0d57..f80b29e17b 100644 --- a/tests/regressiontests/comment_tests/tests/__init__.py +++ b/tests/regressiontests/comment_tests/tests/__init__.py @@ -17,7 +17,7 @@ CT = ContentType.objects.get_for_model @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.UnsaltedMD5PasswordHasher',)) class CommentTestCase(TestCase): fixtures = ["comment_tests"] - urls = 'django.contrib.comments.urls' + urls = 'regressiontests.comment_tests.urls_default' def createSomeComments(self): # Two anonymous comments on two different objects diff --git a/tests/regressiontests/comment_tests/urls_default.py b/tests/regressiontests/comment_tests/urls_default.py new file mode 100644 index 0000000000..bfce8ffc90 --- /dev/null +++ b/tests/regressiontests/comment_tests/urls_default.py @@ -0,0 +1,9 @@ +from django.conf.urls.defaults import * + +urlpatterns = patterns('', + (r'^', include('django.contrib.comments.urls')), + + # Provide the auth system login and logout views + (r'^accounts/login/$', 'django.contrib.auth.views.login', {'template_name': 'login.html'}), + (r'^accounts/logout/$', 'django.contrib.auth.views.logout'), +) diff --git a/tests/regressiontests/resolve_url/__init__.py b/tests/regressiontests/resolve_url/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regressiontests/resolve_url/models.py b/tests/regressiontests/resolve_url/models.py new file mode 100644 index 0000000000..238902edd2 --- /dev/null +++ b/tests/regressiontests/resolve_url/models.py @@ -0,0 +1,12 @@ +""" +Regression tests for the resolve_url function. +""" + +from django.db import models + + +class UnimportantThing(models.Model): + importance = models.IntegerField() + + def get_absolute_url(self): + return '/importance/%d/' % (self.importance,) diff --git a/tests/regressiontests/resolve_url/tests.py b/tests/regressiontests/resolve_url/tests.py new file mode 100644 index 0000000000..d0bf44abde --- /dev/null +++ b/tests/regressiontests/resolve_url/tests.py @@ -0,0 +1,68 @@ +from __future__ import unicode_literals + +from django.core.urlresolvers import NoReverseMatch +from django.contrib.auth.views import logout +from django.utils.unittest import TestCase +from django.shortcuts import resolve_url + +from .models import UnimportantThing + + +class ResolveUrlTests(TestCase): + """ + Tests for the ``resolve_url`` function. + """ + + def test_url_path(self): + """ + Tests that passing a URL path to ``resolve_url`` will result in the + same url. + """ + self.assertEqual('/something/', resolve_url('/something/')) + + def test_full_url(self): + """ + Tests that passing a full URL to ``resolve_url`` will result in the + same url. + """ + url = 'http://example.com/' + self.assertEqual(url, resolve_url(url)) + + def test_model(self): + """ + Tests that passing a model to ``resolve_url`` will result in + ``get_absolute_url`` being called on that model instance. + """ + m = UnimportantThing(importance=1) + self.assertEqual(m.get_absolute_url(), resolve_url(m)) + + def test_view_function(self): + """ + Tests that passing a view name to ``resolve_url`` will result in the + URL path mapping to that view name. + """ + resolved_url = resolve_url(logout) + self.assertEqual('/accounts/logout/', resolved_url) + + def test_valid_view_name(self): + """ + Tests that passing a view function to ``resolve_url`` will result in + the URL path mapping to that view. + """ + resolved_url = resolve_url('django.contrib.auth.views.logout') + self.assertEqual('/accounts/logout/', resolved_url) + + def test_domain(self): + """ + Tests that passing a domain to ``resolve_url`` returns the same domain. + """ + self.assertEqual(resolve_url('example.com'), 'example.com') + + def test_non_view_callable_raises_no_reverse_match(self): + """ + Tests that passing a non-view callable into ``resolve_url`` raises a + ``NoReverseMatch`` exception. + """ + with self.assertRaises(NoReverseMatch): + resolve_url(lambda: 'asdf') + diff --git a/tests/runtests.py b/tests/runtests.py index c548d2745b..a81fee6858 100755 --- a/tests/runtests.py +++ b/tests/runtests.py @@ -92,7 +92,7 @@ def setup(verbosity, test_labels): settings.TEMPLATE_DIRS = (os.path.join(RUNTESTS_DIR, TEST_TEMPLATE_DIR),) settings.USE_I18N = True settings.LANGUAGE_CODE = 'en' - settings.LOGIN_URL = '/accounts/login/' + settings.LOGIN_URL = 'django.contrib.auth.views.login' settings.MIDDLEWARE_CLASSES = ( 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware',