diff --git a/arkindex/documents/serializers/elements.py b/arkindex/documents/serializers/elements.py
index 4222394c9750c005f17a9f75c60100721d46556f..91bbbdc8220676750893b04256766e9c64e35b88 100644
--- a/arkindex/documents/serializers/elements.py
+++ b/arkindex/documents/serializers/elements.py
@@ -895,12 +895,14 @@ class ElementBulkSerializer(serializers.Serializer):
Serialize child elements on a common parent
"""
worker_version = ForbiddenField()
- worker_run_id = serializers.PrimaryKeyRelatedField(
- queryset=WorkerRun.objects.all(),
- help_text='A WorkerRun ID that the new elements will refer to',
- source='worker_run',
- # Avoids thousands of SQL queries when opening the endpoint in a browser
- style={'base_template': 'input.html'},
+ worker_run_id = WorkerRunIDField(
+ help_text=dedent("""
+ A WorkerRun ID that the new elements will refer to.
+
+ Regular users may only use the WorkerRuns of their own `Local` process.
+
+ Tasks authenticated via the Ponos task authentication may only use the WorkerRuns of their process.
+ """).strip(),
)
elements = ElementBulkItemSerializer(
diff --git a/arkindex/documents/tests/test_bulk_elements.py b/arkindex/documents/tests/test_bulk_elements.py
index f849b7658455f696cb6b88a47ebd29b927b664d9..0e280a0c7e2ec5358fa33b347d7643b04759f17f 100644
--- a/arkindex/documents/tests/test_bulk_elements.py
+++ b/arkindex/documents/tests/test_bulk_elements.py
@@ -18,8 +18,9 @@ class TestBulkElements(FixtureAPITestCase):
cls.worker_version = WorkerVersion.objects.get(worker__slug='reco')
cls.worker_run = cls.worker_version.worker_runs.filter(process__mode=ProcessMode.Workers).get()
+ cls.local_worker_run = cls.worker_version.worker_runs.filter(process__mode=ProcessMode.Local).get()
cls.payload = {
- 'worker_run_id': str(cls.worker_run.id),
+ 'worker_run_id': str(cls.local_worker_run.id),
'elements': [
{
'name': 'A',
@@ -111,12 +112,12 @@ class TestBulkElements(FixtureAPITestCase):
})
def test_bulk_create_requires_elements(self):
- self.client.force_login(self.user)
- with self.assertNumQueries(6):
+ self.client.force_login(self.superuser)
+ with self.assertNumQueries(4):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
data={
- 'worker_run_id': str(self.worker_run.id),
+ 'worker_run_id': str(self.local_worker_run.id),
'elements': [],
},
format='json',
@@ -125,12 +126,12 @@ class TestBulkElements(FixtureAPITestCase):
self.assertDictEqual(response.json(), {'elements': {'non_field_errors': ['This list may not be empty.']}})
def test_bulk_create_missing_types(self):
- self.client.force_login(self.user)
- with self.assertNumQueries(7):
+ self.client.force_login(self.superuser)
+ with self.assertNumQueries(5):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
data={
- 'worker_run_id': str(self.worker_run.id),
+ 'worker_run_id': str(self.local_worker_run.id),
'elements': [
{
'name': 'nope',
@@ -155,8 +156,8 @@ class TestBulkElements(FixtureAPITestCase):
self.element.add_parent(parent)
element_path1, element_path2 = self.element.paths.order_by('path__0')
- self.client.force_login(self.user)
- with self.assertNumQueries(13):
+ self.client.force_login(self.superuser)
+ with self.assertNumQueries(11):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
data=self.payload,
@@ -200,12 +201,12 @@ class TestBulkElements(FixtureAPITestCase):
self.assertFalse(element.paths.exists())
# Try to create a sub element on that zone
- self.client.force_login(self.user)
- with self.assertNumQueries(13):
+ self.client.force_login(self.superuser)
+ with self.assertNumQueries(11):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(element.id)}),
data={
- 'worker_run_id': str(self.worker_run.id),
+ 'worker_run_id': str(self.local_worker_run.id),
'elements': [
{
'name': f'surface {i}',
@@ -231,12 +232,12 @@ class TestBulkElements(FixtureAPITestCase):
self.assertEqual(paths.count(), 4)
def test_bulk_create_negative_polygon(self):
- self.client.force_login(self.user)
- with self.assertNumQueries(6):
+ self.client.force_login(self.superuser)
+ with self.assertNumQueries(4):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
{
- 'worker_run_id': str(self.worker_run.id),
+ 'worker_run_id': str(self.local_worker_run.id),
'elements': [
{
'name': 'Blah',
@@ -264,7 +265,7 @@ class TestBulkElements(FixtureAPITestCase):
"""
Created children elements inherit their parent's rotation angle
"""
- self.client.force_login(self.user)
+ self.client.force_login(self.superuser)
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.rotated_page.id)}),
data=self.payload,
@@ -279,7 +280,7 @@ class TestBulkElements(FixtureAPITestCase):
"""
Created children elements inherit their parent's mirroring
"""
- self.client.force_login(self.user)
+ self.client.force_login(self.superuser)
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.mirrored_page.id)}),
data=self.payload,
@@ -294,9 +295,9 @@ class TestBulkElements(FixtureAPITestCase):
"""
Cannot create elements outside their image
"""
- self.client.force_login(self.user)
+ self.client.force_login(self.superuser)
payload = {
- 'worker_run_id': str(self.worker_run.id),
+ 'worker_run_id': str(self.local_worker_run.id),
'elements': [
{
'name': 'A',
@@ -335,7 +336,7 @@ class TestBulkElements(FixtureAPITestCase):
}
]
}
- with self.assertNumQueries(7):
+ with self.assertNumQueries(5):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
data=payload,
@@ -372,15 +373,17 @@ class TestBulkElements(FixtureAPITestCase):
'worker_run_id': [f'Invalid pk "{random_uuid}" - object does not exist.']
})
- def test_bulk_create_worker_run_or_version(self):
- """Either a worker run or a worker version is required"""
-
+ def test_bulk_create_worker_run_required(self):
+ """
+ Worker run attribute is required, worker version attribute is forbidden
+ """
self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
data={
- 'elements': [
+ "worker_version": str(self.worker_version.id),
+ "elements": [
{
'name': 'Blah',
'type': 'surface',
@@ -393,59 +396,75 @@ class TestBulkElements(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {
'worker_run_id': ['This field is required.'],
+ 'worker_version': ['This field is forbidden.'],
})
- def test_bulk_create_worker_run_and_version(self):
- """Worker run and worker version cannot be set at the same time"""
+ def test_worker_run_non_local(self):
+ """
+ A regular user cannot create elements with a WorkerRun of a non-local process
+ """
+ self.client.force_login(self.superuser)
+ payload = {**self.payload, 'worker_run_id': str(self.worker_run.id)}
+ with self.assertNumQueries(4):
+ response = self.client.post(
+ reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
+ data=payload,
+ format='json',
+ )
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+ self.assertEqual(response.json(), {'worker_run_id': [
+ "Ponos task authentication is required to use a WorkerRun of a process other than the user's local process."
+ ]})
- self.client.force_login(self.user)
- with self.assertNumQueries(6):
+ def test_worker_run_other_user(self):
+ """
+ A regular user cannot create elements with a WorkerRun of someone else's local process
+ """
+ worker_run = self.user.processes.get(mode=ProcessMode.Local).worker_runs.first()
+ self.client.force_login(self.superuser)
+ payload = {**self.payload, 'worker_run_id': str(worker_run.id)}
+ with self.assertNumQueries(4):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
- {
- 'worker_version': str(self.worker_version.id),
- 'worker_run_id': str(self.worker_run.id),
- 'elements': [
- {
- 'name': 'Blah',
- 'type': 'surface',
- 'polygon': [[0, 10], [10, 20], [30, 40], [50, 60], [0, 10]]
- }
- ]
- },
+ data=payload,
format='json',
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
- self.assertDictEqual(response.json(), {
- 'worker_version': ['This field is forbidden.'],
- })
+ self.assertEqual(response.json(), {'worker_run_id': [
+ "Ponos task authentication is required to use a WorkerRun of a process other than the user's local process."
+ ]})
+
+ def test_worker_run_other_process(self):
+ """
+ A Ponos task cannot create elements with a WorkerRun of another process
+ """
+ process2 = self.worker_run.process.creator.processes.create(
+ mode=ProcessMode.Workers,
+ corpus=self.corpus,
+ )
+ other_worker_run = process2.worker_runs.create(version=self.worker_run.version, parents=[])
+ self.worker_run.process.start()
+ task = self.worker_run.process.workflow.tasks.first()
+ payload = {**self.payload, 'worker_run_id': str(other_worker_run.id)}
- def test_bulk_create_with_worker_version(self):
- self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
- data={
- "worker_version": str(self.worker_version.id),
- "elements": [
- {
- 'name': 'Blah',
- 'type': 'surface',
- 'polygon': [[0, 10], [10, 20], [30, 40], [50, 60], [0, 10]]
- }
- ]
- },
+ data=payload,
format='json',
+ HTTP_AUTHORIZATION=f'Ponos {task.token}',
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
- self.assertDictEqual(response.json(), {
- 'worker_run_id': ['This field is required.'],
- 'worker_version': ['This field is forbidden.'],
- })
+ self.assertEqual(response.json(), {'worker_run_id': [
+ "Only the WorkerRuns of the authenticated task's process may be used."
+ ]})
- def test_bulk_create_with_worker_run(self):
- self.client.force_login(self.user)
- with self.assertNumQueries(13):
+ def test_bulk_create_local_worker_run(self):
+ """
+ A regular user can create elements with a WorkerRun of their own local process
+ """
+ self.client.force_login(self.superuser)
+ with self.assertNumQueries(11):
response = self.client.post(
reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
data=self.payload,
@@ -473,7 +492,7 @@ class TestBulkElements(FixtureAPITestCase):
'mirrored',
'paths__path',
)),
- 3 * [(self.worker_version.id, self.worker_run.id, self.element.image_id, 0, False, [*parent_path, self.element.id])]
+ 3 * [(self.worker_version.id, self.local_worker_run.id, self.element.image_id, 0, False, [*parent_path, self.element.id])]
)
# Test specific attributes
self.assertListEqual(
@@ -484,3 +503,99 @@ class TestBulkElements(FixtureAPITestCase):
('C', 'surface', 1, LineString(self.payload['elements'][2]['polygon'])),
],
)
+
+ def test_bulk_create_task_auth(self):
+ """
+ Elements can be created with a WorkerRun of a non-local process
+ when authenticated as a Ponos task of this process
+ """
+ self.worker_run.process.start()
+ task = self.worker_run.process.workflow.tasks.first()
+ payload = {**self.payload, 'worker_run_id': str(self.worker_run.id)}
+
+ with self.assertNumQueries(12):
+ response = self.client.post(
+ reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
+ data=payload,
+ format='json',
+ HTTP_AUTHORIZATION=f'Ponos {task.token}',
+ )
+ self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+ elements = (
+ Element.objects
+ .get_descending(self.element.id)
+ .filter(type__slug__in=('act', 'surface'))
+ .order_by('name')
+ )
+ element_ids = list(elements.values_list("id", flat=True))
+ # Ensure elements have been created with valid attributes
+ self.assertListEqual(response.json(), [{'id': str(elt_id)} for elt_id in element_ids])
+ parent_path = self.element.paths.get().path
+ self.assertListEqual(
+ list(elements.values_list(
+ 'worker_version_id',
+ 'worker_run_id',
+ 'image_id',
+ 'rotation_angle',
+ 'mirrored',
+ 'paths__path',
+ )),
+ 3 * [(
+ self.worker_version.id,
+ self.worker_run.id,
+ self.element.image_id,
+ 0,
+ False,
+ [*parent_path, self.element.id]
+ )]
+ )
+
+ def test_worker_run_local_task_auth(self):
+ """
+ Elements can be created with a WorkerRun of a Local process
+ even when authenticated as a Ponos task from a different process
+ """
+ local_process = self.user.processes.get(mode=ProcessMode.Local)
+ local_worker_run = local_process.worker_runs.get()
+
+ self.worker_run.process.start()
+ task = self.worker_run.process.workflow.tasks.first()
+
+ payload = {**self.payload, 'worker_run_id': str(local_worker_run.id)}
+
+ with self.assertNumQueries(12):
+ response = self.client.post(
+ reverse('api:elements-bulk-create', kwargs={'pk': str(self.element.id)}),
+ data=payload,
+ format='json',
+ HTTP_AUTHORIZATION=f'Ponos {task.token}',
+ )
+ self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+ elements = (
+ Element.objects
+ .get_descending(self.element.id)
+ .filter(type__slug__in=('act', 'surface'))
+ .order_by('name')
+ )
+ element_ids = list(elements.values_list("id", flat=True))
+ # Ensure elements have been created with valid attributes
+ self.assertListEqual(response.json(), [{'id': str(elt_id)} for elt_id in element_ids])
+ parent_path = self.element.paths.get().path
+ self.assertListEqual(
+ list(elements.values_list(
+ 'worker_version_id',
+ 'worker_run_id',
+ 'image_id',
+ 'rotation_angle',
+ 'mirrored',
+ 'paths__path',
+ )),
+ 3 * [(
+ local_worker_run.version_id,
+ local_worker_run.id,
+ self.element.image_id,
+ 0,
+ False,
+ [*parent_path, self.element.id]
+ )]
+ )