diff --git a/arkindex/ponos/permissions.py b/arkindex/ponos/permissions.py index a92e6c86104cb599227cdea28ac1f42bf7509cb4..16a158c02f63c33cbea7e34384628207eb461d5c 100644 --- a/arkindex/ponos/permissions.py +++ b/arkindex/ponos/permissions.py @@ -64,7 +64,7 @@ class IsAssignedAgentOrReadOnly(IsAgentOrReadOnly): return super().has_object_permission(request, view, obj) -class IsAssignedAgentOrTaskOrReadOnly(IsAgentOrTask): +class IsAssignedAgentOrTaskOrReadOnly(ProcessACLMixin, IsAgentOrTask): """ Restricts write access to Ponos agents, Ponos tasks, and admins, and allows read access to anyone. When checking object write permissions for a Ponos task, requires either a Ponos agent assigned to the task, @@ -75,12 +75,13 @@ class IsAssignedAgentOrTaskOrReadOnly(IsAgentOrTask): def has_object_permission(self, request, view, obj) -> bool: assert isinstance(obj, Task) + if request.method in SAFE_METHODS: + return True + if isinstance(request.auth, Task): return obj == request.auth - return super().has_object_permission(request, view, obj) and ( - obj.agent_id == request.user.id or request.method in SAFE_METHODS - ) + return (super().has_object_permission(request, view, obj) and obj.agent_id == request.user.id) class IsTaskAdmin(ProcessACLMixin, IsVerified): @@ -98,7 +99,8 @@ class IsTaskAdmin(ProcessACLMixin, IsVerified): class IsAgentOrTaskGuest(ProcessACLMixin, IsAuthenticated): """ - Allow admins, Ponos agents, users with a verified email and guest access to the task's process, or the task itself. + Allow admins, Ponos agents, users with a verified email and guest access to the task's process, the task itself, + or another task belonging to a process the creator of which has access. """ def has_object_permission(self, request, view, task): @@ -113,8 +115,6 @@ class IsAgentOrTaskGuest(ProcessACLMixin, IsAuthenticated): or ( # Users with a verified email getattr(request.user, 'verified_email', False) - # Not authenticated through Ponos task authentication - and not isinstance(request.auth, Task) # process_access_level can return None if there is no access at all and (self.process_access_level(task.process) or 0) >= Role.Guest.value ) diff --git a/arkindex/ponos/tests/test_api.py b/arkindex/ponos/tests/test_api.py index 8ed49f9c0d24fe6b97e3d8822073daf0d2101cb8..72b96030d1d67b543286869109606b06cdc58816 100644 --- a/arkindex/ponos/tests/test_api.py +++ b/arkindex/ponos/tests/test_api.py @@ -27,7 +27,7 @@ from arkindex.ponos.models import ACTIVE_STATES, FINAL_STATES, GPU, Agent, Farm, from arkindex.process.models import Process, ProcessMode, Revision, WorkerVersion from arkindex.project.tests import FixtureAPITestCase from arkindex.project.tools import build_public_key -from arkindex.users.models import Role +from arkindex.users.models import Right, Role, User from rest_framework_simplejwt.tokens import AccessToken, RefreshToken PONOS_PRIVATE_KEY = Path(__file__).absolute().parent / 'fixtures' / 'ponos.key' @@ -61,6 +61,19 @@ class TestAPI(FixtureAPITestCase): cls.process = Process.objects.get(mode=ProcessMode.Workers) cls.process.run() cls.task1, cls.task2, cls.task3 = cls.process.tasks.all() + + # Brand new user and corpus with no preexisting rights + new_user = User.objects.create(email='another@user.com') + new_corpus = Corpus.objects.create(name='corpus the second') + Right.objects.create(user=new_user, content_object=new_corpus, level=50) + cls.process2 = Process.objects.create( + mode=ProcessMode.Workers, + creator=new_user, + corpus=new_corpus + ) + cls.process2.run() + cls.task4 = cls.process2.tasks.first() + cls.dla = WorkerVersion.objects.get(worker__slug='dla') cls.recognizer = WorkerVersion.objects.get(worker__slug='reco') cls.gpu1 = cls.agent.gpus.create( @@ -124,11 +137,15 @@ class TestAPI(FixtureAPITestCase): call("get_object", Params={"Bucket": "ponos", "Key": "somelog"}), ) - def test_task_details_other_task_forbidden(self): + def test_task_details_other_task_no_access_forbidden(self): + # Make corpus private just for this test + self.corpus.public = False + self.corpus.save() + with self.assertNumQueries(2): resp = self.client.get( reverse("api:task-details", args=[self.task2.id]), - HTTP_AUTHORIZATION=f'Ponos {self.task1.token}', + HTTP_AUTHORIZATION=f'Ponos {self.task4.token}', ) self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) diff --git a/arkindex/ponos/tests/test_artifacts_api.py b/arkindex/ponos/tests/test_artifacts_api.py index be122bf9b69eaf1e6df7e1d95d2f676cf89ebd75..3a94095639f74a4f2d9a096a04b315d827ebed52 100644 --- a/arkindex/ponos/tests/test_artifacts_api.py +++ b/arkindex/ponos/tests/test_artifacts_api.py @@ -7,12 +7,13 @@ from django.urls import reverse from django.utils import timezone from rest_framework import status +from arkindex.documents.models import Corpus from arkindex.ponos.authentication import AgentUser from arkindex.ponos.models import Farm from arkindex.process.models import Process, ProcessMode, Repository from arkindex.project.tests import FixtureAPITestCase from arkindex.project.tools import build_public_key -from arkindex.users.models import Role +from arkindex.users.models import Right, Role, User @override_settings(PONOS_LOG_TAIL=42) @@ -41,9 +42,24 @@ class TestAPI(FixtureAPITestCase): revision=cls.repo.revisions.first(), ) + # Make corpus private + cls.corpus.public = False + cls.corpus.save() cls.process = Process.objects.get(mode=ProcessMode.Workers) cls.process.run() + # Two tasks from the same process cls.task1, cls.task2 = cls.process.tasks.all()[:2] + # Brand new user and corpus with no preexisting rights + new_user = User.objects.create(email='another@user.com') + new_corpus = Corpus.objects.create(name='corpus the second') + Right.objects.create(user=new_user, content_object=new_corpus, level=50) + cls.process2 = Process.objects.create( + mode=ProcessMode.Workers, + creator=new_user, + corpus=new_corpus + ) + cls.process2.run() + cls.task3 = cls.process2.tasks.first() cls.artifact1 = cls.task1.artifacts.create( path="path/to/file.json", @@ -218,14 +234,22 @@ class TestAPI(FixtureAPITestCase): ], ) - def test_list_other_task(self): + def test_list_other_task_no_access(self): with self.assertNumQueries(2): response = self.client.get( reverse("api:task-artifacts", args=[self.task1.id]), - HTTP_AUTHORIZATION=f'Ponos {self.task2.token}', + HTTP_AUTHORIZATION=f'Ponos {self.task3.token}', ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_list_other_task_with_access(self): + with self.assertNumQueries(6): + response = self.client.get( + reverse("api:task-artifacts", args=[self.task1.id]), + HTTP_AUTHORIZATION=f'Ponos {self.task2.token}', + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + def test_list_task(self): with self.assertNumQueries(3): response = self.client.get( @@ -401,7 +425,7 @@ class TestAPI(FixtureAPITestCase): self.assertEqual(response.json(), {"size": [expected_error]}) def test_create_other_task(self): - with self.assertNumQueries(2): + with self.assertNumQueries(5): response = self.client.post( reverse("api:task-artifacts", args=[self.task1.id]), data={ @@ -564,10 +588,24 @@ class TestAPI(FixtureAPITestCase): ) ) - def test_download_other_task(self): - with self.assertNumQueries(2): + def test_download_other_task_with_access(self): + with self.assertNumQueries(5): response = self.client.get( reverse("api:task-artifact-download", args=[self.task1.id, "path/to/file.json"]), HTTP_AUTHORIZATION=f'Ponos {self.task2.token}', ) + self.assertEqual(response.status_code, status.HTTP_302_FOUND) + self.assertTrue(response.has_header("Location")) + self.assertTrue( + response["Location"].startswith( + f"http://s3/ponos-artifacts/{self.task1.id}/path/to/file.json" + ) + ) + + def test_download_other_task_no_access(self): + with self.assertNumQueries(2): + response = self.client.get( + reverse("api:task-artifact-download", args=[self.task1.id, "path/to/file.json"]), + HTTP_AUTHORIZATION=f'Ponos {self.task3.token}', + ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)