New @permission_required decorator that sends 403s sometimes. [bug 603500]

apps/access/decorators.py

@@ -1,9 +1,13 @@
 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
+from django.http import HttpResponseForbidden, HttpResponseRedirect
 from django.shortcuts import get_object_or_404
+from django.utils.decorators import available_attrs
+from django.utils.http import urlquote
 
 import access
 
@@ -66,3 +70,23 @@ 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

@@ -4,10 +4,10 @@ from nose.tools import eq_
 import test_utils
 
 import access
-from .helpers import has_perm, has_perm_or_owns
+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
-from forums.models import Forum, Thread
 
 
 class ForumTestPermissions(TestCase):
@@ -161,3 +161,18 @@ class ForumTestPermissions(TestCase):
         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/flagit/views.py

@@ -5,13 +5,13 @@ 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
-from django.contrib.auth.decorators import permission_required
 
 import jingo
 from tower import ugettext as _
 
+from access.decorators import permission_required
+from flagit.models import FlaggedObject
 from sumo.urlresolvers import reverse
-from .models import FlaggedObject
 
 
 @require_POST

apps/questions/tests/test_templates.py

@@ -257,7 +257,18 @@ class AnswersTemplateTestCase(TestCaseBase):
         eq_('0', doc('div.other-helpful span.votes')[0].text)
 
     def test_delete_question_without_permissions(self):
-        """Deleting a question without permissions redirects to login."""
+        """Deleting a question without permissions is a 403."""
+        self.client.login(username='tagger', password='testpass')
+        response = get(self.client, 'questions.delete',
+                       args=[self.question.id])
+        eq_(403, response.status_code)
+        response = post(self.client, 'questions.delete',
+                        args=[self.question.id])
+        eq_(403, response.status_code)
+
+    def test_delete_question_logged_out(self):
+        """Deleting a question while logged out redirects to login."""
+        self.client.logout()
         response = get(self.client, 'questions.delete',
                        args=[self.question.id])
         redirect = response.redirect_chain[0]
@@ -284,7 +295,20 @@ class AnswersTemplateTestCase(TestCaseBase):
         eq_(0, Question.objects.filter(pk=self.question.id).count())
 
     def test_delete_answer_without_permissions(self):
-        """Deleting an answer without permissions redirects to login."""
+        """Deleting an answer without permissions sends 403."""
+        self.client.login(username='tagger', password='testpass')
+        answer = self.question.last_answer
+        response = get(self.client, 'questions.delete_answer',
+                       args=[self.question.id, answer.id])
+        eq_(403, response.status_code)
+
+        response = post(self.client, 'questions.delete_answer',
+                        args=[self.question.id, answer.id])
+        eq_(403, response.status_code)
+
+    def test_delete_answer_logged_out(self):
+        """Deleting an answer while logged out redirects to login."""
+        self.client.logout()
         answer = self.question.last_answer
         response = get(self.client, 'questions.delete_answer',
                        args=[self.question.id, answer.id])
@@ -392,7 +416,15 @@ class AnswersTemplateTestCase(TestCaseBase):
         eq_(403, response.status_code)
 
     def test_lock_question_without_permissions(self):
-        """Trying to lock a question without permission redirects to login."""
+        """Trying to lock a question without permission is a 403."""
+        self.client.login(username='tagger', password='testpass')
+        q = self.question
+        response = post(self.client, 'questions.lock', args=[q.id])
+        eq_(403, response.status_code)
+
+    def test_lock_question_logged_out(self):
+        """Trying to lock a question while logged out redirects to login."""
+        self.client.logout()
         q = self.question
         response = post(self.client, 'questions.lock', args=[q.id])
         redirect = response.redirect_chain[0]

apps/questions/views.py

@@ -1,45 +1,44 @@
+from datetime import datetime
 from itertools import islice
 import json
 import logging
-from datetime import datetime
 
-from django.contrib.auth.decorators import permission_required
-from django.core.exceptions import PermissionDenied, ObjectDoesNotExist
+from django.conf import settings
+from django.contrib.auth.decorators import login_required
 from django.contrib.contenttypes.models import ContentType
+from django.contrib.sites.models import Site
+from django.core.cache import cache
+from django.core.exceptions import PermissionDenied, ObjectDoesNotExist
+from django.db.models import Q
 from django.http import (HttpResponseRedirect, HttpResponse, Http404,
                          HttpResponseBadRequest, HttpResponseForbidden)
 from django.shortcuts import get_object_or_404
 from django.views.decorators.http import require_POST, require_http_methods
