From b4d1f2c045e765cf1556e3ada76b4a8ba5e64a9e Mon Sep 17 00:00:00 2001
From: Erwan Rouchet <rouchet@teklia.com>
Date: Wed, 19 Jul 2023 10:48:40 +0000
Subject: [PATCH] Update permissions on artifact endpoints

---
 arkindex/ponos/api.py                      |  47 +-
 arkindex/ponos/permissions.py              |  69 ++-
 arkindex/ponos/serializers.py              |   2 +-
 arkindex/ponos/tests/test_api.py           | 260 ---------
 arkindex/ponos/tests/test_artifacts_api.py | 592 +++++++++++++++++++++
 arkindex/project/tests/test_ponos_view.py  |  47 +-
 6 files changed, 666 insertions(+), 351 deletions(-)
 create mode 100644 arkindex/ponos/tests/test_artifacts_api.py

diff --git a/arkindex/ponos/api.py b/arkindex/ponos/api.py
index 2689f09e60..2a25caee1c 100644
--- a/arkindex/ponos/api.py
+++ b/arkindex/ponos/api.py
@@ -25,10 +25,11 @@ from arkindex.ponos.keys import load_private_key
 from arkindex.ponos.models import Agent, Artifact, Farm, Secret, State, Task
 from arkindex.ponos.permissions import (
     IsAgent,
-    IsAgentOrArtifactAdmin,
+    IsAgentOrArtifactGuest,
     IsAgentOrTaskAdmin,
-    IsAgentOrTaskAdminOrReadOnly,
+    IsAgentOrTaskGuest,
     IsAssignedAgentOrReadOnly,
+    IsAssignedAgentOrTaskOrReadOnly,
     IsTask,
 )
 from arkindex.ponos.renderers import PublicKeyPEMRenderer
@@ -253,41 +254,52 @@ class TaskDefinition(RetrieveAPIView):
 @extend_schema_view(
     get=extend_schema(
         operation_id="ListArtifacts",
-        description="List all the artifacts of a task",
+        description=dedent("""
+        List the artifacts of a task.
+
+        Requires **guest** access on the task's process, Ponos agent authentication, or authentication as the Ponos task to list artifacts of.
+        """),
     ),
     post=extend_schema(
-        operation_id="CreateArtifact", description="Create an artifact on a task"
+        operation_id="CreateArtifact",
+        description=dedent("""
+        Create an artifact on task.
+
+        Requires authentication as the Ponos task to create an artifact on, or as a Ponos agent assigned to the task.
+        """),
     ),
 )
 class TaskArtifacts(ListCreateAPIView):
-    """
-    List all artifacts linked to a task or create one
-    """
 
     # Used for OpenAPI schema serialization: the ID in the path is the task ID
     queryset = Task.objects.none()
-    permission_classes = (IsAgentOrTaskAdminOrReadOnly, )
     serializer_class = ArtifactSerializer
+    permission_classes = (
+        # On all HTTP methods, require either any Ponos agent, an instance admin, the task itself, or guest access to the process' task
+        IsAgentOrTaskGuest,
+        # On unsafe HTTP methods, require a Ponos agent assigned to the task, or the task itself. Both permission classes are combined.
+        IsAssignedAgentOrTaskOrReadOnly,
+    )
 
     # Force no pagination, even when global settings add them
     pagination_class = None
 
-    def get_task(self):
+    @property
+    def task(self):
         task = get_object_or_404(
             # Select the required tables for permissions checking
-            Task.objects.select_related('process__corpus', 'process__revision'),
+            Task.objects.select_related('process__corpus', 'process__revision__repo'),
             pk=self.kwargs["pk"],
         )
         self.check_object_permissions(self.request, task)
         return task
 
     def get_queryset(self):
-        task = self.get_task()
-        return task.artifacts.all()
+        return self.task.artifacts.all()
 
     def perform_create(self, serializer):
         # Assign task when creating through the API
-        serializer.save(task=self.get_task())
+        serializer.save(task=self.task)
 
 
 class TaskArtifactDownload(APIView):
