diff --git a/arkindex/process/api.py b/arkindex/process/api.py index c0b61314091252adaabfad79ba772c1b011e3d80..e753d3214f31710109cd5d62778b107be6645f5d 100644 --- a/arkindex/process/api.py +++ b/arkindex/process/api.py @@ -2014,8 +2014,15 @@ class CreateProcessTemplate(ProcessACLMixin, WorkerACLMixin, CreateAPIView): serializer_class = CreateProcessTemplateSerializer def get_queryset(self): - return Process.objects \ - .prefetch_related(Prefetch("worker_runs", queryset=WorkerRun.objects.select_related("version__worker__type"))) + return ( + Process + .objects + .prefetch_related(Prefetch("worker_runs", queryset=WorkerRun.objects.select_related( + "version__worker__type", + "version__worker__repository", + ))) + .select_related("corpus") + ) def check_object_permissions(self, request, process): access_level = self.process_access_level(process) @@ -2077,9 +2084,16 @@ class ApplyProcessTemplate(ProcessACLMixin, WorkerACLMixin, CreateAPIView): serializer_class = ApplyProcessTemplateSerializer def get_queryset(self): - return Process.objects \ - .filter(mode=ProcessMode.Template) \ - .prefetch_related(Prefetch("worker_runs", queryset=WorkerRun.objects.select_related("version__worker__type", "model_version__model"))) + return ( + Process.objects + .filter(mode=ProcessMode.Template) + .prefetch_related(Prefetch("worker_runs", queryset=WorkerRun.objects.select_related( + "version__worker__type", + "version__worker__repository", + "model_version__model", + ))) + .select_related("corpus") + ) def check_object_permissions(self, request, template): access_level = self.process_access_level(template) diff --git a/arkindex/process/serializers/imports.py b/arkindex/process/serializers/imports.py index 830e01541b8814e63ec65e34fe137caa5280c8e9..2c659da43c4d91e865e0bd9bdea8d21f933ecdd4 100644 --- a/arkindex/process/serializers/imports.py +++ b/arkindex/process/serializers/imports.py @@ -522,7 +522,7 @@ class CreateProcessTemplateSerializer(serializers.ModelSerializer): class ApplyProcessTemplateSerializer(ProcessACLMixin, serializers.Serializer): process_id = serializers.PrimaryKeyRelatedField( - queryset=Process.objects.all(), + queryset=Process.objects.select_related("corpus"), source="process", required=True, help_text="ID of the process to apply the template to", diff --git a/arkindex/process/tests/test_processes.py b/arkindex/process/tests/test_processes.py index 31d7de782d22fda2b928677b48d2f013b07d6727..f216464246d01b1186545f790a11a2cbfb39f3c0 100644 --- a/arkindex/process/tests/test_processes.py +++ b/arkindex/process/tests/test_processes.py @@ -1,5 +1,4 @@ import uuid -from unittest import expectedFailure from unittest.mock import call, patch from django.conf import settings @@ -753,16 +752,35 @@ class TestProcesses(FixtureAPITestCase): {"__all__": ["Please wait activities to be initialized before deleting this process"]} ) - @expectedFailure - def test_delete_process_no_permission(self): + @patch("arkindex.project.mixins.get_max_level", return_value=None) + def test_delete_process_no_permission(self, get_max_level_mock): """ - A user cannot delete a process linked to a corpus they have no admin access to + A user cannot delete a process linked to a corpus they have no access to """ self.client.force_login(self.user) - self.assertFalse(self.user_img_process.corpus.memberships.filter(user=self.user).exists()) - response = self.client.delete(reverse("api:process-details", kwargs={"pk": self.user_img_process.id})) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + with self.assertNumQueries(4): + response = self.client.delete(reverse("api:process-details", kwargs={"pk": self.user_img_process.id})) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertDictEqual(response.json(), {"detail": "Not found."}) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus)) + + @patch("arkindex.project.mixins.get_max_level", return_value=Role.Contributor.value) + def test_delete_process_not_admin(self, get_max_level_mock): + """ + A user cannot delete a process linked to a corpus they have no admin access to + """ + self.client.force_login(self.user) + + with self.assertNumQueries(4): + response = self.client.delete(reverse("api:process-details", kwargs={"pk": self.user_img_process.id})) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + self.assertDictEqual(response.json(), {"detail": "You do not have a sufficient access level to this process."}) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus)) @patch("arkindex.project.triggers.process_tasks.process_delete.delay") def test_delete_worker_run_in_use(self, process_delay_mock): @@ -944,16 +962,20 @@ class TestProcesses(FixtureAPITestCase): process.refresh_from_db() self.assertEqual(process.element, element) - @expectedFailure - def test_partial_update_no_permission(self): + @patch("arkindex.project.mixins.get_max_level", return_value=None) + def test_partial_update_no_permission(self, get_max_level_mock): """ - A user cannot update a process linked to a corpus he has no admin access to + A user cannot update a process linked to a corpus they do not have admin access to """ self.client.force_login(self.user) - self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value) - response = self.client.patch(reverse("api:process-details", kwargs={"pk": self.elts_process.id})) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertDictEqual(response.json(), {"detail": "You do not have a sufficient access level to this process."}) + + with self.assertNumQueries(5): + response = self.client.patch(reverse("api:process-details", kwargs={"pk": self.elts_process.id})) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + self.assertDictEqual(response.json(), {"detail": "Not found."}) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(self.user, self.corpus)) def test_partial_update_stop(self): """ @@ -1374,13 +1396,17 @@ class TestProcesses(FixtureAPITestCase): process.refresh_from_db() self.assertEqual(process.name, "newName") - @expectedFailure - def test_partial_update_corpus_no_write_right(self): + @patch("arkindex.project.mixins.get_max_level", return_value=Role.Guest.value) + def test_partial_update_corpus_no_write_right(self, get_max_level_mock): self.client.force_login(self.user) - self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value) - response = self.client.patch(reverse("api:process-details", kwargs={"pk": self.elts_process.id})) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + with self.assertNumQueries(5): + response = self.client.patch(reverse("api:process-details", kwargs={"pk": self.elts_process.id})) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual(response.json(), {"detail": "You do not have a sufficient access level to this process."}) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(self.user, self.corpus)) def test_partial_update(self): """ @@ -1584,24 +1610,27 @@ class TestProcesses(FixtureAPITestCase): # Activity initialization runs again self.assertFalse(delay_mock.called) - @expectedFailure - def test_retry_farm_guest(self): + @patch("arkindex.ponos.models.Farm.is_available", return_value=False) + def test_retry_farm_unavailable(self, is_available_mock): self.elts_process.run() self.elts_process.tasks.all().update(state=State.Error) self.elts_process.finished = timezone.now() + self.elts_process.farm = Farm.objects.get(name="Wheat farm") self.elts_process.save() self.assertEqual(self.elts_process.state, State.Error) self.elts_process.farm.memberships.filter(user=self.user).delete() self.client.force_login(self.user) - with self.assertNumQueries(10): + with self.assertNumQueries(6): response = self.client.post(reverse("api:process-retry", kwargs={"pk": self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { "farm": ["You do not have access to this farm."], }) + self.assertEqual(is_available_mock.call_count, 1) + self.assertEqual(is_available_mock.call_args, call(self.user)) @override_settings(PUBLIC_HOSTNAME="https://darkindex.lol") def test_retry_archived_worker(self): @@ -2047,27 +2076,28 @@ class TestProcesses(FixtureAPITestCase): process = Process.objects.get(id=data["id"]) self.assertEqual(process.farm, farm) - @expectedFailure - def test_from_files_farm_guest(self): + @patch("arkindex.ponos.models.Farm.is_available", return_value=False) + def test_from_files_farm_guest(self, is_available_mock): self.client.force_login(self.user) self.assertEqual(self.version_with_model.worker_runs.count(), 0) - self.other_farm.memberships.filter(user=self.user).delete() with ( self.settings(IMPORTS_WORKER_VERSION=str(self.version_with_model.id)), - self.assertNumQueries(9) + self.assertNumQueries(6) ): response = self.client.post(reverse("api:files-process"), { "files": [str(self.img_df.id)], "folder_type": "volume", "element_type": "page", - "farm_id": str(self.other_farm.id), + "farm_id": str(Farm.objects.first().id), }, format="json") self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { "farm_id": ["You do not have access to this farm."], }) + self.assertEqual(is_available_mock.call_count, 1) + self.assertEqual(is_available_mock.call_args, call(self.user)) def test_start_process_requires_login(self): response = self.client.post( @@ -2442,14 +2472,15 @@ class TestProcesses(FixtureAPITestCase): self.assertEqual(workers_process.state, State.Unscheduled) self.assertEqual(workers_process.farm_id, farm.id) - @expectedFailure - def test_start_process_default_farm_guest(self): + @patch("arkindex.ponos.models.Farm.is_available", return_value=False) + @patch("arkindex.process.serializers.imports.get_default_farm") + def test_start_process_default_farm_guest(self, get_default_farm_mock, is_available_mock): + get_default_farm_mock.return_value = Farm.objects.first() workers_process = self.corpus.processes.create(creator=self.user, mode=ProcessMode.Workers) workers_process.worker_runs.create(version=self.recognizer, parents=[], configuration=None) self.client.force_login(self.user) - self.default_farm.memberships.filter(user=self.user).delete() - with self.assertNumQueries(10): + with self.assertNumQueries(5): response = self.client.post( reverse("api:process-start", kwargs={"pk": str(workers_process.id)}) ) @@ -2461,18 +2492,20 @@ class TestProcesses(FixtureAPITestCase): workers_process.refresh_from_db() self.assertEqual(workers_process.state, State.Unscheduled) self.assertIsNone(workers_process.farm) + self.assertEqual(get_default_farm_mock.call_count, 1) + self.assertEqual(is_available_mock.call_count, 1) + self.assertEqual(is_available_mock.call_args, call(self.user)) - @expectedFailure - def test_start_process_farm_guest(self): + @patch("arkindex.ponos.models.Farm.is_available", return_value=False) + def test_start_process_farm_guest(self, is_available_mock): workers_process = self.corpus.processes.create(creator=self.user, mode=ProcessMode.Workers) workers_process.worker_runs.create(version=self.recognizer, parents=[], configuration=None) self.client.force_login(self.user) - self.other_farm.memberships.filter(user=self.user).delete() - with self.assertNumQueries(10): + with self.assertNumQueries(7): response = self.client.post( reverse("api:process-start", kwargs={"pk": str(workers_process.id)}), - {"farm": str(self.other_farm.id)} + {"farm": str(Farm.objects.first().id)} ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @@ -2482,6 +2515,8 @@ class TestProcesses(FixtureAPITestCase): workers_process.refresh_from_db() self.assertEqual(workers_process.state, State.Unscheduled) self.assertIsNone(workers_process.farm) + self.assertEqual(is_available_mock.call_count, 1) + self.assertEqual(is_available_mock.call_args, call(self.user)) def test_start_process_wrong_farm_id(self): """ @@ -2857,8 +2892,8 @@ class TestProcesses(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.json(), {"__all__": ["A process can only be cleared before getting started."]}) - @expectedFailure - def test_clear_process_requires_permissions(self): + @patch("arkindex.project.mixins.get_max_level", return_value=Role.Contributor.value) + def test_clear_process_requires_permissions(self, get_max_level_mock): process = self.corpus.processes.create( creator=self.user, mode=ProcessMode.Workers, @@ -2872,12 +2907,14 @@ class TestProcesses(FixtureAPITestCase): user2.verified_email = True user2.save() - self.corpus.memberships.create(user=user2, level=Role.Contributor.value) self.client.force_login(user2) - with self.assertNumQueries(8): + with self.assertNumQueries(4): response = self.client.delete(reverse("api:clear-process", kwargs={"pk": str(process.id)})) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual(response.json(), {"detail": "You do not have a sufficient access level to this process."}) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(user2, self.corpus)) def test_select_failed_elts_forbidden_methods(self): self.client.force_login(self.user) @@ -2905,16 +2942,17 @@ class TestProcesses(FixtureAPITestCase): response = self.client.post(reverse("api:process-select-failures", kwargs={"pk": str(uuid.uuid4())})) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - @expectedFailure - def test_select_failed_elts_requires_corpus_read_access(self): + @patch("arkindex.project.mixins.get_max_level", return_value=None) + def test_select_failed_elts_requires_corpus_read_access(self, get_max_level_mock): self.client.force_login(self.user) - self.elts_process.corpus.memberships.filter(user=self.user).delete() - self.elts_process.corpus.public = False - self.elts_process.corpus.save() - with self.assertNumQueries(5): + + with self.assertNumQueries(3): response = self.client.post(reverse("api:process-select-failures", kwargs={"pk": str(self.elts_process.id)})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual(response.json(), {"detail": "You do not have a read access to the corpus of this process."}) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(self.user, self.corpus)) def test_select_failed_elts_requires_workers_mode(self): self.client.force_login(self.user) diff --git a/arkindex/process/tests/test_templates.py b/arkindex/process/tests/test_templates.py index 770b6d2f9610bd3b25b6199ca865f79528fa4bf5..cc32112c7f3ea82e57a9080a9001c6c6b3c63366 100644 --- a/arkindex/process/tests/test_templates.py +++ b/arkindex/process/tests/test_templates.py @@ -1,6 +1,6 @@ import json from datetime import datetime, timezone -from unittest import expectedFailure +from unittest.mock import call, patch from rest_framework import status from rest_framework.reverse import reverse @@ -82,7 +82,7 @@ class TestTemplates(FixtureAPITestCase): def test_create(self): self.client.force_login(self.user) - with self.assertNumQueries(9): + with self.assertNumQueries(8): response = self.client.post( reverse( "api:create-process-template", kwargs={"pk": str(self.process_template.id)} @@ -132,38 +132,47 @@ class TestTemplates(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - @expectedFailure - def test_create_requires_contributor_access_rights_process(self): - new_user = User.objects.create(email="new@test.fr", verified_email=True) - self.worker_1.memberships.create(user=new_user, level=Role.Contributor.value) - self.private_corpus.memberships.create(user=new_user, level=Role.Guest.value) - self.client.force_login(new_user) - response = self.client.post( - reverse( - "api:create-process-template", - kwargs={"pk": str(self.private_process_template.id)}, + @patch("arkindex.project.mixins.get_max_level", return_value=Role.Guest.value) + def test_create_requires_contributor_access_rights_process(self, get_max_level_mock): + self.client.force_login(self.user) + + with self.assertNumQueries(4): + response = self.client.post( + reverse( + "api:create-process-template", + kwargs={"pk": str(self.private_process_template.id)}, + ) ) - ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual( response.json(), {"detail": "You do not have a contributor access to this process."}, ) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus)) + + @patch("arkindex.project.mixins.has_access", return_value=False) + @patch("arkindex.project.mixins.get_max_level", return_value=Role.Contributor.value) + def test_create_requires_access_rights_all_workers(self, get_max_level_mock, has_access_mock): + self.client.force_login(self.user) + + with self.assertNumQueries(4): + response = self.client.post( + reverse("api:create-process-template", kwargs={"pk": str(self.private_process_template.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - @expectedFailure - def test_create_requires_access_rights_all_workers(self): - new_user = User.objects.create(email="new@test.fr", verified_email=True) - self.private_corpus.memberships.create(user=new_user, level=Role.Contributor.value) - self.worker_1.memberships.create(user=new_user, level=Role.Guest.value) - self.client.force_login(new_user) - response = self.client.post( - reverse("api:create-process-template", kwargs={"pk": str(self.private_process_template.id)}) - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual( response.json(), ["You do not have an execution access to every worker of this process."], ) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus)) + self.assertEqual(has_access_mock.call_args_list, [ + call(self.user, self.worker_1, Role.Contributor.value, skip_public=False), + call(self.user, self.worker_1.repository, Role.Contributor.value, skip_public=False), + ]) def test_create_unsupported_mode(self): self.client.force_login(self.user) @@ -172,7 +181,7 @@ class TestTemplates(FixtureAPITestCase): self.process.mode = mode self.process.save() - with self.assertNumQueries(5): + with self.assertNumQueries(4): response = self.client.post( reverse("api:create-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(self.process.id)}), @@ -188,7 +197,7 @@ class TestTemplates(FixtureAPITestCase): self.client.force_login(self.user) local_process = self.user.processes.get(mode=ProcessMode.Local) - with self.assertNumQueries(5): + with self.assertNumQueries(4): response = self.client.post( reverse("api:create-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(local_process.id)}), @@ -200,74 +209,72 @@ class TestTemplates(FixtureAPITestCase): local_process.refresh_from_db() self.assertEqual(local_process.template, None) - @expectedFailure - def test_apply_requires_contributor_rights_on_template(self): + @patch("arkindex.project.mixins.get_max_level", return_value=Role.Guest.value) + def test_apply_requires_contributor_rights_on_template(self, get_max_level_mock): """Raise 403 if the user does not have rights on template """ - new_user = User.objects.create(email="new@test.fr", verified_email=True) - # rights on worker of template - self.worker_1.memberships.create(user=new_user, level=Role.Contributor.value) - # no rights on template - self.private_corpus.memberships.create(user=new_user, level=Role.Guest.value) - # rights on target process - self.corpus.memberships.create(user=new_user, level=Role.Contributor.value) - self.client.force_login(new_user) - response = self.client.post( - reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}), - data=json.dumps({"process_id": str(self.process.id)}), - content_type="application/json", - ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.client.force_login(self.user) + + with self.assertNumQueries(4): + response = self.client.post( + reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}), + data=json.dumps({"process_id": str(self.process.id)}), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual( response.json(), {"detail": "You do not have a contributor access to this process."}, ) + self.assertEqual(get_max_level_mock.call_count, 1) + self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus)) - @expectedFailure - def test_apply_requires_contributor_rights_on_process(self): + @patch("arkindex.project.mixins.get_max_level", side_effect=[Role.Contributor.value, Role.Guest.value]) + def test_apply_requires_contributor_rights_on_process(self, get_max_level_mock): """Raise 403 if the user does not have rights on the target process """ - new_user = User.objects.create(email="new@test.fr", verified_email=True) - # rights on worker of template - self.worker_1.memberships.create(user=new_user, level=Role.Contributor.value) - # rights on template - self.private_corpus.memberships.create(user=new_user, level=Role.Contributor.value) - # no rights on target process - self.corpus.memberships.create(user=new_user, level=Role.Guest.value) - self.client.force_login(new_user) - response = self.client.post( - reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}), - data=json.dumps({"process_id": str(self.process.id)}), - content_type="application/json", - ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.client.force_login(self.user) + + with self.assertNumQueries(5): + response = self.client.post( + reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}), + data=json.dumps({"process_id": str(self.process.id)}), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual( response.json(), {"detail": "You do not have a contributor access to this process."}, ) + self.assertEqual(get_max_level_mock.call_args_list, [ + call(self.user, self.private_corpus), + call(self.user, self.corpus), + ]) - @expectedFailure - def test_apply_requires_access_rights_all_workers(self): + @patch("arkindex.project.mixins.has_access", return_value=False) + def test_apply_requires_access_rights_all_workers(self, has_access_mock): """Raise 403 if the user does not have rights on all workers concerned """ - new_user = User.objects.create(email="new@test.fr", verified_email=True) - # no rights on worker of template - self.worker_1.memberships.create(user=new_user, level=Role.Guest.value) - # rights on template - self.private_corpus.memberships.create(user=new_user, level=Role.Contributor.value) - # rights on target process - self.corpus.memberships.create(user=new_user, level=Role.Contributor.value) - self.client.force_login(new_user) - response = self.client.post( - reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}), - data=json.dumps({"process_id": str(self.process.id)}), - content_type="application/json", - ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.client.force_login(self.user) + + with self.assertNumQueries(4): + response = self.client.post( + reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}), + data=json.dumps({"process_id": str(self.process.id)}), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual( response.json(), {"detail": "You do not have an execution access to this worker."}, ) + self.assertEqual(has_access_mock.call_args_list, [ + call(self.user, self.worker_1, Role.Contributor.value, skip_public=False), + call(self.user, self.worker_1.repository, Role.Contributor.value, skip_public=False), + ]) def test_apply_already_applied(self): """Raise 400 if the process already has a template attached @@ -301,7 +308,7 @@ class TestTemplates(FixtureAPITestCase): def test_apply(self): self.assertIsNotNone(self.version_2.docker_image_iid) self.client.force_login(self.user) - with self.assertNumQueries(11): + with self.assertNumQueries(9): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(self.process.id)}), @@ -334,7 +341,7 @@ class TestTemplates(FixtureAPITestCase): self.version_2.save() self.client.force_login(self.user) - with self.assertNumQueries(11): + with self.assertNumQueries(9): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(self.process.id)}), @@ -354,7 +361,7 @@ class TestTemplates(FixtureAPITestCase): parents=[], ) # Apply a template that has two other worker runs - with self.assertNumQueries(13): + with self.assertNumQueries(11): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(process.id)}), @@ -382,7 +389,7 @@ class TestTemplates(FixtureAPITestCase): self.version_2.state = WorkerVersionState.Error self.version_2.save() self.client.force_login(self.user) - with self.assertNumQueries(7): + with self.assertNumQueries(5): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(self.process.id)}), @@ -397,7 +404,7 @@ class TestTemplates(FixtureAPITestCase): self.worker_2.archived = datetime.now(timezone.utc) self.worker_2.save() self.client.force_login(self.user) - with self.assertNumQueries(7): + with self.assertNumQueries(5): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(self.process.id)}), @@ -413,7 +420,7 @@ class TestTemplates(FixtureAPITestCase): self.model_version.save() self.client.force_login(self.user) - with self.assertNumQueries(7): + with self.assertNumQueries(5): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(self.process.id)}), @@ -431,7 +438,7 @@ class TestTemplates(FixtureAPITestCase): self.model.save() self.client.force_login(self.user) - with self.assertNumQueries(7): + with self.assertNumQueries(5): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(self.process.id)}), @@ -450,7 +457,7 @@ class TestTemplates(FixtureAPITestCase): self.process.mode = mode self.process.save() - with self.assertNumQueries(7): + with self.assertNumQueries(5): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(self.process.id)}), @@ -468,7 +475,7 @@ class TestTemplates(FixtureAPITestCase): self.client.force_login(self.user) local_process = self.user.processes.get(mode=ProcessMode.Local) - with self.assertNumQueries(6): + with self.assertNumQueries(5): response = self.client.post( reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}), data=json.dumps({"process_id": str(local_process.id)}), diff --git a/arkindex/process/tests/test_workerruns.py b/arkindex/process/tests/test_workerruns.py index cbe72a1c97beb53b7fcd7413c7ba5d653b8b9597..58ce01dc838ef69ecce5e84797236cfa43ef31ce 100644 --- a/arkindex/process/tests/test_workerruns.py +++ b/arkindex/process/tests/test_workerruns.py @@ -1,6 +1,5 @@ import uuid from datetime import datetime, timezone -from unittest import expectedFailure from unittest.mock import call, patch from django.test import override_settings @@ -97,6 +96,9 @@ class TestWorkerRuns(FixtureAPITestCase): ram_total=99e9, last_ping=datetime.now(timezone.utc), ) + # Add custom attributes to make the agent usable as an authenticated user + cls.agent.is_agent = True + cls.agent.is_anonymous = False def test_list_requires_login(self): with self.assertNumQueries(0): @@ -964,8 +966,9 @@ class TestWorkerRuns(FixtureAPITestCase): """ self.process_1.tasks.create(run=0, depth=0, slug="something", agent=self.agent) - self.client.force_login(self.user) - with self.assertNumQueries(7): + # Agent auth is not implemented in CE + self.client.force_authenticate(user=self.agent) + with self.assertNumQueries(5): response = self.client.get( reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}), ) @@ -1015,7 +1018,6 @@ class TestWorkerRuns(FixtureAPITestCase): "summary": f"Worker Recognizer @ {str(self.version_1.id)[:6]}", }) - @expectedFailure def test_retrieve_agent_unassigned(self): """ A Ponos agent cannot retrieve a WorkerRun on a process where it does not have any assigned tasks @@ -1023,8 +1025,8 @@ class TestWorkerRuns(FixtureAPITestCase): self.process_1.tasks.create(run=0, depth=0, slug="something", agent=None) # Agent auth is not implemented in CE - self.client.force_login(self.user) - with self.assertNumQueries(3): + self.client.force_authenticate(user=self.agent) + with self.assertNumQueries(1): response = self.client.get( reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}), ) @@ -2007,7 +2009,6 @@ class TestWorkerRuns(FixtureAPITestCase): "__all__": ["A WorkerRun already exists on this process with the selected worker version, model version and configuration."], }) - @expectedFailure def test_update_agent(self): """ Ponos agents cannot update WorkerRuns, even when they can access them @@ -2015,8 +2016,8 @@ class TestWorkerRuns(FixtureAPITestCase): self.process_1.tasks.create(run=0, depth=0, slug="something", agent=self.agent) # Agent auth is not implemented in CE - self.client.force_login(self.user) - with self.assertNumQueries(4): + self.client.force_authenticate(user=self.agent) + with self.assertNumQueries(2): response = self.client.put( reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}), ) @@ -2932,7 +2933,6 @@ class TestWorkerRuns(FixtureAPITestCase): "__all__": ["A WorkerRun already exists on this process with the selected worker version, model version and configuration."], }) - @expectedFailure def test_partial_update_agent(self): """ Ponos agents cannot update WorkerRuns, even when they can access them @@ -2940,8 +2940,8 @@ class TestWorkerRuns(FixtureAPITestCase): self.process_1.tasks.create(run=0, depth=0, slug="something", agent=self.agent) # Agent auth is not implemented in CE - self.client.force_login(self.user) - with self.assertNumQueries(4): + self.client.force_authenticate(user=self.agent) + with self.assertNumQueries(2): response = self.client.patch( reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}), ) @@ -3071,7 +3071,6 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertEqual(response.json(), ["WorkerRuns cannot be deleted from a process that has already started."]) - @expectedFailure def test_delete_agent(self): """ Ponos agents cannot delete WorkerRuns, even when they can access them @@ -3079,8 +3078,8 @@ class TestWorkerRuns(FixtureAPITestCase): self.process_1.tasks.create(run=0, depth=0, slug="something", agent=self.agent) # Agent auth is not implemented in CE - self.client.force_login(self.user) - with self.assertNumQueries(3): + self.client.force_authenticate(user=self.agent) + with self.assertNumQueries(1): response = self.client.delete( reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}), ) diff --git a/arkindex/project/mixins.py b/arkindex/project/mixins.py index 99ecfc1c3e11b5af4cbb358d41d4df2e91575239..6f04942aa21809a57f434e94eab942e7bf9801a8 100644 --- a/arkindex/project/mixins.py +++ b/arkindex/project/mixins.py @@ -8,8 +8,7 @@ from rest_framework.exceptions import APIException, PermissionDenied from rest_framework.serializers import CharField, Serializer from arkindex.documents.models import Corpus -from arkindex.process.models import Process, Repository, Worker -from arkindex.training.models import Model +from arkindex.process.models import Process, Repository from arkindex.users.models import Role from arkindex.users.utils import filter_rights, get_max_level, has_access @@ -62,25 +61,6 @@ class WorkerACLMixin(ACLMixin): A public worker is considered as executable (i.e. any user has a contributor access) """ - @property - def executable_workers(self): - return Worker.objects.filter( - Q(public=True) - | Q(id__in=filter_rights(self.user, Worker, Role.Contributor.value).values("id")) - | Q(repository_id__in=filter_rights(self.user, Repository, Role.Contributor.value).values("id")) - ).distinct() - - def get_max_level(self, worker): - # Access right on a worker can be defined by a right on its repository - worker_level = get_max_level(self.user, worker) - if not worker.repository: - return worker_level - repo_level = get_max_level(self.user, worker.repository) - return max( - filter(None, (worker_level, repo_level)), - default=None - ) - def has_worker_access(self, worker, level): if worker.public and level <= Role.Contributor.value: return True @@ -113,18 +93,6 @@ class CorpusACLMixin(ACLMixin): raise PermissionDenied(detail=f"You do not have {str(role).lower()} access to this corpus.") return corpus - @property - def readable_corpora(self): - return Corpus.objects.filter( - id__in=filter_rights(self.user, Corpus, Role.Guest.value).values("id") - ) - - @property - def writable_corpora(self): - return Corpus.objects.filter( - id__in=filter_rights(self.user, Corpus, Role.Contributor.value).values("id") - ) - def has_read_access(self, corpus): return self.has_access(corpus, Role.Guest.value) @@ -140,18 +108,6 @@ class TrainingModelMixin(ACLMixin): Access control mixin for machine learning models """ - @property - def readable_models(self): - return Model.objects.filter( - id__in=filter_rights(self.user, Model, Role.Guest.value).values("id") - ) - - @property - def editable_models(self): - return Model.objects.filter( - id__in=filter_rights(self.user, Model, Role.Contributor.value).values("id") - ) - def has_read_access(self, model): return self.has_access(model, Role.Guest.value) diff --git a/arkindex/project/tests/test_acl_mixin.py b/arkindex/project/tests/test_acl_mixin.py index 64248aaaa46c680eb5c00b4f1ab800784be28b9d..8d84f0d1d5927a76e42a1c165e4c9727262a43fa 100644 --- a/arkindex/project/tests/test_acl_mixin.py +++ b/arkindex/project/tests/test_acl_mixin.py @@ -166,58 +166,6 @@ class TestACLMixin(FixtureTestCase): self.assertEqual(admin_access, access_check) ContentType.objects.clear_cache() - @expectedFailure - def test_corpus_acl_mixin_writable(self): - corpus_acl_mixin = CorpusACLMixin(user=self.user1) - with self.assertNumQueries(1): - corpora = list(corpus_acl_mixin.writable_corpora) - self.assertCountEqual( - list(corpora), - [self.corpus1] - ) - - def test_corpus_readable_orderable(self): - # Assert corpora retrieved via the mixin are still orderable - corpus_acl_mixin = CorpusACLMixin(user=self.user3) - with self.assertNumQueries(1): - corpora = list(corpus_acl_mixin.readable_corpora.order_by("name")) - self.assertListEqual( - [c.name for c in corpora], - ["Corpus1", "Corpus2", "Unit Tests"] - ) - - def test_super_admin_readable_corpora(self): - # A super admin should retrieve all existing corpora with Admin rights - corpus_acl_mixin = CorpusACLMixin(user=self.superuser) - with self.assertNumQueries(1): - corpora = list(corpus_acl_mixin.readable_corpora) - self.assertCountEqual( - list(corpora), - list(Corpus.objects.all()) - ) - - @expectedFailure - def test_anonymous_user_readable_corpora(self): - # An anonymous user should have guest access to any public corpora - corpus_acl_mixin = CorpusACLMixin(user=AnonymousUser()) - with self.assertNumQueries(1): - corpora = list(corpus_acl_mixin.readable_corpora) - self.assertCountEqual( - list(corpora), - list(Corpus.objects.filter(public=True)) - ) - - def test_corpus_right_and_public(self): - # User specific rights should be returned instead of the the defaults access for public rights - Right.objects.create(user=self.user3, content_object=self.corpus, level=42) - corpus_acl_mixin = CorpusACLMixin(user=self.user3) - with self.assertNumQueries(1): - corpora = list(corpus_acl_mixin.readable_corpora) - self.assertCountEqual( - list(corpora), - [self.corpus1, self.corpus2, self.corpus] - ) - @expectedFailure def test_max_level_does_not_exists(self): with self.assertNumQueries(3): @@ -306,22 +254,3 @@ class TestACLMixin(FixtureTestCase): with self.assertNumQueries(0): admin_access = TrainingModelMixin(user=self.user5).has_admin_access(self.model2) self.assertTrue(admin_access) - - @expectedFailure - def test_models_readable(self): - """ - To view a model, a user needs guest access. - """ - with self.assertNumQueries(1): - readable_models = list(TrainingModelMixin(user=self.user4).readable_models) - self.assertListEqual(readable_models, [self.model1]) - - @expectedFailure - def test_models_editable(self): - """ - To edit a model, a user needs contributor access. - User5 only has that access on model2. - """ - with self.assertNumQueries(1): - editable_models = list(TrainingModelMixin(user=self.user5).editable_models) - self.assertListEqual(editable_models, [self.model2])