diff --git a/arkindex/ponos/api.py b/arkindex/ponos/api.py
index 8fa0542c1ce052e1952782c0af45b14972d873cc..4cdee3e3bebb4b3d76386a47f57150f6ad63ce35 100644
--- a/arkindex/ponos/api.py
+++ b/arkindex/ponos/api.py
@@ -26,11 +26,11 @@ from arkindex.ponos.models import Agent, Artifact, Farm, Secret, State, Task
from arkindex.ponos.permissions import (
IsAgent,
IsAgentOrArtifactGuest,
- IsAgentOrTaskAdmin,
IsAgentOrTaskGuest,
IsAssignedAgentOrReadOnly,
IsAssignedAgentOrTaskOrReadOnly,
IsTask,
+ IsTaskAdmin,
)
from arkindex.ponos.renderers import PublicKeyPEMRenderer
from arkindex.ponos.serializers import (
@@ -364,7 +364,9 @@ class TaskArtifactDownload(APIView):
)
class TaskCreate(CreateAPIView):
"""
- Create a task with a parent
+ Create a task that depends on an existing task.
+
+ Requires authentication as a Ponos task. Tasks can only be created on the process of the authenticated task.
"""
authentication_classes = (TaskAuthentication, )
@@ -375,18 +377,28 @@ class TaskCreate(CreateAPIView):
@extend_schema(tags=["ponos"])
@extend_schema_view(
put=extend_schema(
- description="Update a task, allowing humans to change the task's state"
+ description=dedent("""
+ Update a task.
+
+ Requires **admin** access on the task's process, or to be the creator of the process.
+ Cannot be used with Ponos agent or task authentication.
+ """),
),
patch=extend_schema(
- description="Partially update a task, allowing humans to change the task's state"
+ description=dedent("""
+ Partially update a task.
+
+ Requires **admin** access on the task's process, or to be the creator of the process.
+ Cannot be used with Ponos agent or task authentication.
+ """),
),
)
class TaskUpdate(UpdateAPIView):
- """
- Admins and task creators can use this endpoint to update a task
- """
- permission_classes = (IsAgentOrTaskAdmin, )
- queryset = Task.objects.all()
+ # Only allow regular users, not Ponos agents or tasks
+ authentication_classes = (TokenAuthentication, SessionAuthentication)
+ # Only allow regular users that have admin access to the task's process
+ permission_classes = (IsTaskAdmin, )
+ queryset = Task.objects.select_related('process__corpus', 'process__revision__repo')
serializer_class = TaskTinySerializer
diff --git a/arkindex/ponos/permissions.py b/arkindex/ponos/permissions.py
index f8624fd93a5a3260d4058384730392ccb379b353..c27e8d3b5e6e4a04e4b87149c5d279c38846fda8 100644
--- a/arkindex/ponos/permissions.py
+++ b/arkindex/ponos/permissions.py
@@ -1,8 +1,8 @@
from rest_framework.permissions import SAFE_METHODS
from arkindex.ponos.models import Task
-from arkindex.project.mixins import CorpusACLMixin, ProcessACLMixin
-from arkindex.project.permissions import IsAuthenticated
+from arkindex.project.mixins import ProcessACLMixin
+from arkindex.project.permissions import IsAuthenticated, IsVerified
from arkindex.users.models import Role
@@ -79,26 +79,17 @@ class IsAssignedAgentOrTaskOrReadOnly(IsAgentOrTask):
)
-class IsAgentOrTaskAdmin(CorpusACLMixin, IsAuthenticated):
+class IsTaskAdmin(ProcessACLMixin, IsVerified):
"""
- Permission to access a task with high privilege
-
- Allowed for admins, agents, creators of the task's process,
- and users with an admin right on the process' corpus.
+ Allow instance admins and users with a verified email and admin access to the task's process.
"""
def has_object_permission(self, request, view, task):
# Add request to attributes for the ACL mixin to work with self.user
self.request = request
-
- return (
- require_agent_or_admin(request, view)
- or (
- task.process is not None
- and task.process.corpus_id is not None
- and self.has_admin_access(task.process.corpus)
- )
- )
+ level = self.process_access_level(task.process)
+ # process_access_level can return None if there is no access at all
+ return level and level >= Role.Admin.value
class IsAgentOrTaskGuest(ProcessACLMixin, IsAuthenticated):
diff --git a/arkindex/ponos/tests/test_api.py b/arkindex/ponos/tests/test_api.py
index 31501484a2e335bcf0b297004cbbe5c0fbd0a0ac..0e4717cc0386b6ca65712cfe7e2c734ff7ba5bf2 100644
--- a/arkindex/ponos/tests/test_api.py
+++ b/arkindex/ponos/tests/test_api.py
@@ -733,6 +733,85 @@ class TestAPI(FixtureAPITestCase):
# Both parents are completed, the child task is now pending
self.assertEqual(self.task3.state, State.Pending)
+ def test_update_task_requires_login(self):
+ with self.assertNumQueries(0):
+ resp = self.client.put(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ )
+ self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED)
+
+ def test_update_task_requires_verified(self):
+ self.user.verified_email = False
+ self.user.save()
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(2):
+ resp = self.client.put(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ )
+ self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+
+ def test_update_task_forbids_agent(self):
+ with self.assertNumQueries(0):
+ resp = self.client.put(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ HTTP_AUTHORIZATION=f"Bearer {self.agent.token.access_token}",
+ )
+ self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED)
+
+ def test_update_task_forbids_task(self):
+ with self.assertNumQueries(0):
+ resp = self.client.put(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ HTTP_AUTHORIZATION=f"Ponos {self.task1.token}",
+ )
+ self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED)
+
+ def test_update_task_requires_process_admin_corpus(self):
+ self.process.creator = self.superuser
+ self.process.save()
+ self.corpus.public = False
+ self.corpus.save()
+ self.client.force_login(self.user)
+
+ for role in [None, Role.Guest, Role.Contributor]:
+ with self.subTest(role=role):
+ self.corpus.memberships.filter(user=self.user).delete()
+ if role:
+ self.corpus.memberships.create(user=self.user, level=role.value)
+
+ with self.assertNumQueries(5):
+ resp = self.client.put(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ )
+ self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+
+ def test_update_task_requires_process_admin_repo(self):
+ self.process.mode = ProcessMode.Repository
+ self.process.corpus = None
+ self.process.revision = self.rev
+ self.process.creator = self.superuser
+ self.process.save()
+ self.client.force_login(self.user)
+
+ for role in [None, Role.Guest, Role.Contributor]:
+ with self.subTest(role=role):
+ self.rev.repo.memberships.filter(user=self.user).delete()
+ if role:
+ self.rev.repo.memberships.create(user=self.user, level=role.value)
+
+ with self.assertNumQueries(5):
+ resp = self.client.put(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ )
+ self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+
def test_update_running_task_state_stopping(self):
self.task1.state = State.Running
self.task1.save()
@@ -780,7 +859,7 @@ class TestAPI(FixtureAPITestCase):
self.task1.save()
self.client.force_login(self.superuser)
- with self.assertNumQueries(6):
+ with self.assertNumQueries(5):
resp = self.client.put(
reverse("api:task-update", args=[self.task1.id]),
data={"state": State.Pending.value},
@@ -920,6 +999,85 @@ class TestAPI(FixtureAPITestCase):
# Both parents are completed, the child task is now pending
self.assertEqual(self.task3.state, State.Pending)
+ def test_partial_update_task_requires_login(self):
+ with self.assertNumQueries(0):
+ resp = self.client.patch(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ )
+ self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED)
+
+ def test_partial_update_task_requires_verified(self):
+ self.user.verified_email = False
+ self.user.save()
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(2):
+ resp = self.client.patch(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ )
+ self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+
+ def test_partial_update_task_forbids_agent(self):
+ with self.assertNumQueries(0):
+ resp = self.client.patch(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ HTTP_AUTHORIZATION=f"Bearer {self.agent.token.access_token}",
+ )
+ self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED)
+
+ def test_partial_update_task_forbids_task(self):
+ with self.assertNumQueries(0):
+ resp = self.client.patch(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ HTTP_AUTHORIZATION=f"Ponos {self.task1.token}",
+ )
+ self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED)
+
+ def test_partial_update_task_requires_process_admin_corpus(self):
+ self.process.creator = self.superuser
+ self.process.save()
+ self.corpus.public = False
+ self.corpus.save()
+ self.client.force_login(self.user)
+
+ for role in [None, Role.Guest, Role.Contributor]:
+ with self.subTest(role=role):
+ self.corpus.memberships.filter(user=self.user).delete()
+ if role:
+ self.corpus.memberships.create(user=self.user, level=role.value)
+
+ with self.assertNumQueries(5):
+ resp = self.client.patch(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ )
+ self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+
+ def test_partial_update_task_requires_process_admin_repo(self):
+ self.process.mode = ProcessMode.Repository
+ self.process.corpus = None
+ self.process.revision = self.rev
+ self.process.creator = self.superuser
+ self.process.save()
+ self.client.force_login(self.user)
+
+ for role in [None, Role.Guest, Role.Contributor]:
+ with self.subTest(role=role):
+ self.rev.repo.memberships.filter(user=self.user).delete()
+ if role:
+ self.rev.repo.memberships.create(user=self.user, level=role.value)
+
+ with self.assertNumQueries(5):
+ resp = self.client.patch(
+ reverse("api:task-update", args=[self.task1.id]),
+ data={"state": State.Stopping.value},
+ )
+ self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+
def test_partial_update_running_task_state_stopping(self):
self.task1.state = State.Running
self.task1.save()
@@ -967,7 +1125,7 @@ class TestAPI(FixtureAPITestCase):
self.task1.save()
self.client.force_login(self.superuser)
- with self.assertNumQueries(6):
+ with self.assertNumQueries(5):
resp = self.client.patch(
reverse("api:task-update", args=[self.task1.id]),
data={"state": State.Pending.value},
diff --git a/arkindex/project/tests/test_ponos_view.py b/arkindex/project/tests/test_ponos_view.py
deleted file mode 100644
index 3091f790bf60c799ceab7f7448ea66ae0125b5f5..0000000000000000000000000000000000000000
--- a/arkindex/project/tests/test_ponos_view.py
+++ /dev/null
@@ -1,52 +0,0 @@
-from django.test import override_settings
-from django.urls import reverse
-from rest_framework import status
-
-from arkindex.documents.models import Corpus
-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
-
-
-@override_settings(PONOS_PRIVATE_KEY='staging')
-class TestPonosView(FixtureAPITestCase):
-
- @classmethod
- def setUpTestData(cls):
- super().setUpTestData()
- cls.creator = User.objects.create(email="creator@user.me")
- cls.artifact = Artifact.objects.get(path='/path/to/docker_build')
- cls.task = cls.artifact.task
-
- # Assign a corpus to the task's process so we can test the process corpus permissions
- cls.process_corpus = Corpus.objects.create(name='Another public corpus', public=True)
- cls.corpus_admin = User.objects.create(email='corpusadmin@test.me')
- cls.corpus_admin.rights.create(content_object=cls.process_corpus, level=Role.Admin.value)
-
- process = cls.task.process
- process.mode = ProcessMode.Files
- process.corpus = cls.process_corpus
- process.save()
-
- def test_update_task(self):
- """
- Only users with an admin privilege have the ability to update a process task
- """
- test_cases = (
- (None, status.HTTP_403_FORBIDDEN, 0),
- (self.creator, status.HTTP_403_FORBIDDEN, 8),
- (self.user, status.HTTP_403_FORBIDDEN, 8),
- (self.superuser, status.HTTP_200_OK, 10),
- (self.corpus_admin, status.HTTP_200_OK, 14),
- )
- 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.patch(
- reverse('api:task-update', kwargs={'pk': str(self.task.id)}),
- json={'state': 'stopping'}
- )
- self.assertEqual(response.status_code, status_code)