diff --git a/arkindex/process/api.py b/arkindex/process/api.py index 2a5bf6fb8b8ffd7fa9291e60e4d42dfbb3d25d43..bfd5d73fdc1e0ae9b8a4f33f9e52d10194f30d3c 100644 --- a/arkindex/process/api.py +++ b/arkindex/process/api.py @@ -978,7 +978,7 @@ class RevisionRetrieve(RepositoryACLMixin, RetrieveAPIView): """), ), ) -class WorkerList(WorkerACLMixin, ListCreateAPIView): +class WorkerList(ListCreateAPIView): permission_classes = (IsVerified, ) serializer_class = WorkerSerializer queryset = Worker.objects.none() @@ -1017,10 +1017,9 @@ class WorkerList(WorkerACLMixin, ListCreateAPIView): """ List executable workers (i.e. via a direct write right or a write right on its repository). """ - return self.executable_workers \ + return Worker.objects.executable(self.request.user) \ .order_by('name', 'id') \ - .select_related('type') \ - .distinct() + .select_related('type') def create(self, *args, **kwargs): try: @@ -1202,22 +1201,23 @@ class WorkerVersionRetrieve(RetrieveUpdateAPIView): raise PermissionDenied(detail='Only Ponos tasks are allowed to update a worker version.') -@extend_schema( - tags=['repos'], - description='Retrieve a worker', +@extend_schema(tags=['repos']) +@extend_schema_view( + get=extend_schema( + description=dedent(""" + Retrieve a worker. + + Requires a contributor access to the worker or its repository, except for `public` workers. + """), + ), ) -class WorkerRetrieve(WorkerACLMixin, RetrieveAPIView): +class WorkerRetrieve(RetrieveAPIView): permission_classes = (IsVerified, ) serializer_class = WorkerSerializer - queryset = Worker.objects.all() + queryset = Worker.objects.none() - def get_object(self): - queryset = self.filter_queryset(self.get_queryset()).prefetch_related('repository') - worker = get_object_or_404(queryset, pk=self.kwargs['pk']) - if not self.has_read_access(worker): - raise PermissionDenied(detail='You do not have a guest access to this worker.') - super().check_object_permissions(self.request, worker) - return worker + def get_queryset(self): + return Worker.objects.executable(self.request.user).select_related('repository', 'type') @extend_schema(tags=['repos']) diff --git a/arkindex/process/models.py b/arkindex/process/models.py index a40c9581a0f24903393437966bf183502068077f..22131c52d57037e183e086b7681269ca669b071e 100644 --- a/arkindex/process/models.py +++ b/arkindex/process/models.py @@ -29,6 +29,7 @@ from arkindex.project.fields import ArrayField, MD5HashField from arkindex.project.models import IndexableModel from arkindex.project.validators import MaxValueValidator from arkindex.training.models import ModelVersion, ModelVersionState +from arkindex.users.models import Role def process_max_chunks(): @@ -589,6 +590,26 @@ class Repository(models.Model): def __str__(self): return f'{self.url}' + def is_executable(self, user) -> bool: + """ + Whether a user can execute any worker within this repository. + + When a user cannot execute all workers of this repository, some workers may still have + some individual rights assigned to them; see Worker.is_executable. + """ + if user.is_anonymous or getattr(user, 'is_agent', False): + return False + + # Allow admins to execute anything + if user.is_admin: + return True + + from arkindex.users.utils import get_max_level + level = get_max_level(user, self) + + # Otherwise, users will need contributor access to the repository + return level is not None and level >= Role.Contributor.value + class GitRefType(Enum): Branch = 'branch' @@ -664,12 +685,40 @@ class Worker(models.Model): def __str__(self): return f'{self.name}' + def is_editable(self, user) -> bool: + """ + Whether a user can edit this worker + """ + # Require authentication, block edition by Ponos agents (not Ponos tasks) + if user.is_anonymous or getattr(user, 'is_agent', False): + return False + + # Allow admins to edit anything + if user.is_admin: + return True + + from arkindex.users.utils import get_max_level + level = get_max_level(user, self) + + # Otherwise, users should either have contributor access to the worker, + # or the repo when there is one set + return ( + level is not None and level >= Role.Contributor.value + ) or ( + self.repository is not None and self.repository.is_executable(user) + ) + def is_executable(self, user) -> bool: """ - Whether an user has a contributor access to this worker + Whether a user can view and execute this worker """ - from arkindex.project.mixins import WorkerACLMixin - return WorkerACLMixin(user=user).has_execution_access(self) + # Require authentication, block edition by Ponos agents (not Ponos tasks) + if user.is_anonymous or getattr(user, 'is_agent', False): + return False + + # Apply the same rules as for editable workers, + # but also allow any authenticated user to execute a public worker + return self.public or self.is_editable(user) class WorkerType(IndexableModel): diff --git a/arkindex/process/tests/test_workers.py b/arkindex/process/tests/test_workers.py index a020684b2e92d1a5e72c164c746bfe39eca410fb..9ffb00d347d896fa6117b431a37dc01927f9d50b 100644 --- a/arkindex/process/tests/test_workers.py +++ b/arkindex/process/tests/test_workers.py @@ -306,9 +306,9 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): response = self.client.get(reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_workers_retrieve(self): + def test_workers_retrieve_repository(self): """ - A user is able to retrieve a worker if he has a guest access on it or its repository + A user can retrieve a worker by having contributor access on its repository """ repo2 = Repository.objects.create( url='http://gitlab/repo2', @@ -316,10 +316,10 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): credentials=self.creds, ) worker_2 = repo2.workers.create(name='Worker 2', slug='worker_2', type=self.worker_type_dla) - repo2.memberships.create(user=self.user, level=Role.Guest.value) + repo2.memberships.create(user=self.user, level=Role.Contributor.value) self.client.force_login(self.user) - with self.assertNumQueries(9): + with self.assertNumQueries(5): response = self.client.get(reverse('api:worker-retrieve', kwargs={'pk': str(worker_2.id)})) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertDictEqual(response.json(), { @@ -332,9 +332,9 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): def test_workers_retrieve_no_revision(self): self.client.force_login(self.user) - with self.assertNumQueries(7): + with self.assertNumQueries(6): response = self.client.get(reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_custom.id)})) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertDictEqual(response.json(), { 'id': str(self.worker_custom.id), 'repository_id': None, @@ -346,10 +346,21 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): def test_workers_retrieve_no_rights(self): new_user = User.objects.create(email='new@test.fr', verified_email=True) self.client.force_login(new_user) - with self.assertNumQueries(9): + with self.assertNumQueries(6): response = self.client.get(reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)})) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertEqual(response.json(), {'detail': 'You do not have a guest access to this worker.'}) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.json(), {'detail': 'Not found.'}) + + def test_workers_retrieve_guest(self): + """ + Users cannot retrieve workers with guest access, as guest has no meaning on workers + """ + self.client.force_login(self.user) + self.worker_custom.memberships.filter(user=self.user).update(level=Role.Guest.value) + with self.assertNumQueries(5): + response = self.client.get(reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_custom.id)})) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.json(), {'detail': 'Not found.'}) def test_worker_create_requires_login(self): with self.assertNumQueries(0):