Merge pull request #2298 from diox/detail-api-unlisted_and_deleted

Allow reviewers/authors to see deleted/unlisted add-ons in detail API

src/olympia/addons/tests/test_views.py

@@ -1514,9 +1514,9 @@ class TestMobileDetails(TestPersonas, TestMobile):
         assert response.status_code == 301
 
 
-class TestAddonViewSet(TestCase):
+class TestAddonViewSetDetail(TestCase):
     def setUp(self):
-        super(TestAddonViewSet, self).setUp()
+        super(TestAddonViewSetDetail, self).setUp()
         self.addon = addon_factory(
             guid='{%s}' % uuid.uuid4(), name=u'My Addôn', slug='my-addon')
         self.url = reverse('addon-detail', kwargs={'pk': self.addon.pk})
@@ -1580,14 +1580,75 @@ class TestAddonViewSet(TestCase):
         assert response.status_code == 401
 
     def test_get_not_listed(self):
-        # At the moment this API only works with listed addons.
         self.addon.update(is_listed=False)
         response = self.client.get(self.url)
+        assert response.status_code == 401
+
+    def test_get_not_listed_no_rights(self):
+        user = UserProfile.objects.create(username='simpleuser')
+        self.addon.update(is_listed=False)
+        self.client.login_api(user)
+        response = self.client.get(self.url)
+        assert response.status_code == 403
+
+    def test_get_not_listed_simple_reviewer(self):
+        user = UserProfile.objects.create(username='reviewer')
+        self.grant_permission(user, 'Addons:Review')
+        self.addon.update(is_listed=False)
+        self.client.login_api(user)
+        response = self.client.get(self.url)
+        assert response.status_code == 403
+
+    def test_get_not_listed_specific_reviewer(self):
+        user = UserProfile.objects.create(username='reviewer')
+        self.grant_permission(user, 'Addons:ReviewUnlisted')
+        self.addon.update(is_listed=False)
+        self.client.login_api(user)
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+
+    def test_get_not_listed_author(self):
+        user = UserProfile.objects.create(username='author')
+        AddonUser.objects.create(user=user, addon=self.addon)
+        self.addon.update(is_listed=False)
+        self.client.login_api(user)
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+
+    def test_get_deleted(self):
+        self.addon.delete()
+        response = self.client.get(self.url)
         assert response.status_code == 404
 
-    def test_get_deleted(self):
-        # At the moment this API only works with non-deleted addons.
+    def test_get_deleted_no_rights(self):
         self.addon.delete()
+        user = UserProfile.objects.create(username='simpleuser')
+        self.client.login_api(user)
+        response = self.client.get(self.url)
+        assert response.status_code == 404
+
+    def test_get_deleted_reviewer(self):
+        user = UserProfile.objects.create(username='reviewer')
+        self.grant_permission(user, 'Addons:Review')
+        self.addon.delete()
+        self.client.login_api(user)
+        response = self.client.get(self.url)
+        assert response.status_code == 404
+
+    def test_get_deleted_admin(self):
+        user = UserProfile.objects.create(username='admin')
+        self.grant_permission(user, '*:*')
+        self.addon.delete()
+        self.client.login_api(user)
+        response = self.client.get(self.url)
+        assert response.status_code == 200
+
+    def test_get_deleted_author(self):
+        # Owners can't see their own add-on once deleted, only admins can.
+        user = UserProfile.objects.create(username='author')
+        AddonUser.objects.create(user=user, addon=self.addon)
+        self.addon.delete()
+        self.client.login_api(user)
         response = self.client.get(self.url)
         assert response.status_code == 404
 

src/olympia/addons/views.py