@@ -295,10 +307,15 @@ class TaskArtifactDownload(APIView):
     Redirects to the S3 URL of an artifact in order to download it.
     """
 
-    permission_classes = (IsAgentOrArtifactAdmin, )
+    permission_classes = (IsAgentOrArtifactGuest, )
 
     def get_object(self, pk, path):
-        artifact = get_object_or_404(Artifact, task_id=pk, path=path)
+        artifact = get_object_or_404(
+            # Select the required tables for permissions checking
+            Artifact.objects.select_related('task__process__corpus', 'task__process__revision__repo'),
+            task_id=pk,
+            path=path,
+        )
         self.check_object_permissions(self.request, artifact)
         return artifact
 
diff --git a/arkindex/ponos/permissions.py b/arkindex/ponos/permissions.py
index 17b8973568..f8624fd93a 100644
--- a/arkindex/ponos/permissions.py
+++ b/arkindex/ponos/permissions.py
@@ -1,7 +1,6 @@
 from rest_framework.permissions import SAFE_METHODS
 
 from arkindex.ponos.models import Task
-from arkindex.process.models import Process
 from arkindex.project.mixins import CorpusACLMixin, ProcessACLMixin
 from arkindex.project.permissions import IsAuthenticated
 from arkindex.users.models import Role
@@ -29,6 +28,13 @@ class IsAgent(IsAuthenticated):
     checks = IsAuthenticated.checks + (require_agent_or_admin, )
 
 
+class IsAgentOrTask(IsAuthenticated):
+    """
+    Only allow Ponos agents, tasks, and admins.
+    """
+    checks = IsAuthenticated.checks + (require_agent_or_task, )
+
+
 class IsAgentOrReadOnly(IsAgent):
     """
     Restricts write access to Ponos agents and admins,
@@ -54,6 +60,25 @@ class IsAssignedAgentOrReadOnly(IsAgentOrReadOnly):
         return super().has_object_permission(request, view, obj)
 
 
+class IsAssignedAgentOrTaskOrReadOnly(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,
+    or authentication as the task itself.
+    """
+    allow_safe_methods = True
+
+    def has_object_permission(self, request, view, obj) -> bool:
+        assert isinstance(obj, Task)
+
+        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
+        )
+
+
 class IsAgentOrTaskAdmin(CorpusACLMixin, IsAuthenticated):
     """
     Permission to access a task with high privilege
@@ -76,45 +101,31 @@ class IsAgentOrTaskAdmin(CorpusACLMixin, IsAuthenticated):
         )
 
 
-class IsAgentOrTaskAdminOrReadOnly(ProcessACLMixin, IsAuthenticated):
+class IsAgentOrTaskGuest(ProcessACLMixin, IsAuthenticated):
     """
-    Instance admins, agents, process admins, and the task itself are always allowed.
-    For GET/HEAD, only a Guest level on the process is required for regular users.
+    Allow admins, Ponos agents, users with a verified email and guest access to the task's process, or the task itself.
     """
 
     def has_object_permission(self, request, view, task):
-        # Allow agents and instance admins
-        if require_agent_or_admin(request, view):
-            return True
-
-        # Allow a task to access itself
-        if isinstance(request.auth, Task) and request.auth.id == task.id:
-            return True
+        assert isinstance(task, Task)
 
         # Add request to attributes for the ACL mixin to work with self.user
         self.request = request
-        try:
-            level = self.process_access_level(task.process)
-        except Process.DoesNotExist:
-            # Reject if the task has no process
-            return False
-
-        # Require *some* access to the process
-        if level is None:
-            return False
 
-        # Require only a guest access for GET/HEAD
-        if request.method in SAFE_METHODS:
-            return level >= Role.Guest.value
-
-        # Require admin access for other methods
-        return level >= Role.Admin.value
+        return (
+            task == request.auth
+            or require_agent_or_admin(request, view)
+            or (
+                getattr(request.user, 'verified_email', False)
+                # 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
+            )
+        )
 
 
-class IsAgentOrArtifactAdmin(IsAgentOrTaskAdmin):
+class IsAgentOrArtifactGuest(IsAgentOrTaskGuest):
     """
-    Permission to access an artifact with high privilege, based on
-    access to the artifact's task through IsAgentOrTaskAdmin.
+    Permission to access an artifact, based on access to the artifact's task through IsAgentOrTaskGuest.
     """
 
     def has_object_permission(self, request, view, artifact):
diff --git a/arkindex/ponos/serializers.py b/arkindex/ponos/serializers.py
index b37ebc7120..a6842f4108 100644
--- a/arkindex/ponos/serializers.py
+++ b/arkindex/ponos/serializers.py
@@ -506,7 +506,7 @@ class ArtifactSerializer(serializers.ModelSerializer):
 
     def validate_path(self, path):
         """Check that no artifacts with this path already exist in DB"""
-        task = self.context["view"].get_task()
+        task = self.context["view"].task
         if task.artifacts.filter(path=path).exists():
             raise ValidationError("An artifact with this path already exists")
 
diff --git a/arkindex/ponos/tests/test_api.py b/arkindex/ponos/tests/test_api.py
index 917ffa0553..c55ca184cf 100644
--- a/arkindex/ponos/tests/test_api.py
+++ b/arkindex/ponos/tests/test_api.py
@@ -1224,266 +1224,6 @@ class TestAPI(FixtureAPITestCase):
             (0.9, 0.49, now),
         )
 
