From 847688b4bd36a8246afbdae7f326c96e09568705 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 15 Jan 2021 13:03:21 +0000
Subject: [PATCH] Add specific permissions for tasks PATCH and artifacts
 download

---
 arkindex/project/tests/test_ponos_view.py | 89 ++++++++++++++++-------
 arkindex/project/urls.py                  |  6 ++
 arkindex/project/views.py                 | 45 ++++++++++--
 3 files changed, 107 insertions(+), 33 deletions(-)

diff --git a/arkindex/project/tests/test_ponos_view.py b/arkindex/project/tests/test_ponos_view.py
index 1fd6887090..22939fe95b 100644
--- a/arkindex/project/tests/test_ponos_view.py
+++ b/arkindex/project/tests/test_ponos_view.py
@@ -1,17 +1,38 @@
 import tempfile
 from unittest.mock import patch
 
-from django.test import TestCase, override_settings
+from django.test import override_settings
 from django.urls import reverse
 from rest_framework import status
 
 from arkindex.dataimport.models import DataImport, DataImportMode
-from arkindex.users.models import User
-from ponos.models import Agent, Farm, Secret, Workflow, encrypt
+from arkindex.documents.models import Corpus
+from arkindex.project.tests import FixtureAPITestCase
+from arkindex.users.models import Role, User
+from ponos.models import Agent, Artifact, Farm, Secret, encrypt
 
 
 @override_settings(PONOS_PRIVATE_KEY='staging')
-class TestPonosView(TestCase):
+class TestPonosView(FixtureAPITestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+        cls.dataimport_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.dataimport_corpus, level=Role.Admin.value)
+
+        cls.creator = User.objects.create(email="creator@user.me")
+        cls.artifact = Artifact.objects.get(path='/path/to/docker_build')
+        cls.task = cls.artifact.task
+        cls.workflow = cls.task.workflow
+        cls.task = cls.workflow.tasks.get()
+        cls.dataimport = DataImport.objects.create(
+            mode=DataImportMode.Repository,
+            creator=cls.creator,
+            workflow=cls.workflow,
+            corpus=cls.dataimport_corpus
+        )
 
     @patch("ponos.models.settings")
     def test_internal_user_retrieve_secret(self, settings_mock):
@@ -82,27 +103,43 @@ class TestPonosView(TestCase):
 
     def test_update_task(self):
         """
-        Only admin users or the task creator should have the ability to update a task
+        Only users with an admin privilege and creators have the ability to update a dataimport task
         """
-        creator = User.objects.create(email="creator@user.fr")
-        workflow = Workflow.objects.create(
-            recipe="tasks: {test: {image: alpine}}"
+        test_cases = (
+            (None, status.HTTP_403_FORBIDDEN, 1),
+            (self.creator, status.HTTP_200_OK, 9),
+            (self.user, status.HTTP_403_FORBIDDEN, 9),
+            (self.superuser, status.HTTP_200_OK, 7),
+            (self.corpus_admin, status.HTTP_200_OK, 11),
         )
-        workflow.start()
-        task = workflow.tasks.get()
-        DataImport.objects.create(mode=DataImportMode.Repository, creator=creator, workflow=workflow)
-
-        response = self.client.patch(reverse('ponos-task-update', kwargs={'pk': str(task.id)}), json={"state": "stopping"})
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-
-        self.client.force_login(User.objects.create(email="lambda@user.fr"))
-        response = self.client.patch(reverse('ponos-task-update', kwargs={'pk': str(task.id)}), json={"state": "stopping"})
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-
-        self.client.force_login(User.objects.create(email='admin@admin.fr', is_admin=True))
-        response = self.client.patch(reverse('ponos-task-update', kwargs={'pk': str(task.id)}), json={"state": "stopping"})
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-
-        self.client.force_login(creator)
-        response = self.client.patch(reverse('ponos-task-update', kwargs={'pk': str(task.id)}), json={"state": "stopping"})
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        for user, status_code, requests_count in test_cases:
+            if user:
+                self.client.force_login(user)
+            with self.assertNumQueries(requests_count):
+                response = self.client.patch(
+                    reverse('ponos-task-update', kwargs={'pk': str(self.task.id)}),
+                    json={'state': 'stopping'}
+                )
+                self.assertEqual(response.status_code, status_code)
+
+    def test_download_artifacts(self):
+        """
+        Only users with an admin privilege and creators have the ability to
+        download an artifact of a dataimport task
+        """
+        test_cases = (
+            (None, status.HTTP_403_FORBIDDEN, 2),
+            (self.creator, status.HTTP_302_FOUND, 6),
+            (self.user, status.HTTP_403_FORBIDDEN, 9),
+            (self.superuser, status.HTTP_302_FOUND, 4),
+            (self.corpus_admin, status.HTTP_302_FOUND, 8),
+        )
+        for user, status_code, requests_count in test_cases:
+            if user:
+                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}),
+                    follow=False
+                )
+                self.assertEqual(response.status_code, status_code)
diff --git a/arkindex/project/urls.py b/arkindex/project/urls.py
index b6c81bde83..220771de82 100644
--- a/arkindex/project/urls.py
+++ b/arkindex/project/urls.py
@@ -10,6 +10,7 @@ from arkindex.project.views import (
     OpenAPIDocsView,
     PonosAgentDetails,
     PonosAgentsState,
+    PonosArtifactDownload,
     PonosSecretDetails,
     PonosTaskUpdate,
 )