@@ -43,7 +43,8 @@ from olympia.bandwagon.models import (
 from olympia import paypal
 from olympia.api.paginator import ESPageNumberPagination
 from olympia.api.permissions import (
-    AllowAddonAuthor, AllowReadOnlyIfPublic, AllowReviewer, AnyOf)
+    AllowAddonAuthor, AllowReadOnlyIfPublicAndListed, AllowReviewer,
+    AllowReviewerUnlisted, AnyOf)
 from olympia.reviews.forms import ReviewForm
 from olympia.reviews.models import Review, GroupedRating
 from olympia.search.filters import (
@@ -650,16 +651,25 @@ def persona_redirect(request, persona_id):
 
 class AddonViewSet(RetrieveModelMixin, GenericViewSet):
     permission_classes = [
-        AnyOf(AllowReadOnlyIfPublic, AllowAddonAuthor, AllowReviewer),
+        AnyOf(AllowReadOnlyIfPublicAndListed, AllowAddonAuthor,
+              AllowReviewer, AllowReviewerUnlisted),
     ]
     serializer_class = AddonSerializer
     addon_id_pattern = re.compile(r'^(\{.*\}|.*@.*)$')
-    # Permission classes disallow access to non-public add-ons unless logged in
-    # as a reviewer/addon owner/admin, so the default queryset is good here.
-    # FIXME: adjust permission classes to make deleted & unlisted addons work.
-    queryset = Addon.objects.all()
+    # Permission classes disallow access to non-public/unlisted add-ons unless
+    # logged in as a reviewer/addon owner/admin, so the unfiltered queryset
+    # is fine here.
+    queryset = Addon.with_unlisted.all()
     lookup_value_regex = '[^/]+'  # Allow '.' for email-like guids.
 
+    def get_queryset(self):
+        # Special case: admins - and only admins - can see deleted add-ons.
+        # This is handled outside a permission class because that condition
+        # would pollute all other classes otherwise.
+        if self.request.user.is_authenticated() and self.request.user.is_staff:
+            return Addon.unfiltered.all()
+        return super(AddonViewSet, self).get_queryset()
+
     def get_object(self):
         value = self.kwargs.get('pk')
         if value and not value.isdigit():

src/olympia/api/permissions.py

@@ -74,7 +74,7 @@ class AllowReviewer(BasePermission):
     The user logged in must either be making a read-only request and have the
     'ReviewerTools:View' permission, or simply be a reviewer or admin.
 
-    An addons reviewer is someone who is in the group with the following
+    An add-on reviewer is someone who is in the group with the following
     permission: 'Addons:Review'.
     """
     def has_permission(self, request, view):
@@ -83,16 +83,35 @@ class AllowReviewer(BasePermission):
                 acl.check_addons_reviewer(request))
 
     def has_object_permission(self, request, view, obj):
-        return self.has_permission(request, view)
+        return obj.is_listed and self.has_permission(request, view)
 
 
-class AllowReadOnlyIfPublic(BasePermission):
+class AllowReviewerUnlisted(AllowReviewer):
+    """Allow unlisted addons reviewer access.
+
+    Like editors.decorators.unlisted_addons_reviewer_required, but as a
+    permission class and not a decorator.
+
+    The user logged in must an unlisted add-on reviewer or admin.
+
+    An unlisted add-on reviewer is someone who is in the group with the
+    following permission: 'Addons:Review'.
     """
-    Allow access when the object's is_public() method returns True and the
-    request HTTP method is GET/OPTIONS/HEAD.
+    def has_permission(self, request, view):
+        return acl.check_unlisted_addons_reviewer(request)
+
+    def has_object_permission(self, request, view, obj):
+        return not obj.is_listed and self.has_permission(request, view)
+
+
+class AllowReadOnlyIfPublicAndListed(BasePermission):
+    """
+    Allow access when the object's is_public() method and is_listed property
+    both return True and the request HTTP method is GET/OPTIONS/HEAD.
     """
     def has_permission(self, request, view):
         return request.method in SAFE_METHODS
 
     def has_object_permission(self, request, view, obj):
-        return obj.is_public() and self.has_permission(request, view)
+        return (obj.is_public() and obj.is_listed and
+                self.has_permission(request, view))

src/olympia/api/tests/test_permissions.py

@@ -10,8 +10,8 @@ from rest_framework.views import APIView
 from olympia.access.models import Group, GroupUser
 from olympia.addons.models import Addon
 from olympia.api.permissions import (
-    AllowAddonAuthor, AllowReadOnlyIfPublic, AllowReviewer, AnyOf,
-    GroupPermission)
+    AllowAddonAuthor, AllowReadOnlyIfPublicAndListed, AllowReviewer,
+    AllowReviewerUnlisted, AnyOf, GroupPermission)
 from olympia.amo.tests import TestCase, WithDynamicEndpoints
 from olympia.users.models import UserProfile
 
@@ -227,9 +227,53 @@ class TestAllowReviewer(TestCase):
                 request, myview, Mock())
 
 
-class TestAllowReadOnlyIfPublic(TestCase):
+class TestAllowUnlistedReviewer(TestCase):
+    fixtures = ['base/users']
+
+    # Note: be careful when testing, under the hood we're using a method that
+    # relies on UserProfile.groups_list, which is cached on the UserProfile
+    # instance.
     def setUp(self):
-        self.permission = AllowReadOnlyIfPublic()
+        self.permission = AllowReviewerUnlisted()
+        self.request = RequestFactory().get('/')
+
+    def test_user_cannot_be_anonymous(self):
+        self.request.user = AnonymousUser()
+        obj = Mock()
+        obj.is_listed = False
+        assert not self.permission.has_permission(self.request, myview)
+        assert not self.permission.has_object_permission(
+            self.request, myview, obj)
+
+    def test_authenticated_but_not_reviewer(self):
+        self.request.user = UserProfile.objects.get(pk=999)
+        obj = Mock()
+        obj.is_listed = False
+        assert not self.permission.has_permission(self.request, myview)
+        assert not self.permission.has_object_permission(
+            self.request, myview, obj)
+
+    def test_admin(self):
+        self.request.user = UserProfile.objects.get(email='admin@mozilla.com')
+        obj = Mock()
+        obj.is_listed = False
+
+        assert self.permission.has_permission(self.request, myview)
+        assert self.permission.has_object_permission(self.request, myview, obj)
+
+    def test_unlisted_reviewer(self):
+        self.request.user = UserProfile.objects.get(
+            email='senioreditor@mozilla.com')
+        obj = Mock()
+        obj.is_listed = False
+
+        assert self.permission.has_permission(self.request, myview)
+        assert self.permission.has_object_permission(self.request, myview, obj)
+
+
+class TestAllowReadOnlyIfPublicAndListed(TestCase):
+    def setUp(self):
+        self.permission = AllowReadOnlyIfPublicAndListed()
         self.request_factory = RequestFactory()
         self.unsafe_methods = ('patch', 'post', 'put', 'delete')
         self.safe_methods = ('get', 'options', 'head')
@@ -249,6 +293,7 @@ class TestAllowReadOnlyIfPublic(TestCase):
     def test_has_object_permission_public(self):
         obj = Mock()
         obj.is_public.return_value = True
+        obj.is_listed = True
 
         for verb in self.safe_methods:
             assert self.permission.has_object_permission(
@@ -261,6 +306,25 @@ class TestAllowReadOnlyIfPublic(TestCase):
     def test_has_object_permission_not_public(self):
         obj = Mock()
         obj.is_public.return_value = False
+        obj.is_listed = True
+
+        for verb in self.unsafe_methods + self.safe_methods:
+            assert not self.permission.has_object_permission(
+                self.request(verb), myview, obj)
+
+    def test_has_object_permission_not_listed(self):
+        obj = Mock()
+        obj.is_public.return_value = True
+        obj.is_listed = False
+
+        for verb in self.unsafe_methods + self.safe_methods:
+            assert not self.permission.has_object_permission(
+                self.request(verb), myview, obj)
+
+    def test_has_object_permission_not_listed_nor_public(self):
+        obj = Mock()
+        obj.is_public.return_value = False
+        obj.is_listed = False
 
         for verb in self.unsafe_methods + self.safe_methods:
             assert not self.permission.has_object_permission(