-    def test_artifacts_list(self):
-        url = reverse("api:task-artifacts", args=[self.task1.id])
-
-        # Not accessible when logged out
-        resp = self.client.get(url)
-        self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
-
-        # Accessible but empty when logged in
-        resp = self.client.get(
-            url,
-            HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
-        )
-        self.assertEqual(resp.status_code, status.HTTP_200_OK)
-        data = resp.json()
-        self.assertListEqual(data, [])
-
-        # Add a few artifacts
-        xxx = self.task1.artifacts.create(
-            path="path/to/xxx", content_type="application/json", size=123
-        )
-        demo = self.task1.artifacts.create(
-            path="demo.txt", content_type="text/plain", size=12
-        )
-        self.task2.artifacts.create(path="demo.txt", content_type="text/plain", size=1)
-
-        resp = self.client.get(
-            url,
-            HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
-        )
-        self.assertEqual(resp.status_code, status.HTTP_200_OK)
-        data = resp.json()
-
-        # We only get the 2 artifacts on task1, ordered by path
-        self.assertListEqual(
-            data,
-            [
-                {
-                    "content_type": "text/plain",
-                    "created": str_date(demo.created),
-                    "id": str(demo.id),
-                    "path": "demo.txt",
-                    "s3_put_url": None,
-                    "size": 12,
-                    "updated": str_date(demo.updated),
-                    "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/demo.txt",
-                },
-                {
-                    "content_type": "application/json",
-                    "created": str_date(xxx.created),
-                    "id": str(xxx.id),
-                    "path": "path/to/xxx",
-                    "s3_put_url": None,
-                    "size": 123,
-                    "updated": str_date(xxx.updated),
-                    "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/path/to/xxx",
-                },
-            ],
-        )
-
-    def test_list_artifacts_process_creator(self):
-        artifact = self.task1.artifacts.create(
-            path="demo.txt",
-            content_type="text/plain",
-            size=12,
-        )
-        self.client.force_login(self.user)
-
-        with self.assertNumQueries(7):
-            response = self.client.get(reverse('api:task-artifacts', kwargs={'pk': self.task1.id}))
-            self.assertEqual(response.status_code, status.HTTP_200_OK)
-
-        self.assertListEqual(response.json(), [
-            {
-                'id': str(artifact.id),
-                'created': artifact.created.isoformat().replace('+00:00', 'Z'),
-                'updated': artifact.updated.isoformat().replace('+00:00', 'Z'),
-                'content_type': 'text/plain',
-                'path': 'demo.txt',
-                'size': 12,
-                's3_put_url': None,
-                'url': f'http://testserver/api/v1/task/{self.task1.id}/artifact/demo.txt',
-            }
-        ])
-
-    @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999)
-    def test_artifact_creation(self):
-
-        # No artifacts in DB at first
-        self.assertFalse(self.task1.artifacts.exists())
-
-        # Create an artifact through the API
-        url = reverse("api:task-artifacts", args=[self.task1.id])
-        resp = self.client.post(
-            url,
-            data={"path": "some/path.txt", "content_type": "text/plain", "size": 1000},
-            HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
-        )
-        self.assertEqual(resp.status_code, status.HTTP_201_CREATED)
-
-        # Check response has a valid S3 put URL, without matching the parameters in the querystring
-        data = resp.json()
-        s3_put_url = data.get("s3_put_url")
-        self.assertIsNotNone(s3_put_url)
-        del data["s3_put_url"]
-        self.assertTrue(
-            s3_put_url.startswith(
-                f"http://somewhere/ponos-artifacts/{self.task1.id}/some/path.txt"
-            )
-        )
-
-        # An artifact has been created
-        artifact = self.task1.artifacts.get()
-
-        self.assertDictEqual(
-            resp.json(),
-            {
-                "content_type": "text/plain",
-                "created": str_date(artifact.created),
-                "id": str(artifact.id),
-                "path": "some/path.txt",
-                "size": 1000,
-                "updated": str_date(artifact.updated),
-                "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/path.txt",
-            },
-        )
-
-        # Creating another artifact with the same path will fail cleanly
-        resp = self.client.post(
-            url,
-            data={
-                "path": "some/path.txt",
-                "content_type": "text/plain",
-                "size": 10243,
-            },
-            HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
-        )
-        self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertDictEqual(
-            resp.json(), {"path": ["An artifact with this path already exists"]}
-        )
-
-    @override_settings()
-    def test_create_artifact_size_limits(self):
-        params = [
-            (None, -42, "Ensure this value is greater than or equal to 1."),
-            (None, 0, "Ensure this value is greater than or equal to 1."),
-            (
-                None,
-                5 * 1024**3 + 1,
-                "Ensure this value is less than or equal to 5368709120.",
-            ),
-            (123456789000, -42, "Ensure this value is greater than or equal to 1."),
-            (123456789000, 0, "Ensure this value is greater than or equal to 1."),
-            (
-                123456789000,
-                987654321000,
-                "Ensure this value is less than or equal to 123456789000.",
-            ),
-        ]
-
-        for size_setting, size, expected_error in params:
-            with self.subTest(size=size):
-                settings.PONOS_ARTIFACT_MAX_SIZE = size_setting
-                url = reverse("api:task-artifacts", args=[self.task1.id])
-                resp = self.client.post(
-                    url,
-                    data={
-                        "path": "some/path.txt",
-                        "content_type": "text/plain",
-                        "size": size,
-                    },
-                    HTTP_AUTHORIZATION=f"Bearer {self.agent.token.access_token}",
-                )
-                self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
-                self.assertEqual(resp.json(), {"size": [expected_error]})
-
-    @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999)
-    def test_create_artifact_task(self):
-        process = Process.objects.create(
-            creator=self.user,
-            mode=ProcessMode.Repository,
-            revision=self.rev
-        )
-        process.start()
-
-        with self.assertNumQueries(5):
-            resp = self.client.post(
-                reverse("api:task-artifacts", args=[self.task1.id]),
-                data={
-                    "path": "some/path.txt",
-                    "content_type": "text/plain",
-                    "size": 1000,
-                },
-                HTTP_AUTHORIZATION=f"Ponos {self.task1.token}",
-            )
-            self.assertEqual(resp.status_code, status.HTTP_201_CREATED, resp.json())
-
-        data = resp.json()
-        s3_put_url = data.pop("s3_put_url")
-        self.assertTrue(
-            s3_put_url.startswith(
-                f"http://somewhere/ponos-artifacts/{self.task1.id}/some/path.txt"
-            )
-        )
-
-        artifact = self.task1.artifacts.get()
-        self.assertDictEqual(
-            resp.json(),
-            {
-                "content_type": "text/plain",
-                "created": str_date(artifact.created),
-                "id": str(artifact.id),
-                "path": "some/path.txt",
-                "size": 1000,
-                "updated": str_date(artifact.updated),
-                "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/path.txt",
-            },
-        )
-
-    def test_artifact_download(self):
-
-        # Add a new artifact
-        self.task1.artifacts.create(
-            path="path/to/file.json",
-            content_type="application/json",
-            size=42,
-        )
-
-        # Request to get the real download link will fail as anonymous
-        url = reverse(
-            "api:task-artifact-download", args=[self.task1.id, "path/to/file.json"]
-        )
-        resp = self.client.get(url)
-        self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
-
-        # Request to a missing artifact will lead to a 404
-        bad_url = reverse(
-            "api:task-artifact-download", args=[self.task1.id, "nope.xxx"]
-        )
-        resp = self.client.get(
-            bad_url,
-            HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
-        )
-        self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND)
-
-        # Valid request will redirect to the S3 url
-        resp = self.client.get(
-            url,
-            HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
-        )
-        self.assertEqual(resp.status_code, status.HTTP_302_FOUND)
-
-        # Check the S3 download link
-        self.assertTrue(resp.has_header("Location"))
-        self.assertTrue(
-            resp["Location"].startswith(
-                f"http://somewhere/ponos-artifacts/{self.task1.id}/path/to/file.json"
-            )
-        )
-
     def test_task_create_requires_task_auth(self):
         response = self.client.post(reverse("api:task-create"))
         self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
