[615591] Don't use LOGIN_* or LOGOUT_* settings, use reverse() instead.

* Localize our login/logout URLs
* Write a login_required decorator
* Factor our login_required, logout_required and permission_required to use a common user_access_decorator
* Move access tests around to be happier with where they belong.
* Bunch more tests for the permission_required decorator.

apps/access/decorators.py

@@ -1,7 +1,6 @@
 from functools import wraps
 import inspect
 
-from django.conf import settings
 from django.contrib.auth import REDIRECT_FIELD_NAME
 from django.db.models import Model, get_model
 from django.http import HttpResponseForbidden, HttpResponseRedirect
@@ -10,6 +9,76 @@ from django.utils.decorators import available_attrs
 from django.utils.http import urlquote
 
 import access
+from sumo.urlresolvers import reverse
+
+
+def user_access_decorator(redirect_func, redirect_url_func, deny_func=None,
+                          redirect_field=REDIRECT_FIELD_NAME):
+    """
+    Helper function that returns a decorator.
+
+    * redirect func ----- If truthy, a redirect will occur
+    * deny_func --------- If truthy, HttpResponseForbidden is returned.
+    * redirect_url_func - Evaluated at view time, returns the redirect URL
+                          i.e. where to go if redirect_func is truthy.
+    * redirect_field ---- What field to set in the url, defaults to Django's.
+                          Set this to None to exclude it from the URL.
+
+    """
+    def decorator(view_fn):
+        def _wrapped_view(request, *args, **kwargs):
+            if redirect_func(request.user):
+                # We must call reverse at the view level, else the threadlocal
+                # locale prefixing doesn't take effect.
+                redirect_url = redirect_url_func() or reverse('users.login')
+
+                # Redirect back here afterwards?
+                if redirect_field:
+                    path = urlquote(request.get_full_path())
+                    redirect_url = '%s?%s=%s' % (
+                        redirect_url, redirect_field, path)
+
+                return HttpResponseRedirect(redirect_url)
+
+            if deny_func and deny_func(request.user):
+                return HttpResponseForbidden()
+
+            return view_fn(request, *args, **kwargs)
+        return wraps(view_fn, assigned=available_attrs(view_fn))(_wrapped_view)
+
+    return decorator
+
+
+def logout_required(redirect):
+    """Requires that the user *not* be logged in."""
+    redirect_func = lambda u: u.is_authenticated()
+    if hasattr(redirect, '__call__'):
+        return user_access_decorator(
+            redirect_func, redirect_field=None,
+            redirect_url_func=lambda: reverse('home'))(redirect)
+    else:
+        return user_access_decorator(redirect_func, redirect_field=None,
+                                     redirect_url_func=lambda: redirect)
+
+
+def login_required(func, login_url=None, redirect=REDIRECT_FIELD_NAME):
+    """Requires that the user is logged in."""
+    redirect_func = lambda u: not u.is_authenticated()
+    redirect_url_func = lambda: login_url
+    return user_access_decorator(redirect_func, redirect_field=redirect,
+                                 redirect_url_func=redirect_url_func)(func)
+
+
+def permission_required(perm, login_url=None, redirect=REDIRECT_FIELD_NAME):
+    """A replacement for django.contrib.auth.decorators.permission_required
+    that doesn't ask authenticated users to log in."""
+    redirect_func = lambda u: not u.is_authenticated()
+    deny_func = lambda u: not u.has_perm(perm)
+    redirect_url_func = lambda: login_url
+
+    return user_access_decorator(redirect_func, redirect_field=redirect,
+                                 redirect_url_func=redirect_url_func,
+                                 deny_func=deny_func)
 
 
 def has_perm_or_owns_or_403(perm, owner_attr, obj_lookup, perm_obj_lookup,
@@ -70,23 +139,3 @@ def _resolve_lookup((model, lookup, arg_name), view_kwargs):
     if inspect.isclass(model_class) and not issubclass(model_class, Model):
         raise ValueError("The argument '%s' needs to be a model." % model)
     return get_object_or_404(model_class, **{lookup: value})
-
-
-def permission_required(perm, login_url=None, redirect=REDIRECT_FIELD_NAME):
-    """A replacement for django.contrib.auth.decorators.permission_required
-    that doesn't ask authenticated users to log in."""
-
-    if not login_url:
-        login_url = settings.LOGIN_URL
-
-    def decorator(view_fn):
-        def _wrapped_view(request, *args, **kwargs):
-            if request.user.is_authenticated():
-                if request.user.has_perm(perm):
-                    return view_fn(request, *args, **kwargs)
-                return HttpResponseForbidden()
-            path = urlquote(request.get_full_path)
-            tup = login_url, redirect, path
-            return HttpResponseRedirect('%s?%s=%s' % tup)
-        return wraps(view_fn, assigned=available_attrs(view_fn))(_wrapped_view)
-    return decorator

apps/access/tests.py

@@ -1,178 +0,0 @@
-from django.contrib.auth.models import User
-
-from nose.tools import eq_
-import test_utils
-
-import access
-from access.helpers import has_perm, has_perm_or_owns
-from forums.models import Forum, Thread
-from sumo.tests import TestCase
-from sumo.urlresolvers import reverse
-
-
-class ForumTestPermissions(TestCase):
-    fixtures = ['users.json', 'posts.json', 'forums_permissions.json']
-
-    def setUp(self):
-        url = reverse('forums.threads', args=[u'test-forum'])
-        self.context = {'request': test_utils.RequestFactory().get(url)}
-        self.forum_1 = Forum.objects.get(pk=1)
-        self.forum_2 = Forum.objects.get(pk=2)
-
-    def test_has_perm_thread_edit(self):
-        """
-        User in ForumsModerator group can edit thread in forum_1, but not in
-        forum_2.
-        """
-        self.context['request'].user = User.objects.get(pk=47963)
-        allowed = has_perm(self.context, 'forums_forum.thread_edit_forum',
-                           self.forum_1)
-        eq_(allowed, True)
-        allowed = has_perm(self.context, 'forums_forum.thread_edit_forum',
-                           self.forum_2)
-        eq_(allowed, False)
-
-    def test_has_perm_or_owns_thread_edit(self):
-        """
-        User in ForumsModerator group can edit thread in forum_1, but not in
-        forum_2.
-        """
-        me = User.objects.get(pk=118533)
-        my_t = Thread.objects.filter(creator=me)[0]
-        other_t = Thread.objects.exclude(creator=me)[0]
-        self.context['request'].user = me
-        perm = 'forums_forum.thread_edit_forum'
-        allowed = has_perm_or_owns(self.context, perm, my_t, self.forum_1)
-        eq_(allowed, True)
-        allowed = has_perm_or_owns(self.context, perm, other_t, self.forum_1)
-        eq_(allowed, False)
-
-    def test_has_perm_thread_delete(self):
-        """
-        User in ForumsModerator group can delete thread in forum_1, but not in
-        forum_2.
-        """
-        self.context['request'].user = User.objects.get(pk=47963)
-        allowed = has_perm(self.context, 'forums_forum.thread_delete_forum',
-                           self.forum_1)
-        eq_(allowed, True)
-        allowed = has_perm(self.context, 'forums_forum.thread_delete_forum',
-                           self.forum_2)
-        eq_(allowed, False)
-
-    def test_has_perm_thread_sticky(self):
-        """
-        User in ForumsModerator group can change sticky status of thread in
-        forum_1, but not in forum_2.
-        """
-        self.context['request'].user = User.objects.get(pk=47963)
-        allowed = has_perm(self.context, 'forums_forum.thread_sticky_forum',
-                           self.forum_1)
-        eq_(allowed, True)
-        allowed = has_perm(self.context, 'forums_forum.thread_sticky_forum',
-                           self.forum_2)
-        eq_(allowed, False)
-
-    def test_has_perm_thread_locked(self):
-        """
-        Sanity check: ForumsModerator group has no permission to change locked
-        status in forum_1.
-        """
-        self.context['request'].user = User.objects.get(pk=47963)
-        allowed = has_perm(self.context, 'forums_forum.thread_locked_forum',
-                           self.forum_1)
-        eq_(allowed, False)
-
-    def test_has_perm_post_edit(self):
-        """
-        User in ForumsModerator group can edit any post in forum_1, but not
-        in forum_2.
-        """
-        self.context['request'].user = User.objects.get(pk=47963)
-        allowed = has_perm(self.context, 'forums_forum.post_edit_forum',
-                           self.forum_1)
-        eq_(allowed, True)
-        allowed = has_perm(self.context, 'forums_forum.post_edit_forum',
-                           self.forum_2)
-        eq_(allowed, False)
-
-    def test_has_perm_post_delete(self):
-        """
-        User in ForumsModerator group can delete any post in forum_1, but not
-        in forum_2.
-        """
-        self.context['request'].user = User.objects.get(pk=47963)
-        allowed = has_perm(self.context, 'forums_forum.post_delete_forum',
-                           self.forum_1)
-        eq_(allowed, True)
-        allowed = has_perm(self.context, 'forums_forum.post_delete_forum',
-                           self.forum_2)
-        eq_(allowed, False)
-
-    def test_no_perm_thread_delete(self):
-        """
-        User not in ForumsModerator group cannot delete thread in any forum.
-        """
-        self.context['request'].user = User.objects.get(pk=118533)
-        allowed = has_perm(self.context, 'forums_forum.thread_delete_forum',
-                           self.forum_1)
-        eq_(allowed, False)
-        allowed = has_perm(self.context, 'forums_forum.thread_delete_forum',
-                           self.forum_2)
-        eq_(allowed, False)
-
-    def test_admin_perm_thread(self):
-        """Super user can do anything on any forum."""
-        self.context['request'].user = User.objects.get(pk=1)
-
-        # Loop over all forums perms and both forums
-        perms = ('thread_edit_forum', 'thread_delete_forum', 'post_edit_forum',
-                 'thread_sticky_forum', 'thread_locked_forum',
-                 'post_delete_forum')
-        forums = (self.forum_1, self.forum_2)
-
-        for perm in perms:
-            for forum in forums:
-                allowed = has_perm(self.context, 'forums_forum.' + perm,
-                                   forum)
-                eq_(allowed, True)
-
-    def test_util_has_perm_or_owns_sanity(self):
-        """Sanity check for access.has_perm_or_owns."""
-        me = User.objects.get(pk=118533)
-        my_t = Thread.objects.filter(creator=me)[0]
-        other_t = Thread.objects.exclude(creator=me)[0]
-        perm = 'forums_forum.thread_edit_forum'
-        allowed = access.has_perm_or_owns(me, perm, my_t, self.forum_1)
-        eq_(allowed, True)
-        allowed = access.has_perm_or_owns(me, perm, other_t, self.forum_1)
-        eq_(allowed, False)
-
-    def test_has_perm_per_object(self):
-        """Assert has_perm checks per-object permissions correctly."""
-        user = User.objects.get(pk=47963)
-        perm = 'forums_forum.thread_edit_forum'
-        assert access.has_perm(user, perm, self.forum_1)
-        assert not access.has_perm(user, perm, self.forum_2)
-
-    def test_perm_is_defined_on(self):
-        """Test whether we check for permission relationship, independent of
-        whether the permission is actually assigned to anyone."""
-        perm = 'forums_forum.view_in_forum'
-        assert access.perm_is_defined_on(perm, Forum.objects.get(pk=3))
-        assert not access.perm_is_defined_on(perm, Forum.objects.get(pk=2))
-
-    def test_permission_required(self):
-        """Test our new permission required decorator."""
-        url = reverse('flagit.queue', force_locale=True)
-        self.client.logout()
-        resp = self.client.get(url)
-        eq_(302, resp.status_code)
-
-        self.client.login(username='tagger', password='testpass')
-        resp = self.client.get(url)
-        eq_(403, resp.status_code)
-
-        self.client.login(username='admin', password='testpass')
-        resp = self.client.get(url)
-        eq_(200, resp.status_code)

apps/access/tests/__init__.py

@@ -0,0 +1,59 @@
+from django.contrib.auth.models import User
+
+from nose.tools import eq_
+import test_utils
+
+import access
+from forums.models import Forum, Thread
+from sumo.tests import TestCase
+from sumo.urlresolvers import reverse
+
+
+class AccessTests(TestCase):
+    """Test stuff in access/__init__.py"""
+    fixtures = ['users.json', 'posts.json', 'forums_permissions.json']
+
+    def setUp(self):
+        url = reverse('forums.threads', args=[u'test-forum'])
+        self.context = {'request': test_utils.RequestFactory().get(url)}
+        self.forum_1 = Forum.objects.get(pk=1)
+        self.forum_2 = Forum.objects.get(pk=2)
+
+    def test_admin_perm_thread(self):
+        """Super user can do anything on any forum."""
+        admin = User.objects.get(pk=1)
+
+        # Loop over all forums perms and both forums
+        perms = ('thread_edit_forum', 'thread_delete_forum', 'post_edit_forum',
+                 'thread_sticky_forum', 'thread_locked_forum',
+                 'post_delete_forum')
+        forums = (self.forum_1, self.forum_2)
+
+        for perm in perms:
+            for forum in forums:
+                assert access.has_perm(admin, 'forums_forum.' + perm, forum)
+
+    def test_util_has_perm_or_owns_sanity(self):
+        """Sanity check for access.has_perm_or_owns."""
+        me = User.objects.get(pk=118533)
+        my_t = Thread.objects.filter(creator=me)[0]
+        other_t = Thread.objects.exclude(creator=me)[0]
+        perm = 'forums_forum.thread_edit_forum'
+        allowed = access.has_perm_or_owns(me, perm, my_t, self.forum_1)
+        eq_(allowed, True)
+        allowed = access.has_perm_or_owns(me, perm, other_t, self.forum_1)
+        eq_(allowed, False)
+
+    def test_has_perm_per_object(self):
+        """Assert has_perm checks per-object permissions correctly."""
+        user = User.objects.get(pk=47963)
+        perm = 'forums_forum.thread_edit_forum'
+        assert access.has_perm(user, perm, self.forum_1)
+        assert not access.has_perm(user, perm, self.forum_2)
+
+    def test_perm_is_defined_on(self):
+        """Test whether we check for permission relationship, independent of
+        whether the permission is actually assigned to anyone."""
+        perm = 'forums_forum.view_in_forum'
+        assert access.perm_is_defined_on(perm, Forum.objects.get(pk=3))
+        assert not access.perm_is_defined_on(perm, Forum.objects.get(pk=2))

apps/access/tests/test_decorators.py

@@ -0,0 +1,82 @@
+from django.contrib.auth.models import User, AnonymousUser
+from django.http import HttpResponse
+
+from nose.tools import eq_
+import test_utils
+
+from access.decorators import (logout_required, login_required,
+                               permission_required)
+from sumo.tests import TestCase
+
+
+def simple_view(request):
+    return HttpResponse()
+
+
+class LogoutRequiredTestCase(TestCase):
+    fixtures = ['users.json']
+
+    def test_logged_out_default(self):
+        request = test_utils.RequestFactory().get('/foo')
+        request.user = AnonymousUser()
+        view = logout_required(simple_view)
+        response = view(request)
+        eq_(200, response.status_code)
+
+    def test_logged_in_default(self):
+        request = test_utils.RequestFactory().get('/foo')
+        request.user = User.objects.get(username='jsocol')
+        view = logout_required(simple_view)
+        response = view(request)
+        eq_(302, response.status_code)
+
+    def test_logged_in_argument(self):
+        request = test_utils.RequestFactory().get('/foo')
+        request.user = User.objects.get(username='jsocol')
+        view = logout_required('/bar')(simple_view)
+        response = view(request)
+        eq_(302, response.status_code)
+        eq_('/bar', response['location'])
+
+
+class LoginRequiredTestCase(TestCase):
+    fixtures = ['users.json']
+
+    def test_logged_out_default(self):
+        request = test_utils.RequestFactory().get('/foo')
+        request.user = AnonymousUser()
+        view = login_required(simple_view)
+        response = view(request)
+        eq_(302, response.status_code)
+
+    def test_logged_in_default(self):
+        request = test_utils.RequestFactory().get('/foo')
+        request.user = User.objects.get(username='jsocol')
+        view = login_required(simple_view)
+        response = view(request)
+        eq_(200, response.status_code)
+
+
+class PermissionRequiredTestCase(TestCase):
+    fixtures = ['users.json']
+
+    def test_logged_out_default(self):
+        request = test_utils.RequestFactory().get('/foo')
+        request.user = AnonymousUser()
+        view = permission_required('perm')(simple_view)
+        response = view(request)
+        eq_(302, response.status_code)
+
+    def test_logged_in_default(self):
+        request = test_utils.RequestFactory().get('/foo')
+        request.user = User.objects.get(username='jsocol')
+        view = permission_required('perm')(simple_view)
+        response = view(request)
+        eq_(403, response.status_code)
+
+    def test_logged_in_admin(self):
+        request = test_utils.RequestFactory().get('/foo')
+        request.user = User.objects.get(username='admin')
+        view = permission_required('perm')(simple_view)
+        response = view(request)
+        eq_(200, response.status_code)

apps/flagit/tests/__init__.py

@@ -4,7 +4,6 @@ from django.conf import settings
 from django.template.defaultfilters import slugify
 
 from sumo.tests import LocalizingClient, TestCase
-from sumo.urlresolvers import reverse
 
 
 class TestCaseBase(TestCase):

apps/flagit/tests/test_permissions.py

@@ -0,0 +1,23 @@
+from nose.tools import eq_
+
+from sumo.tests import TestCase
+from sumo.urlresolvers import reverse
+
+
+class FlagitTestPermissions(TestCase):
+    fixtures = ['users.json']
+
+    def test_permission_required(self):
+        """Test our new permission required decorator."""
+        url = reverse('flagit.queue', force_locale=True)
+        self.client.logout()
+        resp = self.client.get(url)
+        eq_(302, resp.status_code)
+
+        self.client.login(username='tagger', password='testpass')
+        resp = self.client.get(url)
+        eq_(403, resp.status_code)
+
+        self.client.login(username='admin', password='testpass')
+        resp = self.client.get(url)
+        eq_(200, resp.status_code)

apps/flagit/views.py

@@ -2,14 +2,13 @@ import json
 
 from django.http import HttpResponseRedirect, HttpResponse
 from django.shortcuts import get_object_or_404
-from django.contrib.auth.decorators import login_required
 from django.contrib.contenttypes.models import ContentType
 from django.views.decorators.http import require_POST
 
 import jingo
 from tower import ugettext as _
 
-from access.decorators import permission_required
+from access.decorators import permission_required, login_required
 from flagit.models import FlaggedObject
 from sumo.urlresolvers import reverse
 

apps/forums/tests/__init__.py

@@ -126,8 +126,7 @@ class PostTestCase(ForumTestCase):
     def test_post_no_session(self):
         r = get(self.client, 'forums.new_thread',
                 kwargs={'forum_slug': 'test-forum'})
-        assert('http://testserver' + settings.LOGIN_URL in
-                r.redirect_chain[0][0])
+        assert(settings.LOGIN_URL in r.redirect_chain[0][0])
         eq_(302, r.redirect_chain[0][1])
 
 
@@ -137,6 +136,5 @@ class ThreadTestCase(ForumTestCase):
         """Delete a thread while logged out redirects."""
         r = get(self.client, 'forums.delete_thread',
                 kwargs={'forum_slug': 'test-forum', 'thread_id': 1})
-        assert('http://testserver' + settings.LOGIN_URL in
-               r.redirect_chain[0][0])
+        assert(settings.LOGIN_URL in r.redirect_chain[0][0])
         eq_(302, r.redirect_chain[0][1])

apps/forums/tests/test_permissions.py

@@ -0,0 +1,107 @@
+from django.contrib.auth.models import User
+
+import test_utils
+
+from access.helpers import has_perm, has_perm_or_owns
+from forums.models import Forum, Thread
+from sumo.tests import TestCase
+from sumo.urlresolvers import reverse
+
+
+class ForumTestPermissions(TestCase):
+    """Make sure access helpers work on the forums."""
+
+    fixtures = ['users.json', 'posts.json', 'forums_permissions.json']
+
+    def setUp(self):
+        url = reverse('forums.threads', args=[u'test-forum'])
+        self.context = {'request': test_utils.RequestFactory().get(url)}
+        self.forum_1 = Forum.objects.get(pk=1)
+        self.forum_2 = Forum.objects.get(pk=2)
+
+    def test_has_perm_thread_edit(self):
+        """
+        User in ForumsModerator group can edit thread in forum_1, but not in
+        forum_2.
+        """
+        self.context['request'].user = User.objects.get(pk=47963)
+        assert has_perm(self.context, 'forums_forum.thread_edit_forum',
+                        self.forum_1)
+        assert not has_perm(self.context, 'forums_forum.thread_edit_forum',
+                            self.forum_2)
+
+    def test_has_perm_or_owns_thread_edit(self):
+        """
+        User in ForumsModerator group can edit thread in forum_1, but not in
+        forum_2.
+        """
+        me = User.objects.get(pk=118533)
+        my_t = Thread.objects.filter(creator=me)[0]
+        other_t = Thread.objects.exclude(creator=me)[0]
+        self.context['request'].user = me
+        perm = 'forums_forum.thread_edit_forum'
+        assert has_perm_or_owns(self.context, perm, my_t, self.forum_1)
+        assert not has_perm_or_owns(self.context, perm, other_t, self.forum_1)
+
+    def test_has_perm_thread_delete(self):
+        """
+        User in ForumsModerator group can delete thread in forum_1, but not in
+        forum_2.
+        """
+        self.context['request'].user = User.objects.get(pk=47963)
+        assert has_perm(self.context, 'forums_forum.thread_delete_forum',
+                        self.forum_1)
+        assert not has_perm(self.context, 'forums_forum.thread_delete_forum',
+                            self.forum_2)
+
+    def test_has_perm_thread_sticky(self):
+        """
+        User in ForumsModerator group can change sticky status of thread in
+        forum_1, but not in forum_2.
+        """
+        self.context['request'].user = User.objects.get(pk=47963)
+        assert has_perm(self.context, 'forums_forum.thread_sticky_forum',
+                        self.forum_1)
+        assert not has_perm(self.context, 'forums_forum.thread_sticky_forum',
+                            self.forum_2)
+
+    def test_has_perm_thread_locked(self):
+        """
+        Sanity check: ForumsModerator group has no permission to change locked
+        status in forum_1.
+        """
+        self.context['request'].user = User.objects.get(pk=47963)
+        assert not has_perm(self.context, 'forums_forum.thread_locked_forum',
+                            self.forum_1)
+
+    def test_has_perm_post_edit(self):
+        """
+        User in ForumsModerator group can edit any post in forum_1, but not
+        in forum_2.
+        """
+        self.context['request'].user = User.objects.get(pk=47963)
+        assert has_perm(self.context, 'forums_forum.post_edit_forum',
+                        self.forum_1)
+        assert not has_perm(self.context, 'forums_forum.post_edit_forum',
+                            self.forum_2)
+
+    def test_has_perm_post_delete(self):
+        """
+        User in ForumsModerator group can delete any post in forum_1, but not
+        in forum_2.
+        """
+        self.context['request'].user = User.objects.get(pk=47963)
+        assert has_perm(self.context, 'forums_forum.post_delete_forum',
+                        self.forum_1)
+        assert not has_perm(self.context, 'forums_forum.post_delete_forum',
+                            self.forum_2)
+
+    def test_no_perm_thread_delete(self):
+        """
+        User not in ForumsModerator group cannot delete thread in any forum.
+        """
+        self.context['request'].user = User.objects.get(pk=118533)
+        assert not has_perm(self.context, 'forums_forum.thread_delete_forum',
+                            self.forum_1)
+        assert not has_perm(self.context, 'forums_forum.thread_delete_forum',
+                            self.forum_2)

apps/forums/views.py

@@ -1,7 +1,6 @@
 import logging
 from datetime import datetime
 
-from django.contrib.auth.decorators import login_required
 from django.core.exceptions import PermissionDenied
 from django.http import HttpResponseRedirect, Http404
 from django.shortcuts import get_object_or_404
@@ -10,7 +9,7 @@ from django.views.decorators.http import require_POST
 import jingo
 from authority.decorators import permission_required_or_403
 
-from access.decorators import has_perm_or_owns_or_403
+from access.decorators import has_perm_or_owns_or_403, login_required
 from access import has_perm
 from sumo.urlresolvers import reverse
 from sumo.utils import paginate

apps/gallery/views.py

@@ -3,7 +3,6 @@ import json
 import logging
 
 from django.conf import settings
-from django.contrib.auth.decorators import login_required
 from django.db.models import Q
 from django.http import (HttpResponse, HttpResponseRedirect,
                          HttpResponseBadRequest, Http404)
@@ -14,6 +13,7 @@ from commonware.decorators import xframe_sameorigin
 import jingo
 from tower import ugettext as _
 
+from access.decorators import login_required
 from gallery import ITEMS_PER_PAGE, DRAFT_TITLE_PREFIX
 from gallery.forms import ImageForm, VideoForm
 from gallery.models import Image, Video

apps/kbforums/tests/__init__.py

@@ -105,8 +105,7 @@ class PostTestCase(KBForumTestCase):
     def test_post_no_session(self):
         r = get(self.client, 'wiki.discuss.new_thread',
                 kwargs={'document_slug': 'article-title'})
-        assert('http://testserver' + settings.LOGIN_URL in
-               r.redirect_chain[0][0])
+        assert(settings.LOGIN_URL in r.redirect_chain[0][0])
         eq_(302, r.redirect_chain[0][1])
 
 
@@ -116,6 +115,5 @@ class ThreadTestCase(KBForumTestCase):
         """Delete a thread while logged out redirects."""
         r = get(self.client, 'wiki.discuss.delete_thread',
                 kwargs={'document_slug': 'article-title', 'thread_id': 1})
-        assert('http://testserver' + settings.LOGIN_URL in
-               r.redirect_chain[0][0])
+        assert(settings.LOGIN_URL in r.redirect_chain[0][0])
         eq_(302, r.redirect_chain[0][1])

apps/kbforums/views.py

@@ -1,7 +1,6 @@
 import logging
 from datetime import datetime
 
-from django.contrib.auth.decorators import login_required
 from django.core.exceptions import PermissionDenied
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
@@ -9,7 +8,7 @@ from django.views.decorators.http import require_POST
 
 import jingo
 
-from access.decorators import permission_required
+from access.decorators import permission_required, login_required
 import kbforums
 from kbforums.feeds import ThreadsFeed, PostsFeed
 from kbforums.forms import (ReplyForm, NewThreadForm,

apps/questions/templates/questions/answers.html

@@ -296,10 +296,10 @@
       <div class="user-section"></div>
       <div class="main-section">
         <p>
-          {{ _('You must <a href="{url}">log in to your account</a> to reply to posts.')|fe(url=settings.LOGIN_URL) }}
+          {{ _('You must <a href="{url}">log in to your account</a> to reply to posts.')|fe(url=url('users.login')) }}
         </p>
         <p>
-          {{ _("Don't have an account? You can <a href='{url}'>create a free account</a> now.")|fe(url=settings.REGISTER_URL) }}
+          {{ _("Don't have an account? You can <a href='{url}'>create a free account</a> now.")|fe(url=url('users.register')) }}
         </p>
       </div>
       <div class="side-section"></div>

apps/questions/templates/questions/base.html

@@ -29,11 +29,11 @@
       <nav>
         <a href="{{ profile_url(user) }}">{{ _('Profile') }}</a>
         &bull;
-        <a href="{{ settings.LOGOUT_URL }}">{{ _('Sign Out') }}</a>
+        <a href="{{ url('users.logout') }}">{{ _('Sign Out') }}</a>
       </nav>
     {% else %}
       <p>
-        {% trans login_url=settings.LOGIN_URL, register_url=settings.REGISTER_URL %}
+        {% trans login_url=url('users.login'), register_url=url('users.register') %}
           Want to contribute? <a href="{{ login_url }}">Sign In</a> or <a href="{{ register_url }}">Register</a>
         {% endtrans %}
       </p>

apps/questions/tests/test_templates.py

@@ -274,15 +274,15 @@ class AnswersTemplateTestCase(TestCaseBase):
                        args=[self.question.id])
         redirect = response.redirect_chain[0]
         eq_(302, redirect[1])
-        eq_('http://testserver%s?next=/en-US/questions/1/delete' %
-            settings.LOGIN_URL, redirect[0])
+        eq_('http://testserver/%s%s?next=/en-US/questions/1/delete' %
+            (settings.LANGUAGE_CODE, settings.LOGIN_URL), redirect[0])
 
         response = post(self.client, 'questions.delete',
                         args=[self.question.id])
         redirect = response.redirect_chain[0]
         eq_(302, redirect[1])
-        eq_('http://testserver%s?next=/en-US/questions/1/delete' %
-            settings.LOGIN_URL, redirect[0])
+        eq_('http://testserver/%s%s?next=/en-US/questions/1/delete' %
+            (settings.LANGUAGE_CODE, settings.LOGIN_URL), redirect[0])
 
     def test_delete_question_with_permissions(self):
         """Deleting a question with permissions."""
@@ -315,15 +315,15 @@ class AnswersTemplateTestCase(TestCaseBase):
                        args=[self.question.id, answer.id])
         redirect = response.redirect_chain[0]
         eq_(302, redirect[1])