@@ -25,6 +26,11 @@ urlpatterns = [
     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/', include('ponos.urls')),
     path('admin/', admin.site.urls),
     path('rq/', include('django_rq.urls')),
diff --git a/arkindex/project/views.py b/arkindex/project/views.py
index cc7473e347..81e8a0945d 100644
--- a/arkindex/project/views.py
+++ b/arkindex/project/views.py
@@ -2,9 +2,38 @@ from django.conf import settings
 from django.views.generic import TemplateView, View
 from rest_framework import permissions
 
-from arkindex.project.mixins import CachedViewMixin
+from arkindex.project.mixins import CachedViewMixin, CorpusACLMixin
 from arkindex.project.permissions import IsVerified
-from ponos.api import AgentDetails, AgentsState, SecretDetails, TaskUpdate
+from ponos.api import AgentDetails, AgentsState, SecretDetails, TaskArtifactDownload, TaskUpdate
+
+
+class IsTaskAdminOrCreator(CorpusACLMixin, permissions.BasePermission):
+    """
+    Permission to access a task with high privilege
+    Allowed for superuser, dataimport creators and users with an admin right on its associated 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
+
+        if not self.user.is_authenticated:
+            return False
+        return (
+            self.user.is_admin
+            or self.user.is_internal
+            or self.user.id == task.workflow.dataimport.creator_id
+            or (
+                task.workflow.dataimport.corpus_id
+                and self.has_admin_access(task.workflow.dataimport.corpus)
+            )
+        )
+
+
+class IsArtifactAdminOrCreator(IsTaskAdminOrCreator):
+
+    def has_object_permission(self, request, view, artifact):
+        return super().has_object_permission(request, view, artifact.task)
 
 
 class FrontendView(View):
@@ -70,11 +99,13 @@ class PonosAgentDetails(AgentDetails):
 
 class PonosTaskUpdate(TaskUpdate):
     """
-    Allow any super admin or the task creator to update a task.
+    Allow users with an admin privilege and creators to update a dataimport task
     """
-    class IsAdminOrCreator(permissions.BasePermission):
+    permission_classes = (IsTaskAdminOrCreator, )
 
-        def has_object_permission(self, request, view, obj):
-            return request.user.is_authenticated and (request.user.is_admin or request.user == obj.workflow.dataimport.creator)
 
-    permission_classes = (IsAdminOrCreator, )
+class PonosArtifactDownload(TaskArtifactDownload):
+    """
+    Allow users with an admin privilege and creators to download an artifact of a dataimport task
+    """
+    permission_classes = (IsArtifactAdminOrCreator, )
-- 
GitLab