Merge branch 'worker-version-permissions' into 'master'

Handle corpus rights and read-only access on worker version endpoints

Closes #620 and #619

See merge request !1181

arkindex/dataimport/api.py

@@ -51,7 +51,7 @@ from arkindex.documents.models import ClassificationState, Corpus, Element, Elem
 from arkindex.project.fields import ArrayRemove
 from arkindex.project.mixins import CorpusACLMixin, CustomPaginationViewMixin, DeprecatedMixin, SelectionMixin
 from arkindex.project.openapi import AutoSchema
-from arkindex.project.permissions import IsVerified
+from arkindex.project.permissions import IsVerified, IsVerifiedOrReadOnly
 from arkindex.users.models import OAuthCredentials, Role, User
 from arkindex_common.enums import DataImportMode
 from ponos.models import STATES_ORDERING, State
@@ -744,7 +744,7 @@ class CorpusWorkerVersionList(CorpusACLMixin, ListAPIView):
     """
     List worker versions used by elements of a given corpus.
     """
-    permission_classes = (IsVerified, )
+    permission_classes = (IsVerifiedOrReadOnly, )
     pagination_class = None
     serializer_class = WorkerVersionSerializer
     openapi_overrides = {
@@ -754,7 +754,7 @@ class CorpusWorkerVersionList(CorpusACLMixin, ListAPIView):
 
     def get_queryset(self):
         return WorkerVersion.objects \
-            .filter(elements__corpus_id=self.kwargs['pk']) \
+            .filter(elements__corpus=self.get_corpus(self.kwargs['pk'])) \
             .select_related('revision__repo', 'worker__repository') \
             .prefetch_related('revision__refs', 'revision__versions') \
             .order_by('-revision__created') \
@@ -765,7 +765,7 @@ class WorkerVersionRetrieve(CorpusACLMixin, RetrieveUpdateAPIView):
     """
     Retrieve a specific worker version
     """
-    permission_classes = (IsVerified, )
+    permission_classes = (IsVerifiedOrReadOnly, )
     serializer_class = WorkerVersionSerializer
     openapi_overrides = {
         'tags': ['repos'],

arkindex/dataimport/tests/test_workers.py

-from unittest import expectedFailure
-
 from django.urls import reverse
 from rest_framework import status
 
@@ -275,10 +273,6 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
             'configuration': ['This field is required.']
         })
 
-    def test_retrieve_version_requires_login(self):
-        response = self.client.get(reverse('api:version-retrieve', kwargs={'pk': str(self.version_1.id)}))
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-
     def test_retrieve_version_invalid_id(self):
         self.client.force_login(self.user)
         response = self.client.get(
@@ -286,6 +280,23 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
         )
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
+    def test_retrieve_version_no_login(self):
+        response = self.client.get(reverse('api:version-retrieve', kwargs={'pk': str(self.version_1.id)}))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        data = response.json()
+        revision = data.pop('revision')
+        self.assertEqual(revision['id'], str(self.rev.id))
+        worker = data.pop('worker')
+        self.assertEqual(worker['id'], str(self.worker_1.id))
+        self.assertDictEqual(data, {
+            'id': str(self.version_1.id),
+            'configuration': {"test": 42},
+            'docker_image': str(self.version_1.docker_image.id),
+            'docker_image_iid': None,
+            'docker_image_name': f'my_repo.fake/workers/worker/reco:{self.version_1.id}',
+            'state': 'available',
+        })
+
     def test_retrieve_version(self):
         self.client.force_login(self.user)
         response = self.client.get(reverse('api:version-retrieve', kwargs={'pk': str(self.version_1.id)}))
@@ -295,7 +306,6 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
         self.assertEqual(revision['id'], str(self.rev.id))
         worker = data.pop('worker')
         self.assertEqual(worker['id'], str(self.worker_1.id))
-        self.maxDiff = None
         self.assertDictEqual(data, {
             'id': str(self.version_1.id),
             'configuration': {"test": 42},
@@ -491,33 +501,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
         self.assertEqual(self.version_1.docker_command, 'mysupercommand')
         self.version_1.configuration = {"test": "test1"}
 
-    def test_corpus_worker_version_requires_login(self):
-        with self.assertNumQueries(0):
-            response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
-            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-
-    @expectedFailure
-    def test_corpus_worker_version_requires_verified(self):
-        """
-        This test fails due to a bug in the IsVerified permission class.
-        https://gitlab.com/arkindex/backend/-/issues/554
-        """
-        self.user.verified_email = False
-        self.user.save()
-        self.client.force_login(self.user)
-        with self.assertNumQueries(2):
-            response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
-            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-
-    def test_corpus_worker_version_list(self):
-        self.client.force_login(self.user)
-
-        self.corpus.elements.filter(type__slug='word').update(worker_version=self.version_1)
-
-        with self.assertNumQueries(5):
-            response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
-            self.assertEqual(response.status_code, status.HTTP_200_OK)
-
+    def _assert_corpus_worker_version_list(self, response):
         self.assertListEqual(response.json(), [
             {
                 'id': str(self.version_1.id),
@@ -545,3 +529,31 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
                 'element_count': 9
             }
         ])
+
+    def test_corpus_worker_version_no_login(self):
+        self.corpus.elements.filter(type__slug='word').update(worker_version=self.version_1)
+
+        with self.assertNumQueries(6):
+            response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self._assert_corpus_worker_version_list(response)
+
+    def test_corpus_worker_version_not_verified(self):
+        self.user.verified_email = False
+        self.user.save()
+        self.client.force_login(self.user)
+        self.corpus.elements.filter(type__slug='word').update(worker_version=self.version_1)
+
+        with self.assertNumQueries(8):
+            response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self._assert_corpus_worker_version_list(response)
+
+    def test_corpus_worker_version_list(self):
+        self.client.force_login(self.user)
+        self.corpus.elements.filter(type__slug='word').update(worker_version=self.version_1)
+
+        with self.assertNumQueries(8):
+            response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self._assert_corpus_worker_version_list(response)