-        eq_('http://testserver%s?next=/en-US/questions/1/delete/1' %
-            settings.LOGIN_URL, redirect[0])
+        eq_('http://testserver/%s%s?next=/en-US/questions/1/delete/1' %
+            (settings.LANGUAGE_CODE, settings.LOGIN_URL), redirect[0])
 
         response = post(self.client, 'questions.delete_answer',
                         args=[self.question.id, answer.id])
         redirect = response.redirect_chain[0]
         eq_(302, redirect[1])
-        eq_('http://testserver%s?next=/en-US/questions/1/delete/1' %
-            settings.LOGIN_URL, redirect[0])
+        eq_('http://testserver/%s%s?next=/en-US/questions/1/delete/1' %
+            (settings.LANGUAGE_CODE, settings.LOGIN_URL), redirect[0])
 
     def test_delete_answer_with_permissions(self):
         """Deleting an answer with permissions."""
@@ -428,8 +428,8 @@ class AnswersTemplateTestCase(TestCaseBase):
         response = post(self.client, 'questions.lock', args=[q.id])
         redirect = response.redirect_chain[0]
         eq_(302, redirect[1])
-        eq_('http://testserver%s?next=/en-US/questions/1/lock' %
-            settings.LOGIN_URL, redirect[0])
+        eq_('http://testserver/%s%s?next=/en-US/questions/1/lock' %
+            (settings.LANGUAGE_CODE, settings.LOGIN_URL), redirect[0])
 
     def test_lock_question_with_permissions_GET(self):
         """Trying to lock a question via HTTP GET."""

