Merge pull request kobotoolbox#2530 from kobotoolbox/2529-denied-perm…

…issions-exposed

Hide denied permissions and allow assignment of (all) denied permissions to `AnonymousUser`

kpi/models/object_permission.py

@@ -728,11 +728,15 @@ def assign_perm(self, user_obj, perm, deny=False, defer_recalc=False,
             user_obj.pk == settings.ANONYMOUS_USER_ID
         ):
             # Is an anonymous user allowed to have this permission?
-            fq_permission = '{}.{}'.format(app_label, codename)
-            if fq_permission not in settings.ALLOWED_ANONYMOUS_PERMISSIONS:
+            fq_permission = f'{app_label}.{codename}'
+            if (
+                not deny
+                and fq_permission not in settings.ALLOWED_ANONYMOUS_PERMISSIONS
+            ):
                 raise ValidationError(
-                    'Anonymous users cannot have the permission {}.'.format(
-                        codename)
+                    'Anonymous users cannot be granted the permission {}.'.format(
+                        codename
+                    )
                 )
             # Get the User database representation for AnonymousUser
             user_obj = get_anonymous_user()

kpi/tests/api/v2/test_api_permissions.py

@@ -86,6 +86,36 @@ def test_public_asset_not_in_list_admin(self):
         self.assert_object_in_object_list(self.someusers_public_asset, self.admin, self.admin_password,
                                           in_list=False)
 
+    def test_revoke_anon_from_asset_in_public_collection(self):
+        self.login(self.someuser.username, self.someuser_password)
+        public_collection = self.create_collection('public_collection')
+        child_asset = self.create_asset('child_asset_in_public_collection')
+        self.add_to_collection(child_asset, public_collection)
+        child_asset.refresh_from_db()
+
+        # Anon should have no access at this point
+        self.client.logout()
+        self.assert_viewable(child_asset, viewable=False)
+
+        # Grant anon access to the parent collection
+        self.login(self.someuser.username, self.someuser_password)
+        self.add_perm(public_collection, self.anon, 'view_')
+
+        # Verify anon can access the child asset
+        self.client.logout()
+        # Anon can only see a public asset by accessing the detail view
+        # directly; `assert_viewble()` will always fail because it expects the
+        # asset to be in the list view as well
+        self.assert_detail_viewable(child_asset)
+
+        # Revoke anon's access to the child asset
+        self.login(self.someuser.username, self.someuser_password)
+        self.remove_perm_v2_api(child_asset, self.anon,'view_asset')
+
+        # Make sure anon cannot access the child asset any longer
+        self.client.logout()
+        self.assert_viewable(child_asset, viewable=False)
+
 
 class ApiPermissionsTestCase(KpiTestCase):
     fixtures = ['test_data']

kpi/tests/kpi_test_case.py

@@ -197,6 +197,26 @@ def remove_perm(self, obj, owner, owner_password, other_user,
         # assigned yet and fails when it has.
         self._test_remove_perm(obj, perm_name_prefix, other_user)
 
+    def remove_perm_v2_api(self, asset, user, perm_codename):
+        # TODO: replace `remove_perm()` with this method
+
+        if not isinstance(asset, Asset):
+            raise NotImplementedError
+
+        list_url = reverse(
+            'api_v2:asset-permission-assignment-list', args=[asset.uid]
+        )
+        perm_list = self.client.get(list_url).data
+        for perm in perm_list:
+            # Is there there no simple way to resolve a URL to an object?
+            user_detail = self.client.get(perm['user']).data
+            if user_detail['username'] == user.username:
+                perm_detail = self.client.get(perm['permission']).data
+                if perm_detail['codename'] == perm_codename:
+                    break
+        response = self.client.delete(perm['url'])
+        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+
     def assert_object_in_object_list(self, obj, user=None, password=None,
                                      in_list=True, msg=None):
         view_name = obj._meta.model_name + '-list'

kpi/utils/object_permission_helper.py

@@ -47,7 +47,7 @@ def get_user_permission_assignments_queryset(cls, affected_object, user):
         # `affected_object.permissions` is a `GenericRelation(ObjectPermission)`
         # Don't Prefetch `content_object`.
         # See `AssetPermissionAssignmentSerializer.to_representation()`
-        queryset = affected_object.permissions.select_related(
+        queryset = affected_object.permissions.filter(deny=False).select_related(
             'permission', 'user'
         ).order_by(
                 'user__username', 'permission__codename'

kpi/views/v2/asset.py

@@ -231,7 +231,8 @@ def get_serializer_context(self):
             asset_ids = self.filter_queryset(queryset).values_list('id').distinct()
             object_permissions = ObjectPermission.objects.filter(
                 content_type_id=asset_content_type.pk,
-                object_id__in=asset_ids
+                object_id__in=asset_ids,
+                deny=False,
             ).select_related(
                 'user', 'permission'
             ).order_by(