Skip to content

Commit

Permalink
Fixed django#15552 -- LOGIN_URL and LOGIN_REDIRECT_URL can take URLpa…
Browse files Browse the repository at this point in the history
…ttern names.

Thanks UloPe and Eric Florenzano for the patch, and Malcolm Tredinnick for
review.
  • Loading branch information
carljm committed Sep 8, 2012
1 parent 518c582 commit a78dd10
Show file tree
Hide file tree
Showing 13 changed files with 162 additions and 52 deletions.
6 changes: 3 additions & 3 deletions django/contrib/auth/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
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):
Expand All @@ -23,11 +24,10 @@ def _wrapped_view(request, *args, **kwargs):
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)):
Expand Down
2 changes: 1 addition & 1 deletion django/contrib/auth/tests/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def normal_view(request):
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.
Expand Down
18 changes: 9 additions & 9 deletions django/contrib/auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
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
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -110,17 +110,17 @@ 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,
redirect_field_name=REDIRECT_FIELD_NAME):
"""
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
Expand Down Expand Up @@ -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)
Expand Down
49 changes: 32 additions & 17 deletions django/shortcuts/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
33 changes: 13 additions & 20 deletions docs/ref/settings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`:

.. 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')
.. versionchanged:: 1.5

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 <naming-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

Expand All @@ -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 <naming-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

Expand Down
6 changes: 6 additions & 0 deletions docs/releases/1.5.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 <naming-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
=====================================

Expand Down
7 changes: 7 additions & 0 deletions docs/topics/auth.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 <LOGIN_URL>` now also accepts
view function names and :ref:`named URL patterns <naming-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``
Expand Down
2 changes: 1 addition & 1 deletion tests/regressiontests/comment_tests/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
@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
Expand Down
9 changes: 9 additions & 0 deletions tests/regressiontests/comment_tests/urls_default.py
Original file line number Diff line number Diff line change
@@ -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'),
)
Empty file.
12 changes: 12 additions & 0 deletions tests/regressiontests/resolve_url/models.py
Original file line number Diff line number Diff line change
@@ -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,)
68 changes: 68 additions & 0 deletions tests/regressiontests/resolve_url/tests.py
Original file line number Diff line number Diff line change
@@ -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')

2 changes: 1 addition & 1 deletion tests/runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit a78dd10

Please sign in to comment.