apps/questions/views.py

@@ -5,7 +5,6 @@ import logging
 
 from django.conf import settings
 from django.contrib import auth
-from django.contrib.auth.decorators import login_required
 from django.contrib.auth.forms import AuthenticationForm
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.sites.models import Site
@@ -22,7 +21,8 @@ from taggit.models import Tag
 from tower import ugettext as _
 from tower import ugettext_lazy as _lazy
 
-from access.decorators import has_perm_or_owns_or_403, permission_required
+from access.decorators import (has_perm_or_owns_or_403, permission_required,
+                               login_required)
 from notifications import create_watch, destroy_watch
 import questions as constants
 from questions.feeds import QuestionsFeed, AnswersFeed, TaggedQuestionsFeed

apps/sumo/decorators.py

@@ -1,8 +1,5 @@
-from functools import wraps
-
 from django.conf import settings
 from django.http import HttpResponseRedirect
-from django.utils.decorators import available_attrs
 
 
 def ssl_required(view_func):
@@ -17,23 +14,3 @@ def ssl_required(view_func):
 
         return view_func(request, *args, **kwargs)
     return _checkssl
-
-
-def logout_required(redirect='/'):
-    """Requires that the user *not* be logged in."""
-    callable = False
-    if hasattr(redirect, '__call__'):
-        callable = True
-        view = redirect
-        redirect = '/'
-
-    def decorator(view_func):
-        def _wrapped(request, *args, **kwargs):
-            if request.user and request.user.is_authenticated():
-                return HttpResponseRedirect(redirect)
-            return view_func(request, *args, **kwargs)
-        return wraps(view_func, assigned=available_attrs(view_func))(_wrapped)
-
-    if callable:
-        return decorator(view)
-    return decorator

