From 8e8d7c89064dadf872d769671d66842a2fdf63aa Mon Sep 17 00:00:00 2001 From: Erwan Rouchet <rouchet@teklia.com> Date: Thu, 26 Jan 2023 17:01:00 +0100 Subject: [PATCH] Fix tests, merge URLs and views --- arkindex/ponos/api.py | 78 ++++++++++++++++----- arkindex/ponos/authentication.py | 12 ++-- arkindex/ponos/permissions.py | 75 ++++++++++++++++----- arkindex/ponos/tests/test_api.py | 31 +++++---- arkindex/ponos/urls.py | 52 -------------- arkindex/process/permissions.py | 37 ---------- arkindex/process/tests/test_signals.py | 2 +- arkindex/project/ponos_v1.py | 51 ++++++++++++++ arkindex/project/settings.py | 3 + arkindex/project/tests/test_ponos_view.py | 32 ++++----- arkindex/project/urls.py | 27 +------- arkindex/project/views.py | 82 +---------------------- 12 files changed, 218 insertions(+), 264 deletions(-) delete mode 100644 arkindex/ponos/urls.py delete mode 100644 arkindex/process/permissions.py create mode 100644 arkindex/project/ponos_v1.py diff --git a/arkindex/ponos/api.py b/arkindex/ponos/api.py index a269c4bf91..2bbe8df38c 100644 --- a/arkindex/ponos/api.py +++ b/arkindex/ponos/api.py @@ -3,12 +3,11 @@ import uuid from collections import defaultdict from textwrap import dedent -from django.core.exceptions import PermissionDenied from django.db.models import Count, Max, Q from django.shortcuts import get_object_or_404, redirect from django.utils import timezone from drf_spectacular.utils import OpenApiExample, extend_schema, extend_schema_view -from rest_framework.exceptions import ValidationError +from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError from rest_framework.generics import ( CreateAPIView, ListAPIView, @@ -17,14 +16,20 @@ from rest_framework.generics import ( RetrieveUpdateAPIView, UpdateAPIView, ) -from rest_framework.permissions import IsAuthenticated +from rest_framework.permissions import SAFE_METHODS from rest_framework.response import Response from rest_framework.views import APIView from arkindex.ponos.authentication import AgentAuthentication from arkindex.ponos.keys import load_private_key from arkindex.ponos.models import Agent, Artifact, Farm, Secret, State, Task, Workflow -from arkindex.ponos.permissions import IsAgent, IsAssignedAgentOrReadOnly +from arkindex.ponos.permissions import ( + IsAgent, + IsAgentOrArtifactAdmin, + IsAgentOrInternal, + IsAgentOrTaskAdmin, + IsAssignedAgentOrReadOnly, +) from arkindex.ponos.recipe import parse_recipe from arkindex.ponos.renderers import PublicKeyPEMRenderer from arkindex.ponos.serializers import ( @@ -42,6 +47,10 @@ from arkindex.ponos.serializers import ( TaskTinySerializer, WorkflowSerializer, ) +from arkindex.process.models import Process +from arkindex.project.mixins import ProcessACLMixin +from arkindex.project.permissions import IsAuthenticated, IsVerified +from arkindex.users.models import Role from rest_framework_simplejwt.views import TokenRefreshView @@ -81,16 +90,29 @@ class PublicKeyEndpoint(APIView): @extend_schema(tags=["ponos"]) @extend_schema_view( - get=extend_schema(description="Retrieve a Ponos workflow status"), - put=extend_schema(description="Update a workflow's status and tasks"), - patch=extend_schema(description="Partially update a workflow"), + get=extend_schema(description=dedent(""" + Retrieve a Ponos workflow status. + + Requires **guest** access to its process. + """).strip()), + put=extend_schema(description=dedent(""" + Update a workflow's status and tasks. + + Requires **admin** access to its process. + """).strip()), + patch=extend_schema(description=dedent(""" + Partially update a workflow's status and tasks. + + Requires **admin** access to its process. + """).strip()), ) -class WorkflowDetails(RetrieveUpdateAPIView): +class WorkflowDetails(ProcessACLMixin, RetrieveUpdateAPIView): """ Retrieve information about a workflow, or update its state. Updating a workflow's state to :attr:`~arkindex.ponos.models.State.Stopping` will cause it to stop. """ + permission_classes = (IsVerified, ) queryset = Workflow.objects.prefetch_related("tasks__parents").annotate( last_run=Max("tasks__run") ) @@ -99,6 +121,26 @@ class WorkflowDetails(RetrieveUpdateAPIView): def perform_update(self, serializer): serializer.instance.stop() + def check_object_permissions(self, request, workflow): + super().check_object_permissions(request, workflow) + if self.request.user.is_admin: + return + + required_access = Role.Admin.value + # Allow a user with a read access on the process to retrieve its workflow + if request.method in SAFE_METHODS: + required_access = Role.Guest.value + + process = get_object_or_404( + Process, + workflow_id=workflow.id + ) + access_level = self.process_access_level(process) + if not access_level: + raise NotFound + if access_level < required_access: + raise PermissionDenied(detail='You do not have an admin access to this process.') + @extend_schema(tags=["ponos"]) @extend_schema_view( @@ -167,8 +209,10 @@ class AgentTokenRefresh(TokenRefreshView): class AgentDetails(RetrieveAPIView): """ Retrieve details of an agent including its running tasks - """ + Requires authentication with a verified e-mail address. + """ + permission_classes = (IsVerified, ) serializer_class = AgentDetailsSerializer queryset = Agent.objects.all() @@ -180,9 +224,10 @@ class AgentDetails(RetrieveAPIView): class AgentsState(ListAPIView): """ List all agents on the system with their health state. - No authentication nor permission is required to read agents state. - """ + Requires authentication with a verified e-mail address. + """ + permission_classes = (IsVerified, ) serializer_class = AgentStateSerializer queryset = ( @@ -269,7 +314,7 @@ class TaskArtifacts(ListCreateAPIView): # Used for OpenAPI schema serialization: the ID in the path is the task ID queryset = Task.objects.none() - permission_classes = (IsAuthenticated,) + permission_classes = (IsAuthenticated, ) serializer_class = ArtifactSerializer # Force no pagination, even when global settings add them @@ -295,10 +340,10 @@ class TaskArtifacts(ListCreateAPIView): class TaskArtifactDownload(APIView): """ - Redirect to the S3 url of an artifact in order to download it + Redirects to the S3 URL of an artifact in order to download it. """ - permission_classes = (IsAgent,) + permission_classes = (IsAgentOrArtifactAdmin, ) def get_object(self, pk, path): artifact = get_object_or_404(Artifact, task_id=pk, path=path) @@ -352,9 +397,8 @@ class TaskCreate(CreateAPIView): class TaskUpdate(UpdateAPIView): """ Admins and task creators can use this endpoint to update a task - Permissions must be implemented by the top-level Django application """ - + permission_classes = (IsAgentOrTaskAdmin, ) queryset = Task.objects.all() serializer_class = TaskTinySerializer @@ -370,7 +414,7 @@ class SecretDetails(RetrieveAPIView): Retrieve a Ponos secret content as cleartext """ - permission_classes = (IsAgent,) + permission_classes = (IsAgentOrInternal, ) serializer_class = ClearTextSecretSerializer def get_object(self): diff --git a/arkindex/ponos/authentication.py b/arkindex/ponos/authentication.py index 8355955b80..8eaa6bf008 100644 --- a/arkindex/ponos/authentication.py +++ b/arkindex/ponos/authentication.py @@ -15,22 +15,18 @@ class AgentUser(Agent): is_staff: bool = False is_superuser: bool = False + is_authenticated: bool = True + is_anonymous: bool = False + is_admin: bool = False is_active: bool = True is_agent: bool = True is_internal: bool = False + verified_email: bool = False @property def username(self) -> str: return str(self.id) - @property - def is_anonymous(self) -> bool: - return False - - @property - def is_authenticated(self) -> bool: - return True - def get_username(self) -> str: return self.username diff --git a/arkindex/ponos/permissions.py b/arkindex/ponos/permissions.py index 77f1a814b5..58d8de04e7 100644 --- a/arkindex/ponos/permissions.py +++ b/arkindex/ponos/permissions.py @@ -1,43 +1,86 @@ -from rest_framework.permissions import SAFE_METHODS, IsAuthenticated +from rest_framework.permissions import SAFE_METHODS from arkindex.ponos.models import Task +from arkindex.project.mixins import CorpusACLMixin +from arkindex.project.permissions import IsAuthenticated, require_internal + + +def require_agent(request, view): + return getattr(request.user, 'is_admin', False) or getattr(request.user, 'is_agent', False) + + +def require_agent_or_internal(request, view): + return require_internal(request, view) or getattr(request.user, 'is_agent', False) class IsAgent(IsAuthenticated): """ - Only allow Ponos agents and admins + Only allow Ponos agents and admins. """ - - def has_permission(self, request, view) -> bool: - if ( - request.user.is_staff - or hasattr(request.user, "is_agent") - and request.user.is_agent - ): - return super().has_permission(request, view) - return False + checks = IsAuthenticated.checks + (require_agent, ) class IsAgentOrReadOnly(IsAgent): """ - Restricts write access to Ponos agents and admins + Restricts write access to Ponos agents and admins, + leave read access to everyone without any authentication. """ - - def has_permission(self, request, view) -> bool: - return request.method in SAFE_METHODS or super().has_permission(request, view) + allow_safe_methods = True class IsAssignedAgentOrReadOnly(IsAgentOrReadOnly): """ Restricts write access to Ponos agents and admins, and restricts write access on tasks to admins and agents assigned to them. + Leave read access to everyone without any authentication. """ def has_object_permission(self, request, view, obj) -> bool: if ( isinstance(obj, Task) - and not request.user.is_staff + and not getattr(request.user, 'is_admin', False) and not request.user == obj.agent ): return request.method in SAFE_METHODS return super().has_object_permission(request, view, obj) + + +class IsAgentOrTaskAdmin(CorpusACLMixin, IsAuthenticated): + """ + Permission to access a task with high privilege + + Allowed for admins, agents, internal users, creators of the process + whose workflow contains the task, and users with an admin right on the + process' corpus. + """ + + 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(request, view) + or require_internal(request, view) + or ( + task.workflow.process is not None + and task.workflow.process.corpus_id is not None + and self.has_admin_access(task.workflow.process.corpus) + ) + ) + + +class IsAgentOrArtifactAdmin(IsAgentOrTaskAdmin): + """ + Permission to access an artifact with high privilege, based on + access to the artifact's task through IsAgentOrTaskAdmin. + """ + + def has_object_permission(self, request, view, artifact): + return super().has_object_permission(request, view, artifact.task) + + +class IsAgentOrInternal(IsAuthenticated): + """ + Allow access to agents or internal users, and not admins. + """ + checks = (require_agent_or_internal, ) diff --git a/arkindex/ponos/tests/test_api.py b/arkindex/ponos/tests/test_api.py index d141fe0e62..05be3a9eaf 100644 --- a/arkindex/ponos/tests/test_api.py +++ b/arkindex/ponos/tests/test_api.py @@ -80,6 +80,7 @@ class TestAPI(APITestCase): index=1, ram_total=8 * 1024 * 1024 * 1024, ) + cls.superuser = User.objects.create_superuser('root@root.root', 'root') def _build_workflow_response(self, response, **kwargs): """ @@ -133,9 +134,10 @@ class TestAPI(APITestCase): return data def test_workflow_details(self): - with self.assertNumQueries(4): + self.client.force_login(self.superuser) + with self.assertNumQueries(6): resp = self.client.get( - reverse("ponos:workflow-details", args=[self.workflow.id]) + reverse("ponos:workflow-details", args=[self.workflow.id]), ) self.assertEqual(resp.status_code, status.HTTP_200_OK) data = resp.json() @@ -147,6 +149,7 @@ class TestAPI(APITestCase): def test_stop_workflow(self): self.task1.state = State.Pending self.task1.save() + self.client.force_login(self.superuser) resp = self.client.patch( reverse("ponos:workflow-details", kwargs={"pk": self.workflow.id}), {"state": State.Stopping.value}, @@ -165,6 +168,7 @@ class TestAPI(APITestCase): self.task1.save() self.task2.state = State.Completed self.task2.save() + self.client.force_login(self.superuser) resp = self.client.patch( reverse("ponos:workflow-details", kwargs={"pk": self.workflow.id}), {"state": State.Stopping.value}, @@ -179,6 +183,7 @@ class TestAPI(APITestCase): """ new_id = uuid.uuid4() new_farm_id = uuid.uuid4() + self.client.force_login(self.superuser) resp = self.client.patch( reverse("ponos:workflow-details", kwargs={"pk": self.workflow.id}), { @@ -197,6 +202,7 @@ class TestAPI(APITestCase): self.assertNotEqual(self.workflow.farm_id, new_farm_id) def test_change_state_workflow(self): + self.client.force_login(self.superuser) resp = self.client.patch( reverse("ponos:workflow-details", kwargs={"pk": self.workflow.id}), {"state": State.Completed.value}, @@ -359,7 +365,6 @@ class TestAPI(APITestCase): # One GPU is assigned self.assertTrue(GPU.objects.filter(task__isnull=False).exists()) - self.maxDiff = None resp = self.client.get(reverse("ponos:task-details", args=[self.task1.id])) self.assertEqual(resp.status_code, status.HTTP_200_OK) data = resp.json() @@ -896,7 +901,7 @@ class TestAPI(APITestCase): # Not accessible when logged out resp = self.client.get(url) - self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) # Accessible but empty when logged in resp = self.client.get( @@ -950,6 +955,7 @@ class TestAPI(APITestCase): ], ) + @override_settings(PONOS_ARTIFACT_MAX_SIZE=99999) def test_artifact_creation(self): # No artifacts in DB at first @@ -1055,7 +1061,7 @@ class TestAPI(APITestCase): "ponos:task-artifact-download", args=[self.task1.id, "path/to/file.json"] ) resp = self.client.get(url) - self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) # Request to a missing artifact will lead to a 404 bad_url = reverse( @@ -1288,7 +1294,7 @@ class TestAPI(APITestCase): response = self.client.get( reverse("ponos:secret-details", kwargs={"name": "abc"}) ) - self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertDictEqual( response.json(), {"detail": "Authentication credentials were not provided."} ) @@ -1350,10 +1356,11 @@ class TestAPI(APITestCase): for state in State ] ) - with self.assertNumQueries(4): + self.client.force_login(self.superuser) + with self.assertNumQueries(6): response = self.client.get(reverse("ponos:agents-state")) + self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() self.assertEqual(data["count"], 1) agent_state = data["results"][0] @@ -1410,15 +1417,15 @@ class TestAPI(APITestCase): ] ) running_task = self.agent.tasks.get(state=State.Running) - with self.assertNumQueries(5): + self.client.force_login(self.superuser) + with self.assertNumQueries(7): response = self.client.get( reverse("ponos:agent-details", kwargs={"pk": str(self.agent.id)}) ) + self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() del data["last_ping"] - self.maxDiff = None self.assertDictEqual( data, { @@ -1473,11 +1480,11 @@ class TestAPI(APITestCase): barley_farm = Farm.objects.create(name="Barley") with self.assertNumQueries(2): response = self.client.get(reverse("ponos:farm-list")) - self.maxDiff = None self.assertDictEqual( response.json(), { "count": 2, + "number": 1, "previous": None, "next": None, "results": [ diff --git a/arkindex/ponos/urls.py b/arkindex/ponos/urls.py deleted file mode 100644 index 67853c54d5..0000000000 --- a/arkindex/ponos/urls.py +++ /dev/null @@ -1,52 +0,0 @@ -from django.urls import path - -from arkindex.ponos.api import ( - AgentActions, - AgentDetails, - AgentRegister, - AgentsState, - AgentTokenRefresh, - FarmList, - PublicKeyEndpoint, - SecretDetails, - TaskArtifactDownload, - TaskArtifacts, - TaskCreate, - TaskDefinition, - TaskDetailsFromAgent, - TaskUpdate, - WorkflowDetails, -) - -app_name = "ponos" -urlpatterns = [ - path("v1/workflow/<uuid:pk>/", WorkflowDetails.as_view(), name="workflow-details"), - path("v1/task/", TaskCreate.as_view(), name="task-create"), - path("v1/task/<uuid:pk>/", TaskUpdate.as_view(), name="task-update"), - path( - "v1/task/<uuid:pk>/from-agent/", - TaskDetailsFromAgent.as_view(), - name="task-details", - ), - path( - "v1/task/<uuid:pk>/definition/", - TaskDefinition.as_view(), - name="task-definition", - ), - path( - "v1/task/<uuid:pk>/artifacts/", TaskArtifacts.as_view(), name="task-artifacts" - ), - path( - "v1/task/<uuid:pk>/artifact/<path:path>", - TaskArtifactDownload.as_view(), - name="task-artifact-download", - ), - path("v1/agent/", AgentRegister.as_view(), name="agent-register"), - path("v1/agent/<uuid:pk>/", AgentDetails.as_view(), name="agent-details"), - path("v1/agent/refresh/", AgentTokenRefresh.as_view(), name="agent-token-refresh"), - path("v1/agent/actions/", AgentActions.as_view(), name="agent-actions"), - path("v1/agents/", AgentsState.as_view(), name="agents-state"), - path("v1/public-key/", PublicKeyEndpoint.as_view(), name="public-key"), - path("v1/secret/<path:name>", SecretDetails.as_view(), name="secret-details"), - path("v1/farms/", FarmList.as_view(), name="farm-list"), -] diff --git a/arkindex/process/permissions.py b/arkindex/process/permissions.py deleted file mode 100644 index b54f2d15b6..0000000000 --- a/arkindex/process/permissions.py +++ /dev/null @@ -1,37 +0,0 @@ -from rest_framework import permissions - -from arkindex.project.mixins import CorpusACLMixin - - -class IsTaskAdmin(CorpusACLMixin, permissions.BasePermission): - """ - Permission to access a task with high privilege - Allowed for superuser, process creators and users with an admin right on its associated corpus - Allowed for agents too - """ - - def has_object_permission(self, request, view, task): - # Add request to attributes for the ACL mixin to work with self.user - self.request = request - - if not self.user.is_authenticated: - return False - return ( - # Admin and internal fields are present for humans and workers - (hasattr(self.user, "is_admin") and self.user.is_admin) - or (hasattr(self.user, "is_internal") and self.user.is_internal) - - # The agent field is only present for agents - or (hasattr(self.user, "is_agent") and self.user.is_agent) - - or ( - task.workflow.process.corpus_id - and self.has_admin_access(task.workflow.process.corpus) - ) - ) - - -class IsArtifactAdmin(IsTaskAdmin): - - def has_object_permission(self, request, view, artifact): - return super().has_object_permission(request, view, artifact.task) diff --git a/arkindex/process/tests/test_signals.py b/arkindex/process/tests/test_signals.py index ae384d45b3..cfec840cf9 100644 --- a/arkindex/process/tests/test_signals.py +++ b/arkindex/process/tests/test_signals.py @@ -321,7 +321,7 @@ class TestSignals(FixtureAPITestCase): task.save() resp = self.client.patch( - reverse('ponos-task-update', kwargs={'pk': task.id}), + reverse('ponos:task-update', kwargs={'pk': task.id}), {'state': new_task_state.value} ) self.assertEqual(resp.status_code, status.HTTP_200_OK) diff --git a/arkindex/project/ponos_v1.py b/arkindex/project/ponos_v1.py new file mode 100644 index 0000000000..6076f776db --- /dev/null +++ b/arkindex/project/ponos_v1.py @@ -0,0 +1,51 @@ +from django.urls import path + +from arkindex.ponos.api import ( + AgentActions, + AgentDetails, + AgentRegister, + AgentsState, + AgentTokenRefresh, + FarmList, + PublicKeyEndpoint, + SecretDetails, + TaskArtifactDownload, + TaskArtifacts, + TaskCreate, + TaskDefinition, + TaskDetailsFromAgent, + TaskUpdate, + WorkflowDetails, +) + +ponos = [ + path("workflow/<uuid:pk>/", WorkflowDetails.as_view(), name="workflow-details"), + path("task/", TaskCreate.as_view(), name="task-create"), + path("task/<uuid:pk>/", TaskUpdate.as_view(), name="task-update"), + path( + "task/<uuid:pk>/from-agent/", + TaskDetailsFromAgent.as_view(), + name="task-details", + ), + path( + "task/<uuid:pk>/definition/", + TaskDefinition.as_view(), + name="task-definition", + ), + path( + "task/<uuid:pk>/artifacts/", TaskArtifacts.as_view(), name="task-artifacts" + ), + path( + "task/<uuid:pk>/artifact/<path:path>", + TaskArtifactDownload.as_view(), + name="task-artifact-download", + ), + path("agent/", AgentRegister.as_view(), name="agent-register"), + path("agent/<uuid:pk>/", AgentDetails.as_view(), name="agent-details"), + path("agent/refresh/", AgentTokenRefresh.as_view(), name="agent-token-refresh"), + path("agent/actions/", AgentActions.as_view(), name="agent-actions"), + path("agents/", AgentsState.as_view(), name="agents-state"), + path("public-key/", PublicKeyEndpoint.as_view(), name="public-key"), + path("secret/<path:name>", SecretDetails.as_view(), name="secret-details"), + path("farms/", FarmList.as_view(), name="farm-list"), +] diff --git a/arkindex/project/settings.py b/arkindex/project/settings.py index 5d5bee8989..ee98eae3ec 100644 --- a/arkindex/project/settings.py +++ b/arkindex/project/settings.py @@ -558,6 +558,9 @@ if TEST_ENV: AWS_ENDPOINT = 'http://s3' PONOS_PRIVATE_KEY = None PONOS_DEFAULT_FARM = None + PONOS_AWS_ENDPOINT = 'http://somewhere' + PONOS_S3_ARTIFACTS_BUCKET = 'ponos-artifacts' + PONOS_S3_LOGS_BUCKET = 'ponos-logs' LOCAL_IMAGESERVER_ID = 1 # Causes RQ tasks to run on the main thread diff --git a/arkindex/project/tests/test_ponos_view.py b/arkindex/project/tests/test_ponos_view.py index c67a8b0b8a..a5a90ba415 100644 --- a/arkindex/project/tests/test_ponos_view.py +++ b/arkindex/project/tests/test_ponos_view.py @@ -59,7 +59,7 @@ class TestPonosView(FixtureAPITestCase): ) internal_user = User.objects.create(email='internal', is_internal=True) self.client.force_login(internal_user) - response = self.client.get(reverse('secret-details', kwargs={'name': account_name})) + response = self.client.get(reverse('ponos:secret-details', kwargs={'name': account_name})) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json(), { 'id': str(secret.id), @@ -77,14 +77,14 @@ class TestPonosView(FixtureAPITestCase): for user in non_internal_users: with self.subTest(user=user): self.client.force_login(user) - response = self.client.get(reverse('secret-details', kwargs={"name": "secret/name"})) + response = self.client.get(reverse('ponos:secret-details', kwargs={"name": "secret/name"})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_list_agents_requires_login(self): """ Only authenticated users should have the ability to list agents """ - response = self.client.get(reverse('ponos-agents')) + response = self.client.get(reverse('ponos:agents-state')) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_list_agents_requires_verified(self): @@ -94,19 +94,19 @@ class TestPonosView(FixtureAPITestCase): self.user.verified_email = False self.user.save() self.client.force_login(self.user) - response = self.client.get(reverse('ponos-agents')) + response = self.client.get(reverse('ponos:agents-state')) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_list_agents(self): self.client.force_login(self.user) - response = self.client.get(reverse('ponos-agents')) + response = self.client.get(reverse('ponos:agents-state')) self.assertEqual(response.status_code, status.HTTP_200_OK) def test_retrieve_agent_requires_login(self): """ Only authenticated users should have the ability to retrieve details of an agent """ - response = self.client.get(reverse('ponos-agent-details', kwargs={'pk': str(self.agent.id)})) + response = self.client.get(reverse('ponos:agent-details', kwargs={'pk': str(self.agent.id)})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_retrieve_agent_requires_verified(self): @@ -116,12 +116,12 @@ class TestPonosView(FixtureAPITestCase): self.user.verified_email = False self.user.save() self.client.force_login(self.user) - response = self.client.get(reverse('ponos-agent-details', kwargs={'pk': str(self.agent.id)})) + response = self.client.get(reverse('ponos:agent-details', kwargs={'pk': str(self.agent.id)})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_retrieve_agent(self): self.client.force_login(self.user) - response = self.client.get(reverse('ponos-agent-details', kwargs={'pk': str(self.agent.id)})) + response = self.client.get(reverse('ponos:agent-details', kwargs={'pk': str(self.agent.id)})) self.assertEqual(response.status_code, status.HTTP_200_OK) def test_update_task(self): @@ -129,7 +129,7 @@ class TestPonosView(FixtureAPITestCase): Only users with an admin privilege have the ability to update a process task """ test_cases = ( - (None, status.HTTP_403_FORBIDDEN, 1), + (None, status.HTTP_403_FORBIDDEN, 0), (self.creator, status.HTTP_403_FORBIDDEN, 9), (self.user, status.HTTP_403_FORBIDDEN, 9), (self.superuser, status.HTTP_200_OK, 10), @@ -141,7 +141,7 @@ class TestPonosView(FixtureAPITestCase): self.client.force_login(user) with self.assertNumQueries(requests_count): response = self.client.patch( - reverse('ponos-task-update', kwargs={'pk': str(self.task.id)}), + reverse('ponos:task-update', kwargs={'pk': str(self.task.id)}), json={'state': 'stopping'} ) self.assertEqual(response.status_code, status_code) @@ -152,7 +152,7 @@ class TestPonosView(FixtureAPITestCase): download an artifact of a process task """ test_cases = ( - (None, status.HTTP_403_FORBIDDEN, 2), + (None, status.HTTP_403_FORBIDDEN, 0), (self.creator, status.HTTP_403_FORBIDDEN, 10), (self.user, status.HTTP_403_FORBIDDEN, 10), (self.superuser, status.HTTP_302_FOUND, 4), @@ -164,7 +164,7 @@ class TestPonosView(FixtureAPITestCase): self.client.force_login(user) with self.assertNumQueries(requests_count): response = self.client.get( - reverse('ponos-artifact-dl', kwargs={'pk': str(self.task.id), 'path': self.artifact.path}), + reverse('ponos:task-artifact-download', kwargs={'pk': str(self.task.id), 'path': self.artifact.path}), follow=False ) self.assertEqual(response.status_code, status_code) @@ -184,7 +184,7 @@ class TestPonosView(FixtureAPITestCase): with self.assertNumQueries(3): response = self.client.get( - reverse('ponos-artifact-dl', kwargs={'pk': str(self.task.id), 'path': self.artifact.path}), + reverse('ponos:task-artifact-download', kwargs={'pk': str(self.task.id), 'path': self.artifact.path}), follow=False, HTTP_AUTHORIZATION="Bearer {}".format(agent_user.token.access_token), ) @@ -197,7 +197,7 @@ class TestPonosView(FixtureAPITestCase): self.client.force_login(self.user) self.process_corpus.memberships.create(user=self.user, level=Role.Guest.value) with self.assertNumQueries(10): - response = self.client.get(reverse('ponos-workflow-details', kwargs={'pk': str(self.workflow.id)})) + response = self.client.get(reverse('ponos:workflow-details', kwargs={'pk': str(self.workflow.id)})) self.assertEqual(response.status_code, status.HTTP_200_OK) def test_retry_workflow_requires_admin_access(self): @@ -210,7 +210,7 @@ class TestPonosView(FixtureAPITestCase): self.process_corpus.memberships.create(user=self.user, level=Role.Guest.value) with self.assertNumQueries(9): response = self.client.patch( - reverse('ponos-workflow-details', kwargs={'pk': str(self.workflow.id)}), + reverse('ponos:workflow-details', kwargs={'pk': str(self.workflow.id)}), {'status': 'stopped'} ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) @@ -223,7 +223,7 @@ class TestPonosView(FixtureAPITestCase): self.process_corpus.memberships.create(user=self.user, level=Role.Admin.value) with self.assertNumQueries(15): response = self.client.patch( - reverse('ponos-workflow-details', kwargs={'pk': str(self.workflow.id)}), + reverse('ponos:workflow-details', kwargs={'pk': str(self.workflow.id)}), {'status': 'stopped'} ) self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/arkindex/project/urls.py b/arkindex/project/urls.py index 180f41b929..05c35875ef 100644 --- a/arkindex/project/urls.py +++ b/arkindex/project/urls.py @@ -4,18 +4,8 @@ from django.contrib.staticfiles.urls import staticfiles_urlpatterns from django.urls import include, path, re_path from arkindex.project.api_v1 import api -from arkindex.project.views import ( - CdnHome, - FrontendView, - OpenAPIDocsView, - PonosAgentDetails, - PonosAgentsState, - PonosArtifactDownload, - PonosSecretDetails, - PonosTaskUpdate, - PonosWorkflowDetails, - RobotsTxt, -) +from arkindex.project.ponos_v1 import ponos +from arkindex.project.views import CdnHome, FrontendView, OpenAPIDocsView, RobotsTxt # Fallback to the dummy frontend view when CDN_ASSETS_URL is not set frontend_view = FrontendView if settings.CDN_ASSETS_URL is None else CdnHome @@ -23,18 +13,7 @@ frontend_view = FrontendView if settings.CDN_ASSETS_URL is None else CdnHome urlpatterns = [ path('api/v1/', include((api, 'api'), namespace='api')), path('api-docs/', OpenAPIDocsView.as_view(), name='openapi-docs'), - # Override Ponos endpoints - path('ponos/v1/secret/<path:name>', PonosSecretDetails.as_view(), name='secret-details'), - path('ponos/v1/agents/', PonosAgentsState.as_view(), name='ponos-agents'), - path('ponos/v1/agent/<uuid:pk>/', PonosAgentDetails.as_view(), name='ponos-agent-details'), - path('ponos/v1/task/<uuid:pk>/', PonosTaskUpdate.as_view(), name='ponos-task-update'), - path( - 'ponos/v1/task/<uuid:pk>/artifact/<path:path>', - PonosArtifactDownload.as_view(), - name='ponos-artifact-dl' - ), - path('ponos/v1/workflow/<uuid:pk>/', PonosWorkflowDetails.as_view(), name='ponos-workflow-details'), - path('ponos/', include('ponos.urls')), + path('ponos/v1/', include((ponos, 'ponos'), namespace='ponos')), path('admin/', admin.site.urls), path('rq/', include('django_rq.urls')), # Frontend URLs the backend needs with django.urls.reverse diff --git a/arkindex/project/views.py b/arkindex/project/views.py index b2c386bad6..5a8a9c95f9 100644 --- a/arkindex/project/views.py +++ b/arkindex/project/views.py @@ -1,23 +1,7 @@ from django.conf import settings -from django.shortcuts import get_object_or_404 from django.views.generic import TemplateView, View -from drf_spectacular.utils import extend_schema, extend_schema_view -from rest_framework import permissions -from rest_framework.exceptions import NotFound, PermissionDenied -from arkindex.ponos.api import ( - AgentDetails, - AgentsState, - SecretDetails, - TaskArtifactDownload, - TaskUpdate, - WorkflowDetails, -) -from arkindex.process.models import Process -from arkindex.process.permissions import IsArtifactAdmin, IsTaskAdmin -from arkindex.project.mixins import CachedViewMixin, ProcessACLMixin -from arkindex.project.permissions import IsInternal, IsVerified -from arkindex.users.models import Role +from arkindex.project.mixins import CachedViewMixin class FrontendView(View): @@ -58,70 +42,6 @@ class OpenAPIDocsView(CachedViewMixin, TemplateView): cache_prefix = settings.VERSION -class PonosSecretDetails(SecretDetails): - """ - Allow Arkindex internal users to retrieve a secret as if they were Ponos agents - """ - permission_classes = (IsInternal, ) - - -class PonosAgentsState(AgentsState): - """ - Allow any verified user to see the state of Ponos agents on this instance - """ - permission_classes = (IsVerified, ) - - -class PonosAgentDetails(AgentDetails): - """ - Allow any verified user to see the details of an agent including all its running tasks - """ - permission_classes = (IsVerified, ) - - -class PonosTaskUpdate(TaskUpdate): - """ - Allow users with an admin privilege to update a process task - """ - permission_classes = (IsTaskAdmin, ) - - -class PonosArtifactDownload(TaskArtifactDownload): - """ - Allow users with an admin privilege to download an artifact of a process task - """ - permission_classes = (IsArtifactAdmin, ) - - -@extend_schema_view( - put=extend_schema(description="Update a workflow's status and tasks.\n\nRequires an **admin** right to its process."), - patch=extend_schema(description='Partially update a workflow.\n\nRequires an **admin** right to its process.') -) -class PonosWorkflowDetails(ProcessACLMixin, WorkflowDetails): - """ - Allow users with an admin access to a process to retry its workflow - """ - permission_classes = (IsVerified, ) - - def check_object_permissions(self, request, workflow): - super().check_object_permissions(request, workflow) - - required_access = Role.Admin.value - # Allow a user with a read access on the process to retrieve its workflow - if request.method in permissions.SAFE_METHODS: - required_access = Role.Guest.value - - process = get_object_or_404( - Process, - workflow_id=workflow.id - ) - access_level = self.process_access_level(process) - if not access_level: - raise NotFound - if access_level < required_access: - raise PermissionDenied(detail='You do not have an admin access to this process.') - - class RobotsTxt(TemplateView): """ A plain text view to serve robots.txt with disallowed paths -- GitLab