Added 403 handlers (bug 584442)

apps/addons/decorators.py

@@ -1,6 +1,7 @@
 import functools
 
 from django import http
+from django.core.exceptions import PermissionDenied
 from django.shortcuts import get_object_or_404
 
 import waffle
@@ -54,7 +55,7 @@ def wrapper(request, addon, *args, **kw):
             log.error('Marketplace waffle switch is off')
             raise http.Http404
         if not addon.can_be_purchased():
-            return http.HttpResponseForbidden()
+            raise PermissionDenied
         return f(request, addon, *args, **kw)
     return wrapper
 
@@ -67,7 +68,7 @@ def has_purchased(f):
     @functools.wraps(f)
     def wrapper(request, addon, *args, **kw):
         if addon.is_premium() and not addon.has_purchased(request.amo_user):
-            return http.HttpResponseForbidden()
+            raise PermissionDenied
         return f(request, addon, *args, **kw)
     return wrapper
 
@@ -81,7 +82,7 @@ def has_purchased_or_refunded(f):
     def wrapper(request, addon, *args, **kw):
         if addon.is_premium() and not (addon.has_purchased(request.amo_user) or
                                        addon.is_refunded(request.amo_user)):
-            return http.HttpResponseForbidden()
+            raise PermissionDenied
         return f(request, addon, *args, **kw)
     return wrapper
 
@@ -91,7 +92,7 @@ def has_not_purchased(f):
     @functools.wraps(f)
     def wrapper(request, addon, *args, **kw):
         if addon.is_premium() and addon.has_purchased(request.amo_user):
-            return http.HttpResponseForbidden()
+            raise PermissionDenied
         return f(request, addon, *args, **kw)
     return wrapper
 
@@ -101,6 +102,6 @@ def can_become_premium(f):
     @functools.wraps(f)
     def wrapper(request, addon_id, addon, *args, **kw):
         if not addon.can_become_premium():
-            return http.HttpResponseForbidden()
+            raise PermissionDenied
         return f(request, addon_id, addon, *args, **kw)
     return wrapper

apps/addons/tests/test_decorators.py

@@ -1,4 +1,5 @@
 from django import http
+from django.core.exceptions import PermissionDenied
 
 import mock
 from nose.tools import eq_
@@ -105,8 +106,8 @@ def setUp(self):
     def test_cant_become_premium(self):
         self.addon.can_become_premium.return_value = False
         view = dec.can_become_premium(self.func)
-        res = view(self.request, self.addon.pk, self.addon)
-        eq_(res.status_code, 403)
+        with self.assertRaises(PermissionDenied):
+            view(self.request, self.addon.pk, self.addon)
 
     def test_can_become_premium(self):
         self.addon.can_become_premium.return_value = True
@@ -123,8 +124,8 @@ def test_has_purchased_failure(self):
         view = dec.has_purchased(self.func)
         self.addon.is_premium.return_value = True
         self.addon.has_purchased.return_value = False
-        res = view(self.request, self.addon)
-        eq_(res.status_code, 403)
+        with self.assertRaises(PermissionDenied):
+            view(self.request, self.addon)
 
     def test_has_not_purchased(self):
         view = dec.has_not_purchased(self.func)
@@ -136,5 +137,5 @@ def test_has_not_purchased_failure(self):
         view = dec.has_not_purchased(self.func)
         self.addon.is_premium.return_value = True
         self.addon.has_purchased.return_value = True
-        res = view(self.request, self.addon)
-        eq_(res.status_code, 403)
+        with self.assertRaises(PermissionDenied):
+            view(self.request, self.addon)

apps/addons/tests/test_models.py

@@ -1334,7 +1334,7 @@ def test_featured_random(self):
 
 
 class TestBackupVersion(amo.tests.TestCase):
-    fixtures = ['addons/update']
+    fixtures = ['addons/update', 'base/platforms']
 
     def setUp(self):
         self.version_1_2_0 = 105387

apps/amo/decorators.py

@@ -3,6 +3,7 @@
 
 from django import http
 from django.conf import settings
+from django.core.exceptions import PermissionDenied
 from django.utils.http import urlquote
 
 import commonware.log
@@ -60,7 +61,7 @@ def wrapper(request, *args, **kw):
             if acl.action_allowed(request, app, action):
                 return f(request, *args, **kw)
             else:
-                return http.HttpResponseForbidden()
+                raise PermissionDenied
         return wrapper
     return decorator
 
@@ -77,7 +78,7 @@ def wrapper(request, *args, **kw):
             for app, action in pairs:
                 if acl.action_allowed(request, app, action):
                     return f(request, *args, **kw)
-            return http.HttpResponseForbidden()
+            raise PermissionDenied
         return wrapper
     return decorator
 