apps/sumo/tests/test_decorators.py

@@ -1,38 +0,0 @@
-from django.contrib.auth.models import AnonymousUser, User
-from django.http import HttpResponse
-
-from nose.tools import eq_
-from test_utils import RequestFactory
-
-from sumo.decorators import logout_required
-from sumo.tests import TestCase
-
-
-def simple_view(request):
-    return HttpResponse()
-
-
-class LogoutRequiredTestCase(TestCase):
-    fixtures = ['users.json']
-
-    def test_logged_out_default(self):
-        request = RequestFactory().get('/foo')
-        request.user = AnonymousUser()
-        view = logout_required(simple_view)
-        response = view(request)
-        eq_(200, response.status_code)
-
-    def test_logged_in_default(self):
-        request = RequestFactory().get('/foo')
-        request.user = User.objects.get(username='jsocol')
-        view = logout_required(simple_view)
-        response = view(request)
-        eq_(302, response.status_code)
-
-    def test_logged_in_argument(self):
-        request = RequestFactory().get('/foo')
-        request.user = User.objects.get(username='jsocol')
-        view = logout_required('/bar')(simple_view)
-        response = view(request)
-        eq_(302, response.status_code)
-        eq_('/bar', response['location'])

apps/upload/views.py

@@ -1,6 +1,5 @@
 import json
 