diff --git a/arkindex/ponos/tests/test_artifacts_api.py b/arkindex/ponos/tests/test_artifacts_api.py
new file mode 100644
index 0000000000..6dfa397b57
--- /dev/null
+++ b/arkindex/ponos/tests/test_artifacts_api.py
@@ -0,0 +1,592 @@
+import hashlib
+import uuid
+
+from django.conf import settings
+from django.test import override_settings
+from django.urls import reverse
+from django.utils import timezone
+from rest_framework import status
+
+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
+
+
+@override_settings(PONOS_LOG_TAIL=42)
+class TestAPI(FixtureAPITestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+        cls.wheat_farm = Farm.objects.get(name='Wheat farm')
+        pubkey = build_public_key()
+        cls.agent = AgentUser.objects.create(
+            id=uuid.UUID(hashlib.md5(pubkey.encode("utf-8")).hexdigest()),
+            farm=cls.wheat_farm,
+            hostname="ghostname",
+            cpu_cores=2,
+            cpu_frequency=1e9,
+            public_key=pubkey,
+            ram_total=2e9,
+            last_ping=timezone.now(),
+        )
+
+        cls.repo = Repository.objects.first()
+        cls.repository_process = Process.objects.create(
+            mode=ProcessMode.Repository,
+            creator=cls.superuser,
+            revision=cls.repo.revisions.first(),
+        )
+
+        cls.process = Process.objects.get(mode=ProcessMode.Workers)
+        cls.process.start()
+        cls.task1, cls.task2 = cls.process.tasks.all()[:2]
+
+        cls.artifact1 = cls.task1.artifacts.create(
+            path="path/to/file.json",
+            content_type="application/json",
+            size=42,
+        )
+        cls.artifact2 = cls.task1.artifacts.create(
+            path="some/text.txt",
+            content_type="text/plain",
+            size=1337,
+        )
+
+    def test_list_requires_login(self):
+        with self.assertNumQueries(0):
+            response = self.client.get(reverse("api:task-artifacts", args=[self.task1.id]))
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_list_requires_verified(self):
+        self.user.verified_email = False
+        self.user.save()
+        self.client.force_login(self.user)
+
+        with self.assertNumQueries(3):
+            response = self.client.get(reverse("api:task-artifacts", args=[self.task1.id]))
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_list_requires_process_guest(self):
+        self.process.creator = self.superuser
+        self.process.save()
+        self.corpus.memberships.filter(user=self.user).delete()
+        self.corpus.public = False
+        self.corpus.save()
+        self.client.force_login(self.user)
+
+        with self.assertNumQueries(5):
+            response = self.client.get(reverse("api:task-artifacts", args=[self.task1.id]))
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_list_process_level_corpus(self):
+        self.process.creator = self.superuser
+        self.process.save()
+        self.client.force_login(self.user)
+
+        for role in Role:
+            with self.subTest(role=role):
+                self.corpus.memberships.filter(user=self.user).update(level=role.value)
+
+                with self.assertNumQueries(6):
+                    response = self.client.get(reverse("api:task-artifacts", args=[self.task1.id]))
+                    self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+                self.assertListEqual(
+                    response.json(),
+                    [
+                        {
+                            "content_type": "application/json",
+                            "created": self.artifact1.created.isoformat().replace("+00:00", "Z"),
+                            "id": str(self.artifact1.id),
+                            "path": "path/to/file.json",
+                            "s3_put_url": None,
+                            "size": 42,
+                            "updated": self.artifact1.updated.isoformat().replace("+00:00", "Z"),
+                            "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/path/to/file.json",
+                        },
+                        {
+                            "content_type": "text/plain",
+                            "created": self.artifact2.created.isoformat().replace("+00:00", "Z"),
+                            "id": str(self.artifact2.id),
+                            "path": "some/text.txt",
+                            "s3_put_url": None,
+                            "size": 1337,
+                            "updated": self.artifact2.updated.isoformat().replace("+00:00", "Z"),
+                            "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/text.txt",
+                        },
+                    ],
+                )
+
+    def test_list_process_level_repo(self):
+        self.client.force_login(self.user)
+        membership = self.repo.memberships.create(user=self.user, level=Role.Guest.value)
+
+        for role in Role:
+            with self.subTest(role=role):
+                membership.level = role.value
+                membership.save()
+
+                with self.assertNumQueries(7):
+                    response = self.client.get(reverse("api:task-artifacts", args=[self.task1.id]))
+                    self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+                self.assertListEqual(
+                    response.json(),
+                    [
+                        {
+                            "content_type": "application/json",
+                            "created": self.artifact1.created.isoformat().replace("+00:00", "Z"),
+                            "id": str(self.artifact1.id),
+                            "path": "path/to/file.json",
+                            "s3_put_url": None,
+                            "size": 42,
+                            "updated": self.artifact1.updated.isoformat().replace("+00:00", "Z"),
+                            "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/path/to/file.json",
+                        },
+                        {
+                            "content_type": "text/plain",
+                            "created": self.artifact2.created.isoformat().replace("+00:00", "Z"),
+                            "id": str(self.artifact2.id),
+                            "path": "some/text.txt",
+                            "s3_put_url": None,
+                            "size": 1337,
+                            "updated": self.artifact2.updated.isoformat().replace("+00:00", "Z"),
+                            "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/text.txt",
+                        },
+                    ],
+                )
+
+    def test_list_admin(self):
+        self.client.force_login(self.superuser)
+        with self.assertNumQueries(4):
+            response = self.client.get(reverse("api:task-artifacts", args=[self.task1.id]))
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        self.assertListEqual(
+            response.json(),
+            [
+                {
+                    "content_type": "application/json",
+                    "created": self.artifact1.created.isoformat().replace("+00:00", "Z"),
+                    "id": str(self.artifact1.id),
+                    "path": "path/to/file.json",
+                    "s3_put_url": None,
+                    "size": 42,
+                    "updated": self.artifact1.updated.isoformat().replace("+00:00", "Z"),
+                    "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/path/to/file.json",
+                },
+                {
+                    "content_type": "text/plain",
+                    "created": self.artifact2.created.isoformat().replace("+00:00", "Z"),
+                    "id": str(self.artifact2.id),
+                    "path": "some/text.txt",
+                    "s3_put_url": None,
+                    "size": 1337,
+                    "updated": self.artifact2.updated.isoformat().replace("+00:00", "Z"),
+                    "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/text.txt",
+                },
+            ],
+        )
+
+    def test_list_agent(self):
+        with self.assertNumQueries(3):
+            response = self.client.get(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
+            )
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        self.assertListEqual(
+            response.json(),
+            [
+                {
+                    "content_type": "application/json",
+                    "created": self.artifact1.created.isoformat().replace("+00:00", "Z"),
+                    "id": str(self.artifact1.id),
+                    "path": "path/to/file.json",
+                    "s3_put_url": None,
+                    "size": 42,
+                    "updated": self.artifact1.updated.isoformat().replace("+00:00", "Z"),
+                    "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/path/to/file.json",
+                },
+                {
+                    "content_type": "text/plain",
+                    "created": self.artifact2.created.isoformat().replace("+00:00", "Z"),
+                    "id": str(self.artifact2.id),
+                    "path": "some/text.txt",
+                    "s3_put_url": None,
+                    "size": 1337,
+                    "updated": self.artifact2.updated.isoformat().replace("+00:00", "Z"),
+                    "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/text.txt",
+                },
+            ],
+        )
+
+    def test_list_other_task(self):
+        with self.assertNumQueries(5):
+            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_403_FORBIDDEN)
+
+    def test_list_task(self):
+        with self.assertNumQueries(3):
+            response = self.client.get(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                HTTP_AUTHORIZATION=f'Ponos {self.task1.token}',
+            )
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        self.assertListEqual(
+            response.json(),
+            [
+                {
+                    "content_type": "application/json",
+                    "created": self.artifact1.created.isoformat().replace("+00:00", "Z"),
+                    "id": str(self.artifact1.id),
+                    "path": "path/to/file.json",
+                    "s3_put_url": None,
+                    "size": 42,
+                    "updated": self.artifact1.updated.isoformat().replace("+00:00", "Z"),
+                    "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/path/to/file.json",
+                },
+                {
+                    "content_type": "text/plain",
+                    "created": self.artifact2.created.isoformat().replace("+00:00", "Z"),
+                    "id": str(self.artifact2.id),
+                    "path": "some/text.txt",
+                    "s3_put_url": None,
+                    "size": 1337,
+                    "updated": self.artifact2.updated.isoformat().replace("+00:00", "Z"),
+                    "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/text.txt",
+                },
+            ],
+        )
+
+    def test_create_requires_login(self):
+        with self.assertNumQueries(0):
+            response = self.client.post(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                data={"path": "some/path.txt", "content_type": "text/plain", "size": 1000},
+            )
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999)
+    def test_create_forbids_admin(self):
+        self.client.force_login(self.superuser)
+        with self.assertNumQueries(3):
+            response = self.client.post(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                data={"path": "some/path.txt", "content_type": "text/plain", "size": 1000},
+            )
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999)
+    def test_create_forbids_user(self):
+        self.client.force_login(self.user)
+        with self.assertNumQueries(2):
+            response = self.client.post(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                data={"path": "some/path.txt", "content_type": "text/plain", "size": 1000},
+            )
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999)
+    def test_create_unassigned_agent(self):
+        self.assertNotEqual(self.task1.agent, self.agent)
+
+        with self.assertNumQueries(2):
+            response = self.client.post(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                data={"path": "some/path.txt", "content_type": "text/plain", "size": 1000},
+                HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
+            )
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999)
+    def test_create_assigned_agent(self):
+        self.task1.agent = self.agent
+        self.task1.save()
+
+        with self.assertNumQueries(5):
+            response = self.client.post(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                data={"path": "some/path.txt", "content_type": "text/plain", "size": 1000},
+                HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
+            )
+            self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+        # Check response has a valid S3 put URL, without matching the parameters in the querystring
+        data = response.json()
+        s3_put_url = data.get("s3_put_url")
+        self.assertIsNotNone(s3_put_url)
+        del data["s3_put_url"]
+        self.assertTrue(
+            s3_put_url.startswith(
+                f"http://somewhere/ponos-artifacts/{self.task1.id}/some/path.txt"
+            )
+        )
+
+        # An artifact has been created
+        artifact = self.task1.artifacts.get(path='some/path.txt')
+
+        self.assertDictEqual(
+            response.json(),
+            {
+                "content_type": "text/plain",
+                "created": artifact.created.isoformat().replace("+00:00", "Z"),
+                "id": str(artifact.id),
+                "path": "some/path.txt",
+                "size": 1000,
+                "updated": artifact.updated.isoformat().replace("+00:00", "Z"),
+                "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/path.txt",
+            },
+        )
+
+    @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999)
+    def test_create_unique_path(self):
+        self.task1.agent = self.agent
+        self.task1.save()
+        self.task1.artifacts.create(
+            path="some/path.txt",
+            content_type="something",
+            size=42,
+        )
+
+        with self.assertNumQueries(3):
+            response = self.client.post(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                data={
+                    "path": "some/path.txt",
+                    "content_type": "text/plain",
+                    "size": 10243,
+                },
+                HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
+            )
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+        self.assertDictEqual(
+            response.json(), {"path": ["An artifact with this path already exists"]}
+        )
+
+    @override_settings()
+    def test_create_size_limits(self):
+        self.task1.agent = self.agent
+        self.task1.save()
+
+        params = [
+            (None, -42, "Ensure this value is greater than or equal to 1."),
+            (None, 0, "Ensure this value is greater than or equal to 1."),
+            (
+                None,
+                5 * 1024**3 + 1,
+                "Ensure this value is less than or equal to 5368709120.",
+            ),
+            (123456789000, -42, "Ensure this value is greater than or equal to 1."),
+            (123456789000, 0, "Ensure this value is greater than or equal to 1."),
+            (
+                123456789000,
+                987654321000,
+                "Ensure this value is less than or equal to 123456789000.",
+            ),
+        ]
+
+        for size_setting, size, expected_error in params:
+            with self.subTest(size=size), self.assertNumQueries(3):
+                settings.PONOS_ARTIFACT_MAX_SIZE = size_setting
+                response = self.client.post(
+                    reverse("api:task-artifacts", args=[self.task1.id]),
+                    data={
+                        "path": "some/path.txt",
+                        "content_type": "text/plain",
+                        "size": size,
+                    },
+                    HTTP_AUTHORIZATION=f"Bearer {self.agent.token.access_token}",
+                )
+                self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+                self.assertEqual(response.json(), {"size": [expected_error]})
+
+    def test_create_other_task(self):
+        with self.assertNumQueries(5):
+            response = self.client.post(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                data={
+                    "path": "some/path.txt",
+                    "content_type": "text/plain",
+                    "size": 1000,
+                },
+                HTTP_AUTHORIZATION=f"Ponos {self.task2.token}",
+            )
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999)
+    def test_create_task(self):
+        with self.assertNumQueries(5):
+            response = self.client.post(
+                reverse("api:task-artifacts", args=[self.task1.id]),
+                data={
+                    "path": "some/path.txt",
+                    "content_type": "text/plain",
+                    "size": 1000,
+                },
+                HTTP_AUTHORIZATION=f"Ponos {self.task1.token}",
+            )
+            self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+        data = response.json()
+        s3_put_url = data.pop("s3_put_url")
+        self.assertTrue(
+            s3_put_url.startswith(
+                f"http://somewhere/ponos-artifacts/{self.task1.id}/some/path.txt"
+            )
+        )
+
+        artifact = self.task1.artifacts.get(path='some/path.txt')
+        self.assertDictEqual(
+            data,
+            {
+                "content_type": "text/plain",
+                "created": artifact.created.isoformat().replace("+00:00", "Z"),
+                "id": str(artifact.id),
+                "path": "some/path.txt",
+                "size": 1000,
+                "updated": artifact.updated.isoformat().replace("+00:00", "Z"),
+                "url": f"http://testserver/api/v1/task/{self.task1.id}/artifact/some/path.txt",
+            },
+        )
+
+    def test_download_requires_login(self):
+        with self.assertNumQueries(0):
+            response = self.client.get(
+                reverse("api:task-artifact-download", args=[self.task1.id, "path/to/file.json"])
+            )
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_download_requires_verified(self):
+        self.user.verified_email = False
+        self.user.save()
+        self.client.force_login(self.user)
+
+        with self.assertNumQueries(3):
+            response = self.client.get(
+                reverse("api:task-artifact-download", args=[self.task1.id, "path/to/file.json"]),
+            )
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_download_requires_process_guest(self):
+        self.corpus.memberships.filter(user=self.user).delete()
+        self.corpus.public = False
+        self.corpus.save()
+        self.client.force_login(self.user)
+
+        with self.assertNumQueries(5):
+            response = self.client.get(
+                reverse("api:task-artifact-download", args=[self.task1.id, "path/to/file.json"]),
+            )
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+    def test_download_process_level_corpus(self):
+        self.client.force_login(self.user)
+
+        for role in Role:
+            with self.subTest(role=role):
+                self.corpus.memberships.filter(user=self.user).update(level=role.value)
+
+                with self.assertNumQueries(5):
+                    response = self.client.get(
+                        reverse("api:task-artifact-download", args=[self.task1.id, "path/to/file.json"]),
+                    )
+                    self.assertEqual(response.status_code, status.HTTP_302_FOUND)
+
+                self.assertTrue(response.has_header("Location"))
+                self.assertTrue(
+                    response["Location"].startswith(
+                        f"http://somewhere/ponos-artifacts/{self.task1.id}/path/to/file.json"
+                    )
+                )
+
+    def test_download_process_level_repo(self):
+        self.client.force_login(self.user)
+        membership = self.repo.memberships.create(user=self.user, level=Role.Guest.value)
+
+        task = self.repository_process.tasks.create(run=0, depth=0, slug='a')
+        task.artifacts.create(path='path/to/file.json', content_type='application/json', size=42)
+
+        for role in Role:
+            with self.subTest(role=role):
+                membership.level = role.value
+                membership.save()
+
+                with self.assertNumQueries(6):
+                    response = self.client.get(
+                        reverse("api:task-artifact-download", args=[task.id, "path/to/file.json"]),
+                    )
+                    self.assertEqual(response.status_code, status.HTTP_302_FOUND)
+
+                self.assertTrue(response.has_header("Location"))
+                self.assertTrue(
+                    response["Location"].startswith(
+                        f"http://somewhere/ponos-artifacts/{task.id}/path/to/file.json"
+                    )
+                )
+
+    def test_download_not_found(self):
+        with self.assertNumQueries(2):
+            response = self.client.get(
+                reverse("api:task-artifact-download", args=[self.task1.id, "nope.xxx"]),
+                HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
+            )
+            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+    def test_download_agent(self):
+        # Being assigned to the task is not required
+        self.assertNotEqual(self.task1.agent, self.agent)
+
+        with self.assertNumQueries(2):
+            response = self.client.get(
+                reverse("api:task-artifact-download", args=[self.task1.id, "path/to/file.json"]),
+                HTTP_AUTHORIZATION=f'Bearer {self.agent.token.access_token}',
+            )
+            self.assertEqual(response.status_code, status.HTTP_302_FOUND)
+
+        self.assertTrue(response.has_header("Location"))
+        self.assertTrue(
+            response["Location"].startswith(
+                f"http://somewhere/ponos-artifacts/{self.task1.id}/path/to/file.json"
+            )
+        )
+
+    def test_download_task(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.task1.token}',
+            )
+            self.assertEqual(response.status_code, status.HTTP_302_FOUND)
+
+        self.assertTrue(response.has_header("Location"))
+        self.assertTrue(
+            response["Location"].startswith(
+                f"http://somewhere/ponos-artifacts/{self.task1.id}/path/to/file.json"
+            )
+        )
+
+    def test_download_other_task(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://somewhere/ponos-artifacts/{self.task1.id}/path/to/file.json"
+            )
+        )
diff --git a/arkindex/project/tests/test_ponos_view.py b/arkindex/project/tests/test_ponos_view.py
index f4f3cd4400..3091f790bf 100644
--- a/arkindex/project/tests/test_ponos_view.py
+++ b/arkindex/project/tests/test_ponos_view.py
@@ -3,8 +3,7 @@ from django.urls import reverse
 from rest_framework import status
 
 from arkindex.documents.models import Corpus
