From 5a947024e524c0938e6d2e454430131425523082 Mon Sep 17 00:00:00 2001 From: James Socol Date: Wed, 13 Oct 2010 18:04:07 -0400 Subject: [PATCH] New @permission_required decorator that sends 403s sometimes. [bug 603500] --- apps/access/decorators.py | 26 ++++++++++++++- apps/access/tests.py | 19 +++++++++-- apps/flagit/views.py | 4 +-- apps/questions/tests/test_templates.py | 38 ++++++++++++++++++++-- apps/questions/views.py | 45 +++++++++++++------------- apps/wiki/tests/test_templates.py | 23 ++++++++++++- apps/wiki/views.py | 3 +- 7 files changed, 125 insertions(+), 33 deletions(-) diff --git a/apps/access/decorators.py b/apps/access/decorators.py index 4deaf8604..84f04630d 100644 --- a/apps/access/decorators.py +++ b/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 diff --git a/apps/access/tests.py b/apps/access/tests.py index bb1cd55da..985346760 100644 --- a/apps/access/tests.py +++ b/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) diff --git a/apps/flagit/views.py b/apps/flagit/views.py index f7c23700e..28905d843 100644 --- a/apps/flagit/views.py +++ b/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 diff --git a/apps/questions/tests/test_templates.py b/apps/questions/tests/test_templates.py index 648bd44f0..3b93e2fc4 100644 --- a/apps/questions/tests/test_templates.py +++ b/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] diff --git a/apps/questions/views.py b/apps/questions/views.py index 4ab0c8159..afc372813 100644 --- a/apps/questions/views.py +++ b/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 diff --git a/apps/wiki/tests/test_templates.py b/apps/wiki/tests/test_templates.py index 13c690b00..ed86a8de1 100644 --- a/apps/wiki/tests/test_templates.py +++ b/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): diff --git a/apps/wiki/views.py b/apps/wiki/views.py index 5e0820e5d..918faa1fa 100644 --- a/apps/wiki/views.py +++ b/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