-from django.contrib.auth.decorators import login_required
 from django.views.decorators.http import require_POST
 from django.core.exceptions import ObjectDoesNotExist
 from django.db.models import get_model
@@ -10,7 +9,7 @@ from django.http import (HttpResponse, HttpResponseNotFound,
 from commonware.decorators import xframe_sameorigin
 from tower import ugettext as _
 
-from access.decorators import has_perm_or_owns_or_403
+from access.decorators import has_perm_or_owns_or_403, login_required
 from upload.models import ImageAttachment
 from upload.utils import upload_imageattachment, FileTooLargeError
 

apps/users/templates/users/pw_reset_complete.html

@@ -6,6 +6,6 @@
 {% block content %}
   <article id="pw-reset" class="main">
     <h1>{{ title }}</h1>
-    <p><a href="{{ settings.LOGIN_URL|urlparams(next=settings.LOGIN_REDIRECT_URL) }}">{{ _('Log in') }}</a></p>
+    <p><a href="{{ url('users.login')|urlparams(next=url('home')) }}">{{ _('Log in') }}</a></p>
   </article>
 {% endblock content %}

apps/users/tests/test_templates.py

@@ -60,7 +60,8 @@ class LoginTests(TestCaseBase):
                                     {'username': 'rrosario',
                                      'password': 'testpass'})
         eq_(302, response.status_code)