@@ -94,7 +95,7 @@ def wrapper(request, *args, **kw):
             or not acl.action_allowed(request, 'Restricted', 'UGC')):
             return f(request, *args, **kw)
         else:
-            return http.HttpResponseForbidden()
+            raise PermissionDenied
     return wrapper
 
 

apps/amo/templates/amo/403.html

@@ -1,6 +1,6 @@
 {% extends "base_modal.html" if request.is_ajax() else "impala/base_shared.html" %}
 
-{% block title %}{{ _('Oops') }}{% endblock %}
+{% block title %}{{ _('Not allowed') }}{% endblock %}
 
 {% block content %}
   <section class="primary">

apps/amo/tests/test_decorators.py

@@ -1,5 +1,7 @@
 from datetime import datetime, timedelta
+
 from django import http
+from django.core.exceptions import PermissionDenied
 
 import mock
 from nose import SkipTest
@@ -150,7 +152,8 @@ def setUp(self):
     def test_permission_not_allowed(self, action_allowed):
         action_allowed.return_value = False
         func = decorators.permission_required('', '')(self.f)
-        eq_(func(self.request).status_code, 403)
+        with self.assertRaises(PermissionDenied):
+            func(self.request)
 
     @mock.patch('access.acl.action_allowed')
     def test_permission_allowed(self, action_allowed):

apps/amo/tests/test_views.py

@@ -24,6 +24,24 @@
 from users.models import UserProfile
 
 
+class Test403(amo.tests.TestCase):
+    fixtures = ['base/users']
+
+    def setUp(self):
+        assert self.client.login(username='regular@mozilla.com',
+                                 password='password')
+
+    def test_403_no_app(self):
+        response = self.client.get('/en-US/admin/')
+        eq_(response.status_code, 403)
+        self.assertTemplateUsed(response, 'amo/403.html')
+
+    def test_403_app(self):
+        response = self.client.get('/en-US/thunderbird/admin/', follow=True)
+        eq_(response.status_code, 403)
+        self.assertTemplateUsed(response, 'amo/403.html')
+
+
 class Test404(amo.tests.TestCase):
 
     def test_404_no_app(self):

apps/amo/views.py

@@ -3,6 +3,7 @@
 
 from django import http
 from django.conf import settings
+from django.core.exceptions import PermissionDenied
 from django.http import HttpResponse, HttpResponseBadRequest
 from django.utils.encoding import iri_to_uri
 from django.views.decorators.cache import never_cache
@@ -77,16 +78,25 @@ def robots(request):
     return HttpResponse(template, mimetype="text/plain")
 
 
+def handler403(request):
+    if request.path_info.startswith('/api/'):
+        # Pass over to handler403 view in api if api was targeted.
+        return api.views.handler403(request)
+    else:
+        return jingo.render(request, 'amo/403.html', status=403)
+
+
 def handler404(request):
     if request.path_info.startswith('/api/'):
-        # Pass over to handler404 view in api if api was targeted
+        # Pass over to handler404 view in api if api was targeted.
         return api.views.handler404(request)
     else:
         return jingo.render(request, 'amo/404.html', status=404)
 
 
 def handler500(request):
     if request.path_info.startswith('/api/'):
+        # Pass over to handler500 view in api if api was targeted.
         return api.views.handler500(request)
     else:
         return jingo.render(request, 'amo/500.html', status=500)
@@ -156,7 +166,7 @@ def record(request):
     # we can just turn the percentage down to zero.
     if get_collect_timings():
         return django_statsd_record(request)
-    return http.HttpResponseForbidden()
+    raise PermissionDenied
 
 
 def plugin_check_redirect(request):

apps/api/tests/test_views.py

@@ -208,7 +208,7 @@ def test_handler404(self):
         """
         Check separate handler404 response for API.
         """
-        response = self.client.get('/en-us/firefox/api/nonsense')
+        response = self.client.get('/en-US/firefox/api/nonsense')
         doc = pq(response.content)
         eq_(response.status_code, 404)
         d = doc('error')

apps/api/views.py

@@ -86,6 +86,11 @@ def render_xml(request, template, context={}, **kwargs):
     return HttpResponse(rendered, **kwargs)
 
 
+def handler403(request):
+    context = {'error_level': ERROR, 'msg': 'Not allowed'}
+    return render_xml(request, 'api/message.xml', context, status=403)
+
+
 def handler404(request):
     context = {'error_level': ERROR, 'msg': 'Not Found'}
     return render_xml(request, 'api/message.xml', context, status=404)

apps/bandwagon/views.py

@@ -2,6 +2,7 @@
 import os
 
 from django import http
+from django.core.exceptions import PermissionDenied
 from django.db.models import Q
 from django.shortcuts import get_object_or_404, redirect
 