-from django.contrib.auth.decorators import login_required
-from django.db.models import Q
-from django.core.cache import cache
-from django.conf import settings
-from django.contrib.sites.models import Site
 
 import jingo
 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
-from search.clients import WikiClient, QuestionsClient, SearchError
-from search.utils import locale_or_default, sphinx_locale
-from sumo.models import WikiPage
-from sumo.urlresolvers import reverse
-from sumo.helpers import urlparams
-from sumo.utils import paginate
+from access.decorators import has_perm_or_owns_or_403, permission_required
 from notifications import create_watch, destroy_watch
-from .models import (Question, Answer, QuestionVote, AnswerVote,
-                     CONFIRMED, UNCONFIRMED)
-from .forms import (NewQuestionForm, EditQuestionForm,
+import questions as constants
+from questions.feeds import QuestionsFeed, AnswersFeed, TaggedQuestionsFeed
+from questions.forms import (NewQuestionForm, EditQuestionForm,
                     AnswerForm, WatchQuestionForm,
                     FREQUENCY_CHOICES)
-from .feeds import QuestionsFeed, AnswersFeed, TaggedQuestionsFeed
-from .tags import add_existing_tag
-from .tasks import (cache_top_contributors, build_solution_notification,
+from questions.models import (Question, Answer, QuestionVote, AnswerVote,
+                     CONFIRMED, UNCONFIRMED)
+from questions.tags import add_existing_tag
+from questions.tasks import (cache_top_contributors, build_solution_notification,
                     send_confirmation_email)
-import questions as constants
-from .question_config import products
+from questions.question_config import products
+from search.clients import WikiClient, QuestionsClient, SearchError
+from search.utils import locale_or_default, sphinx_locale
+from sumo.helpers import urlparams
+from sumo.models import WikiPage
+from sumo.urlresolvers import reverse
+from sumo.utils import paginate
 from upload.models import ImageAttachment
 from upload.views import upload_imageattachment
 

apps/wiki/tests/test_templates.py

@@ -88,7 +88,13 @@ class NewDocumentTests(TestCaseBase):
         """Trying to create a document without permission redirects to login"""
         self.client.login(username='rrosario', password='testpass')
         response = self.client.get(reverse('wiki.new_document'))
-        eq_(302, response.status_code)
+        eq_(403, response.status_code)
+
+    def test_new_document_GET_without_perm(self):
+        """Trying to create a document without permission redirects to login"""
+        self.client.login(username='rrosario', password='testpass')
+        response = self.client.get(reverse('wiki.new_document'))
+        eq_(403, response.status_code)
 
     def test_new_document_GET_with_perm(self):
         """HTTP GET to new document URL renders the form."""
@@ -469,6 +475,14 @@ class ReviewRevisionTests(TestCaseBase):
     def test_review_without_permission(self):
         """Make sure unauthorized users can't review revisions."""
         self.client.login(username='rrosario', password='testpass')
+        response = post(self.client, 'wiki.review_revision',
+                        {'reject': 'Reject Revision'},
+                        args=[self.document.slug, self.revision.id])
+        eq_(403, response.status_code)
+
+    def test_review_logged_out(self):
+        """Make sure logged out users can't review revisions."""
+        self.client.logout()
         response = post(self.client, 'wiki.review_revision',
                         {'reject': 'Reject Revision'},
                         args=[self.document.slug, self.revision.id])
@@ -583,6 +597,13 @@ class TranslateTests(TestCaseBase):
         self.client.login(username='rrosario', password='testpass')
         url = reverse('wiki.translate', locale='es', args=[self.d.slug])
         response = self.client.get(url)
+        eq_(403, response.status_code)
+
+    def test_translate_GET_logged_out(self):
+        """Try to create a translation while logged out."""
+        self.client.logout()
+        url = reverse('wiki.translate', locale='es', args=[self.d.slug])
+        response = self.client.get(url)
         eq_(302, response.status_code)
 
     def test_translate_GET_with_perm(self):

apps/wiki/views.py

@@ -3,7 +3,7 @@ import json
 from string import ascii_letters
 
 from django.conf import settings
-from django.contrib.auth.decorators import permission_required, login_required
+from django.contrib.auth.decorators import login_required
 from django.core.exceptions import PermissionDenied
 from django.http import (HttpResponse, HttpResponseRedirect,
                          Http404, HttpResponseBadRequest)
@@ -14,6 +14,7 @@ from django.views.decorators.http import (require_GET, require_POST,
 import jingo
 from tower import ugettext_lazy as _lazy
 
+from access.decorators import permission_required
 from notifications import create_watch, destroy_watch
 from sumo.helpers import urlparams
 from sumo.urlresolvers import reverse