-        eq_('http://testserver' + settings.LOGIN_REDIRECT_URL,
+        eq_('http://testserver' +
+                reverse('home', locale=settings.LANGUAGE_CODE),
             response['location'])
 
     def test_login_next_parameter(self):
@@ -98,7 +99,7 @@ class LoginTests(TestCaseBase):
         '''Test with an invalid ?next=http://example.com parameter.'''
         get_current.return_value.domain = 'testserver.com'
         invalid_next = 'http://foobar.com/evil/'
-        valid_next = settings.LOGIN_REDIRECT_URL
+        valid_next = reverse('home', locale=settings.LANGUAGE_CODE)
 
         # Verify that _valid_ next parameter is set in form hidden field.
         response = self.client.get(urlparams(reverse('users.login'),

apps/users/views.py

@@ -3,7 +3,6 @@ import urlparse
 
 from django.conf import settings
 from django.contrib import auth
-from django.contrib.auth.decorators import login_required
 from django.contrib.auth.forms import PasswordResetForm, SetPasswordForm
 from django.contrib.auth.models import User
 from django.contrib.auth.tokens import default_token_generator
@@ -15,7 +14,8 @@ from django.utils.http import base36_to_int
 
 import jingo
 
-from sumo.decorators import ssl_required, logout_required
+from access.decorators import logout_required, login_required
+from sumo.decorators import ssl_required
 from sumo.urlresolvers import reverse
 from upload.tasks import _create_image_thumbnail
 from users.backends import Sha256Backend  # Monkey patch User.set_password.
@@ -27,7 +27,7 @@ from users.utils import handle_login, handle_register
 @ssl_required
 def login(request):
     """Try to log the user in."""
-    next_url = _clean_next_url(request) or settings.LOGIN_REDIRECT_URL
+    next_url = _clean_next_url(request) or reverse('home')
     form = handle_login(request)
 
     if request.user.is_authenticated():
@@ -43,7 +43,7 @@ def logout(request):
     auth.logout(request)
     next_url = _clean_next_url(request) if 'next' in request.GET else ''
 
-    return HttpResponseRedirect(next_url or settings.LOGOUT_REDIRECT_URL)
+    return HttpResponseRedirect(next_url or reverse('home'))
 
 
 @ssl_required

apps/wiki/templates/wiki/includes/common_macros.html

@@ -24,11 +24,11 @@
       <nav>
         <a href="{{ profile_url(user) }}">{{ _('Profile') }}</a>
         |
-        <a href="{{ settings.LOGOUT_URL }}">{{ _('Sign Out') }}</a>
+        <a href="{{ url('users.logout') }}">{{ _('Sign Out') }}</a>
       </nav>
     {% else %}
       <p>
-        {% trans login_url=settings.LOGIN_URL, register_url=settings.REGISTER_URL %}
+        {% trans login_url=url('users.login'), register_url=url('users.register') %}
           Want to contribute?<br /> <a href="{{ login_url }}">Sign In</a> or <a href="{{ register_url }}">Register</a>
         {% endtrans %}
       </p>

apps/wiki/tests/test_templates.py

@@ -608,8 +608,10 @@ class ReviewRevisionTests(TestCaseBase):
                         args=[self.document.slug, self.revision.id])
         redirect = response.redirect_chain[0]
         eq_(302, redirect[1])
-        eq_('http://testserver%s?next=/en-US/kb/test-document/review/%s' %
-            (settings.LOGIN_URL, str(self.revision.id)), redirect[0])
+        eq_('http://testserver/%s%s?next=/en-US/kb/test-document/review/%s' %
+            (settings.LANGUAGE_CODE, settings.LOGIN_URL,
+                 str(self.revision.id)),
+            redirect[0])
 
     def test_review_translation(self):
         """Make sure it works for localizations as well."""
@@ -1137,16 +1139,18 @@ class RevisionDeleteTestCase(TestCaseBase):
                        args=[self.d.slug, self.r.id])
         redirect = response.redirect_chain[0]
         eq_(302, redirect[1])
-        eq_('http://testserver%s?next=/en-US/kb/%s/revision/%s/delete' %
-            (settings.LOGIN_URL, self.d.slug, self.r.id),
+        eq_('http://testserver/%s%s?next=/en-US/kb/%s/revision/%s/delete' %
+            (settings.LANGUAGE_CODE, settings.LOGIN_URL, self.d.slug,
+                self.r.id),
             redirect[0])
 
         response = post(self.client, 'wiki.delete_revision',
                         args=[self.d.slug, self.r.id])
         redirect = response.redirect_chain[0]
         eq_(302, redirect[1])
-        eq_('http://testserver%s?next=/en-US/kb/%s/revision/%s/delete' %
-            (settings.LOGIN_URL, self.d.slug, self.r.id),
+        eq_('http://testserver/%s%s?next=/en-US/kb/%s/revision/%s/delete' %
+            (settings.LANGUAGE_CODE, settings.LOGIN_URL, self.d.slug,
+                self.r.id),
             redirect[0])
 
     def test_delete_revision_with_permissions(self):

apps/wiki/views.py

@@ -4,7 +4,6 @@ import logging
 from string import ascii_letters
 
 from django.conf import settings
-from django.contrib.auth.decorators import login_required
 from django.core.exceptions import PermissionDenied
 from django.http import (HttpResponse, HttpResponseRedirect,
                          Http404, HttpResponseBadRequest)
@@ -17,7 +16,7 @@ from taggit.models import Tag
 from tower import ugettext_lazy as _lazy
 from tower import ugettext as _
 
-from access.decorators import permission_required
+from access.decorators import permission_required, login_required
 from notifications import create_watch, destroy_watch
 from sumo.helpers import urlparams
 from sumo.urlresolvers import reverse