@@ -50,7 +51,7 @@ def wrapper(request, username, slug, *args, **kw):
                                               require_owner=require_owner):
                 return func(request, collection, username, slug, *args, **kw)
             else:
-                return http.HttpResponseForbidden()
+                raise PermissionDenied
         return wrapper
     return decorator(f) if f else decorator
 
@@ -179,7 +180,7 @@ def filter(self, field):
 def collection_detail(request, username, slug):
     c = get_collection(request, username, slug)
     if not (c.listed or acl.check_collection_ownership(request, c)):
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
 
     if request.GET.get('format') == 'rss':
         return http.HttpResponsePermanentRedirect(c.feed_url())
@@ -222,7 +223,7 @@ def collection_detail(request, username, slug):
 def collection_detail_json(request, username, slug):
     c = get_collection(request, username, slug)
     if not (c.listed or acl.check_collection_ownership(request, c)):
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
 
     addons = c.addons.valid()
     addons_dict = [addon_to_dict(a) for a in addons]
@@ -375,7 +376,7 @@ def collection_alter(request, username, slug, action):
 
 def change_addon(request, collection, action):
     if not acl.check_collection_ownership(request, collection):
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
 
     try:
         addon = get_object_or_404(Addon.objects, pk=request.POST['addon_id'])
@@ -513,7 +514,7 @@ def delete(request, username, slug):
     if not acl.check_collection_ownership(request, collection, True):
         log.info(u'%s is trying to delete collection %s'
                  % (request.amo_user, collection.id))
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
 
     data = dict(collection=collection, username=username, slug=slug)
 

apps/devhub/decorators.py

@@ -1,6 +1,7 @@
 import functools
 
-from django import http
+from django.core.exceptions import PermissionDenied
+
 import waffle
 
 from amo.decorators import login_required
@@ -40,7 +41,7 @@ def wrapper(request, addon, *args, **kw):
                 if not getattr(f, 'submitting', False) and step:
                     return _resume(addon, step)
                 return fun()
-            return http.HttpResponseForbidden()
+            raise PermissionDenied
         return wrapper
     # The arg will be a function if they didn't pass owner_for_post.
     if callable(owner_for_post):

apps/devhub/views.py

@@ -8,10 +8,11 @@
 import uuid
 
 from django import http
+from django.core.exceptions import PermissionDenied
 from django.core.files.storage import default_storage as storage
 from django.conf import settings
 from django import forms as django_forms
-from django.db import models, transaction
+from django.db import transaction
 from django.db.models import Count
 from django.shortcuts import get_object_or_404, redirect
 from django.utils.http import urlquote
@@ -264,7 +265,7 @@ def feed(request, addon_id=None):
 
             if not acl.check_addon_ownership(request, addons, viewer=True,
                                              ignore_disabled=True):
-                return http.HttpResponseForbidden()
+                raise PermissionDenied
         else:
             rssurl = _get_rss_feed(request)
             addon = None
@@ -637,7 +638,7 @@ def check_addon_compatibility(request):
 @json_view
 def file_perf_tests_start(request, addon_id, addon, file_id):
     if not waffle.flag_is_active(request, 'perf-tests'):
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
     file_ = get_object_or_404(File, pk=file_id)
 
     plats = perf.PLATFORM_MAP.get(file_.platform.id, None)
@@ -1684,7 +1685,7 @@ def _resume(addon, step):
 @dev_required
 def submit_bump(request, addon_id, addon, webapp=False):
     if not acl.action_allowed(request, 'Admin', 'EditSubmitStep'):
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
     step = SubmitStep.objects.filter(addon=addon)
     step = step[0] if step else None
     if request.method == 'POST' and request.POST.get('step'):
@@ -1702,7 +1703,7 @@ def submit_bump(request, addon_id, addon, webapp=False):
 @login_required
 def submit_persona(request):
     if not waffle.flag_is_active(request, 'submit-personas'):
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
     form = addon_forms.NewPersonaForm(data=request.POST or None,
                                       files=request.FILES or None,
                                       request=request)
@@ -1716,7 +1717,7 @@ def submit_persona(request):
 @dev_required
 def submit_persona_done(request, addon_id, addon):
     if not waffle.flag_is_active(request, 'submit-personas'):
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
     if addon.is_public():
         return redirect(addon.get_url_path())
     return jingo.render(request, 'devhub/personas/submit_done.html',
@@ -1772,7 +1773,7 @@ def validator_redirect(request, version_id):
 @addon_view
 def admin(request, addon):
     if not acl.action_allowed(request, 'Addons', 'Configure'):
-        return http.HttpResponseForbidden()
+        raise PermissionDenied
     form = forms.AdminForm(request, request.POST or None, instance=addon)
     if form.is_valid():
         form.save()