-from arkindex.ponos.authentication import AgentUser
-from arkindex.ponos.models import Artifact, Farm
+from arkindex.ponos.models import Artifact
 from arkindex.process.models import ProcessMode
 from arkindex.project.tests import FixtureAPITestCase
 from arkindex.users.models import Role, User
@@ -51,47 +50,3 @@ class TestPonosView(FixtureAPITestCase):
                         json={'state': 'stopping'}
                     )
                     self.assertEqual(response.status_code, status_code)
-
-    def test_download_artifacts(self):
-        """
-        Only users with an admin privilege have the ability to
-        download an artifact of a process task
-        """
-        test_cases = (
-            (None, status.HTTP_403_FORBIDDEN, 0),
-            (self.creator, status.HTTP_403_FORBIDDEN, 9),
-            (self.user, status.HTTP_403_FORBIDDEN, 9),
-            (self.superuser, status.HTTP_302_FOUND, 4),
-            (self.corpus_admin, status.HTTP_302_FOUND, 9),
-        )
-        for user, status_code, requests_count in test_cases:
-            with self.subTest(user=user):
-                if user:
-                    self.client.force_login(user)
-                with self.assertNumQueries(requests_count):
-                    response = self.client.get(
-                        reverse('api:task-artifact-download', kwargs={'pk': str(self.task.id), 'path': self.artifact.path}),
-                        follow=False
-                    )
-                    self.assertEqual(response.status_code, status_code)
-
-    def test_download_artifacts_by_agent(self):
-        """
-        Agents should still be able to download artifacts in order
-        to run followup tasks
-        """
-        agent_user = AgentUser.objects.create(
-            cpu_cores=3,
-            cpu_frequency=3e9,
-            farm_id=Farm.objects.create().id,
-            ram_total=2e9,
-            last_ping='1999-09-09'
-        )
-
-        with self.assertNumQueries(3):
-            response = self.client.get(
-                reverse('api:task-artifact-download', kwargs={'pk': str(self.task.id), 'path': self.artifact.path}),
-                follow=False,
-                HTTP_AUTHORIZATION="Bearer {}".format(agent_user.token.access_token),
-            )
-            self.assertEqual(response.status_code, status.HTTP_302_FOUND)
-- 
GitLab