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,16 +1580,60 @@ 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 == 404
+        assert response.status_code == 401
+
+    def test_get_not_listed_no_rights(self):
+        self.addon.update(is_listed=False)
+        user = UserProfile.objects.create(username='simpleuser')
+        self.client.login_api(user)
+        response = self.client.get(self.url)
+        assert response.status_code == 403
+
+    def test_get_not_listed_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 == 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):
-        # At the moment this API only works with non-deleted addons.
         self.addon.delete()
         response = self.client.get(self.url)
-        assert response.status_code == 404
+        assert response.status_code == 401
+
+    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 == 403
+
+    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 == 200
+
+    def test_get_deleted_author(self):
+        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 == 200
 
     def test_get_not_found(self):
         self.url = reverse('addon-detail', kwargs={'pk': self.addon.pk + 42})

src/olympia/addons/views.py

@@ -43,7 +43,7 @@ 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, AnyOf)
 from olympia.reviews.forms import ReviewForm
 from olympia.reviews.models import Review, GroupedRating
 from olympia.search.filters import (
@@ -650,14 +650,14 @@ def persona_redirect(request, persona_id):
 
 class AddonViewSet(RetrieveModelMixin, GenericViewSet):
     permission_classes = [
-        AnyOf(AllowReadOnlyIfPublic, AllowAddonAuthor, AllowReviewer),
+        AnyOf(AllowReadOnlyIfPublicAndListed, AllowAddonAuthor, AllowReviewer),
     ]
     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 (deleted add-ons are not considered public).
+    queryset = Addon.unfiltered.all()
     lookup_value_regex = '[^/]+'  # Allow '.' for email-like guids.
 
     def get_object(self):

src/olympia/api/permissions.py

@@ -86,13 +86,14 @@ class AllowReviewer(BasePermission):
         return self.has_permission(request, view)
 
 
-class AllowReadOnlyIfPublic(BasePermission):
+class AllowReadOnlyIfPublicAndListed(BasePermission):
     """
-    Allow access when the object's is_public() method returns True and the
-    request HTTP method is GET/OPTIONS/HEAD.
+    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,7 +10,7 @@ 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,
+    AllowAddonAuthor, AllowReadOnlyIfPublicAndListed, AllowReviewer, AnyOf,
     GroupPermission)
 from olympia.amo.tests import TestCase, WithDynamicEndpoints
 from olympia.users.models import UserProfile
@@ -227,9 +227,9 @@ class TestAllowReviewer(TestCase):
                 request, myview, Mock())
 
 
-class TestAllowReadOnlyIfPublic(TestCase):
+class TestAllowReadOnlyIfPublicAndListed(TestCase):
     def setUp(self):
-        self.permission = AllowReadOnlyIfPublic()
+        self.permission = AllowReadOnlyIfPublicAndListed()
         self.request_factory = RequestFactory()
         self.unsafe_methods = ('patch', 'post', 'put', 'delete')
         self.safe_methods = ('get', 'options', 'head')
@@ -249,6 +249,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 +262,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(