From 0cdb29c7aec6086acf9060ba087a007c234ff899 Mon Sep 17 00:00:00 2001 From: Valentin Rigal <rigal@teklia.com> Date: Mon, 22 Feb 2021 15:25:08 +0000 Subject: [PATCH] Process rights --- arkindex/dataimport/api.py | 265 ++++++----- arkindex/dataimport/serializers/imports.py | 68 +-- arkindex/dataimport/tests/test_imports.py | 432 ++++++++++-------- .../dataimport/tests/test_process_elements.py | 57 +-- arkindex/dataimport/tests/test_workerruns.py | 46 +- arkindex/dataimport/tests/test_workers.py | 2 +- .../dataimport/tests/test_workflows_api.py | 104 +++-- arkindex/project/mixins.py | 35 +- arkindex/project/tests/test_acl_mixin.py | 62 ++- arkindex/project/tests/test_ponos_view.py | 40 +- arkindex/project/urls.py | 2 + arkindex/project/views.py | 38 +- 12 files changed, 725 insertions(+), 426 deletions(-) diff --git a/arkindex/dataimport/api.py b/arkindex/dataimport/api.py index cc074d2b66..847b785a8a 100644 --- a/arkindex/dataimport/api.py +++ b/arkindex/dataimport/api.py @@ -57,12 +57,13 @@ from arkindex.project.mixins import ( CorpusACLMixin, CustomPaginationViewMixin, DeprecatedMixin, + ProcessACLMixin, RepositoryACLMixin, SelectionMixin, WorkerACLMixin, ) from arkindex.project.permissions import IsVerified, IsVerifiedOrReadOnly -from arkindex.users.models import OAuthCredentials, Role, User +from arkindex.users.models import OAuthCredentials, Role from arkindex.users.utils import get_max_level from ponos.models import STATES_ORDERING, State @@ -81,14 +82,9 @@ logger = logging.getLogger(__name__) required=False, ), OpenApiParameter( - 'creator', - type={ - 'oneOf': [ - {'type': 'integer'}, - {'type': 'string', 'format': 'email'}, - ] - }, - description="Filter imports by creator ID or email address", + 'created', + type=bool, + description='Restrict to or exclude imports you created', required=False, ), OpenApiParameter( @@ -112,60 +108,55 @@ logger = logging.getLogger(__name__) 'with_workflow', type=bool, description='Restrict to or exclude imports with workflows', + required=False, ) ] ) ) -class DataImportsList(CorpusACLMixin, ListAPIView): +class DataImportsList(ProcessACLMixin, ListAPIView): """ - List all data imports + List all visible data imports. """ permission_classes = (IsVerified, ) serializer_class = DataImportLightSerializer def get_queryset(self): - with_workflow = self.request.query_params.get('with_workflow') - filters = Q( - Q( - Q(corpus__isnull=True) - | Q(corpus__in=Corpus.objects.readable(self.request.user)) - ) - & Q(workflow__isnull=bool(with_workflow and with_workflow.lower() in ('false', '0'))) - ) + filters = Q() + + if 'with_workflow' in self.request.query_params: + with_workflow = self.request.query_params['with_workflow'] + filters &= Q(workflow__isnull=bool(with_workflow and with_workflow.lower() in ('false', '0'))) if 'corpus' in self.request.query_params: corpus_id = self.request.query_params['corpus'] try: corpus_id = UUID(corpus_id) except (AttributeError, ValueError): - raise ValidationError({'corpus': "'{}' is not a valid UUID".format(corpus_id)}) - filters &= Q(corpus=self.get_corpus(corpus_id)) + raise ValidationError({'corpus': ["'{}' is not a valid UUID".format(corpus_id)]}) + # No supplementary validation is required on the corpus ID filter + filters &= Q(corpus=corpus_id) if 'mode' in self.request.query_params: try: filters &= Q(mode=DataImportMode(self.request.query_params['mode'])) except ValueError: raise ValidationError({ - 'mode': "Mode '{}' does not exist".format(self.request.query_params['mode']), + 'mode': ["Mode '{}' does not exist".format(self.request.query_params['mode'])] }) - if 'creator' in self.request.query_params: - try: - user_filters = {'id': int(self.request.query_params['creator'])} - except ValueError: - user_filters = {'email': self.request.query_params['creator']} - - try: - filters &= Q(creator=User.objects.get(**user_filters)) - except User.DoesNotExist: - raise ValidationError({ - 'creator': "User with email or id '{}' does not exist".format(self.request.query_params['creator']), - }) + if 'created' in self.request.query_params: + created = self.request.query_params['created'] + creator_filter = Q(creator_id=self.request.user.id) + if created.lower() in ('false', '0'): + creator_filter = ~creator_filter + filters &= creator_filter if 'id' in self.request.query_params: filters &= Q(id__startswith=self.request.query_params['id']) - qs = DataImport.objects.filter(filters).prefetch_related('workflow__tasks') + qs = self.readable_processes \ + .filter(filters) \ + .prefetch_related('workflow__tasks') # Order workflow by date of last updated task in workflow qs = qs.annotate( last_task=Max('workflow__tasks__updated'), @@ -177,7 +168,7 @@ class DataImportsList(CorpusACLMixin, ListAPIView): try: state = State(self.request.query_params['state']) except ValueError: - raise ValidationError({'state': f"State '{state_value}' does not exist"}) + raise ValidationError({'state': [f"State '{state_value}' does not exist"]}) # Filter out processes which have a task with an incompatible state on their last run excluding_states = STATES_ORDERING[:STATES_ORDERING.index(state)] excluded_imports = qs.filter( @@ -196,21 +187,29 @@ class DataImportsList(CorpusACLMixin, ListAPIView): @extend_schema(tags=['imports']) @extend_schema_view( - delete=extend_schema(description='Delete a data import. Cannot be used on currently running data imports.') + get=extend_schema(description='Retrieve details of a process.\n\nRequires a **guest** access.'), + patch=extend_schema(description='Partially update a process.\n\nRequires an **admin** access.'), + put=extend_schema(description='Update dependencies of a process.\n\nRequires an **admin** access.'), + delete=extend_schema( + description=( + 'Delete a process.\n\n' + 'Cannot be used on currently running data imports. ' + 'Requires an **admin** access.' + ) + ), ) -class DataImportDetails(CorpusACLMixin, RetrieveUpdateDestroyAPIView): +class DataImportDetails(ProcessACLMixin, RetrieveUpdateDestroyAPIView): """ Retrieve or edit a process """ - permission_classes = (IsVerified, ) serializer_class = DataImportSerializer def get_queryset(self): - return DataImport.objects.filter( - Q(corpus__isnull=True) - | Q(corpus__in=Corpus.objects.readable(self.request.user)) - ).select_related('corpus', 'workflow').prefetch_related('workflow__tasks').annotate(last_run=Max('workflow__tasks__run')) + return DataImport.objects \ + .select_related('corpus', 'workflow') \ + .prefetch_related('workflow__tasks') \ + .annotate(last_run=Max('workflow__tasks__run')) def get_object(self): if not hasattr(self, 'dataimport'): @@ -223,25 +222,28 @@ class DataImportDetails(CorpusACLMixin, RetrieveUpdateDestroyAPIView): context['dataimport'] = self.get_object() return context - def check_object_permissions(self, request, instance): - super().check_object_permissions(request, instance) - if request.method == 'GET': - return + def check_object_permissions(self, request, process): + super().check_object_permissions(request, process) - if not instance.corpus: - user = request.user - # Fall back basic user checks - if user != instance.creator and not user.is_admin: - self.permission_denied(request, message='You are not the creator of this process nor an admin.') - elif not self.has_write_access(instance.corpus): - self.permission_denied(request, message=f'You have no write access to corpus "{instance.corpus.name}".') - if request.method == 'DELETE' and instance.state == State.Running: + access_level = self.process_access_level(process) + required_access = Role.Admin.value + if request.method in permissions.SAFE_METHODS: + # A guest role allow to retrieve information on a process + required_access = Role.Guest.value + + if not access_level: + raise NotFound + if access_level < required_access: + raise PermissionDenied(detail='You do not have a sufficient access level to this process.') + + if request.method == 'DELETE' and process.state == State.Running: raise ValidationError({'__all__': ['Cannot delete a workflow while it is running']}) -class DataImportRetry(CorpusACLMixin, GenericAPIView): +class DataImportRetry(ProcessACLMixin, GenericAPIView): """ - Retry a data import. Can only be used on imports with Error, Failed, Stopped or Completed states. + Retry a data import. Can only be used on imports with Error, Failed, Stopped or Completed states.\n\n + Requires an **admin** access to the process. """ permission_classes = (IsVerified, ) serializer_class = DataImportSerializer @@ -258,18 +260,21 @@ class DataImportRetry(CorpusACLMixin, GenericAPIView): request=None, ) def post(self, request, *args, **kwargs): - instance = self.get_object() - if not instance.corpus: - user = self.request.user - # Fall back basic user checks - if user is not instance.creator and not user.is_admin: - raise PermissionDenied - if instance.workflow is not None and instance.state in (State.Unscheduled, State.Pending): - raise ValidationError('This workflow is already pending') - elif instance.state == State.Running: - raise ValidationError('This workflow is already running') - instance.retry() - serializer = self.get_serializer(instance) + process = self.get_object() + + access_level = self.process_access_level(process) + if not access_level: + raise NotFound + if access_level < Role.Admin.value: + raise PermissionDenied(detail='You do not have an admin access to this process.') + + if process.workflow is not None and process.state in (State.Unscheduled, State.Pending): + raise ValidationError({'__all__': ['This workflow is already pending']}) + elif process.state == State.Running: + raise ValidationError({'__all__': ['This workflow is already running']}) + + process.retry() + serializer = self.get_serializer(process) return Response(serializer.data) @@ -303,7 +308,7 @@ class DataImportFromFiles(CreateAPIView): if folder and folder.corpus != corpus: # The files' corpus is already validated as writable - raise ValidationError('Element and files are in different corpora') + raise ValidationError({'__all__': ['Element and files are in different corpora']}) # Start a dataimport with thumbnails generation self.dataimport = corpus.imports.create( @@ -324,9 +329,10 @@ class DataImportFromFiles(CreateAPIView): responses={201: DataImportSerializer}, ) ) -class CorpusWorkflow(SelectionMixin, CreateAPIView): +class CorpusWorkflow(SelectionMixin, CorpusACLMixin, CreateAPIView): """ - Create a distributed workflow from elements of an Arkindex corpus + Create a distributed workflow from elements of an Arkindex corpus.\n\n + Requires an **admin** access to the corpus. """ permission_classes = (IsVerified, ) serializer_class = ElementsWorkflowSerializer @@ -383,9 +389,10 @@ class CorpusWorkflow(SelectionMixin, CreateAPIView): ) -class StartProcess(APIView): +class StartProcess(CorpusACLMixin, APIView): """ - Start a process, used to build a Workflow with Workers + Start a process, used to build a Workflow with Workers.\n\n + Requires an **admin** access to the corpus of this process. """ permission_classes = (IsVerified, ) # For OpenAPI type discovery @@ -398,19 +405,26 @@ class StartProcess(APIView): responses=DataImportSerializer ) def post(self, request, pk=None, **kwargs): - dataimport = get_object_or_404(DataImport, pk=self.kwargs['pk']) + process = get_object_or_404( + DataImport.objects.filter(corpus_id__isnull=False), + pk=self.kwargs['pk'] + ) + + if not self.has_admin_access(process.corpus): + raise PermissionDenied(detail='You do not have an admin access to the corpus of this process.') - if dataimport.mode != DataImportMode.Workers or dataimport.workflow is not None: - raise ValidationError('Only a DataImport with Workers mode and not already launched can be started later on') + if process.mode != DataImportMode.Workers or process.workflow is not None: + raise ValidationError( + {'__all__': ['Only a DataImport with Workers mode and not already launched can be started later on']}) serializer = StartProcessSerializer(data=request.data) serializer.is_valid(raise_exception=True) - dataimport.start(**serializer.validated_data) + process.start(**serializer.validated_data) return Response( status=status.HTTP_200_OK, data=DataImportSerializer( - dataimport, + process, context={'request': request} ).data, ) @@ -882,17 +896,23 @@ class WorkerRetrieve(WorkerACLMixin, RetrieveAPIView): queryset = self.filter_queryset(self.get_queryset()).prefetch_related('repository') worker = get_object_or_404(queryset, pk=self.kwargs['pk']) if not self.has_read_access(worker): - raise PermissionDenied(detail='You do not have an execution access to this worker.') + raise PermissionDenied(detail='You do not have a guest access to this worker.') super().check_object_permissions(self.request, worker) return worker @extend_schema(tags=['imports']) @extend_schema_view( - get=extend_schema(description='List worker runs for a given process ID.'), - post=extend_schema(description='Create a worker run for a given process ID.'), + get=extend_schema(description=( + 'List worker runs for a given process ID.\n\n' + 'Requires an **admin** access to the process project.' + )), + post=extend_schema(description=( + 'Create a worker run for a given process ID.\n\n' + 'Requires an **admin** access to the process project and a **contributor** access to the worker.' + )), ) -class WorkerRunList(ListCreateAPIView, WorkerACLMixin): +class WorkerRunList(WorkerACLMixin, ListCreateAPIView): """ List worker runs for a given process UUID. Create a run on a process. @@ -902,14 +922,27 @@ class WorkerRunList(ListCreateAPIView, WorkerACLMixin): queryset = WorkerRun.objects.none() def get_queryset(self): - return WorkerRun.objects \ - .filter(dataimport_id=self.kwargs['pk']) \ + process = get_object_or_404( + DataImport.objects.filter(corpus_id__isnull=False), + pk=self.kwargs['pk'] + ) + + if not self.has_access(process.corpus, Role.Admin.value): + raise PermissionDenied(detail='You do not have an admin access to the corpus of this process.') + + return process.worker_runs \ .select_related('version__worker') \ .order_by('id') def perform_create(self, serializer): - dataimport = get_object_or_404(DataImport, pk=self.kwargs['pk']) + process = get_object_or_404( + DataImport.objects.filter(corpus_id__isnull=False), + pk=self.kwargs['pk'] + ) + # User must have admin access to the process project and a contributor access to the worker run + if not self.has_access(process.corpus, Role.Admin.value): + raise PermissionDenied(detail='You do not have an admin access to the corpus of this process.') try: worker = Worker.objects.get(versions__id=serializer.validated_data['version_id']) except Worker.DoesNotExist: @@ -917,26 +950,34 @@ class WorkerRunList(ListCreateAPIView, WorkerACLMixin): if not self.has_execution_access(worker): raise ValidationError({'version_id': ['You do not have an execution access to this version.']}) - if dataimport.mode != DataImportMode.Workers: + if process.mode != DataImportMode.Workers: raise ValidationError({'dataimport': ['Import mode must be Workers']}) - if dataimport.workflow_id is not None: + if process.workflow_id is not None: raise ValidationError({'__all__': ["Cannot create a WorkerRun on a DataImport that has already started"]}) - serializer.save(dataimport=dataimport) + serializer.save(dataimport=process) @extend_schema(tags=['imports']) @extend_schema_view( - get=extend_schema(description='Retrieve details of a worker run'), - patch=extend_schema(description='Partially update a worker run'), - put=extend_schema(description='Update dependencies of a worker run'), - delete=extend_schema(description='Delete a worker run'), + get=extend_schema( + description='Retrieve details of a worker run.\n\nRequires an **admin** access to the process project.' + ), + patch=extend_schema( + description='Partially update a worker run.\n\nRequires an **admin** access to the process project.' + ), + put=extend_schema( + description='Update dependencies of a worker run.\n\nRequires an **admin** access to the process project.' + ), + delete=extend_schema( + description='Delete a worker run.\n\nRequires an **admin** access to the process project.' + ), ) -class WorkerRunDetails(WorkerACLMixin, RetrieveUpdateDestroyAPIView): +class WorkerRunDetails(CorpusACLMixin, RetrieveUpdateDestroyAPIView): """ Retrieve or edit a worker run i.e. update its dependencies. - An execution right is required on the worker. + Requires an admin access to the process project. """ permission_classes = (IsVerified, ) serializer_class = WorkerRunEditSerializer @@ -944,25 +985,23 @@ class WorkerRunDetails(WorkerACLMixin, RetrieveUpdateDestroyAPIView): def get_queryset(self): # Use default DB to avoid a race condition checking process workflow return WorkerRun.objects \ + .filter(dataimport__corpus_id__isnull=False) \ .using('default') \ - .select_related('version__worker', 'dataimport__workflow') + .select_related('version__worker', 'dataimport__workflow', 'dataimport__corpus') def check_object_permissions(self, request, worker_run): - # This may cause a dead lock if a user lose their access and then try to edit a worker run on their process - if not self.has_execution_access(worker_run.version.worker): - raise PermissionDenied(detail='You do not have an execution access to this worker.') - super().check_object_permissions(request, worker_run) + if not self.has_admin_access(worker_run.dataimport.corpus): + raise PermissionDenied(detail='You do not have an admin access to the process project.') - def perform_update(self, serializer): - if serializer.instance.dataimport.workflow_id is not None: + # Updating a worker run is not possible once the process is started + if request.method not in permissions.SAFE_METHODS and worker_run.dataimport.workflow_id is not None: raise ValidationError({'__all__': ["Cannot update a WorkerRun on a DataImport that has already started"]}) - super().perform_update(serializer) + super().check_object_permissions(request, worker_run) def perform_destroy(self, instance): # Remove references to this instance from its children parents field instance.dataimport.worker_runs.filter(parents__contains=[instance.id]).update(parents=ArrayRemove('parents', instance.id)) - return super().perform_destroy(instance) @@ -1009,9 +1048,10 @@ class ImportTranskribus(CreateAPIView): @extend_schema_view(get=extend_schema(operation_id='ListProcessElements', tags=['imports'])) -class ListProcessElements(CustomPaginationViewMixin, ListAPIView): +class ListProcessElements(CustomPaginationViewMixin, CorpusACLMixin, ListAPIView): """ - List all elements for a specific process + List all elements for a process with workers.\n\n + Requires an **admin** access to the process corpus. """ permission_classes = (IsVerified, ) serializer_class = ProcessElementSerializer @@ -1019,17 +1059,16 @@ class ListProcessElements(CustomPaginationViewMixin, ListAPIView): queryset = Element.objects.none() def get_queryset(self): - dataimport = get_object_or_404( + process = get_object_or_404( # Avoid stale read on newly created dataimports - DataImport.objects.using('default'), + DataImport.objects.using('default').filter(corpus_id__isnull=False), Q(pk=self.kwargs['pk']) - & Q( - Q(corpus_id__isnull=False) - & Q(corpus_id__in=Corpus.objects.readable(self.request.user).values('id')) - ) ) + if not self.has_admin_access(process.corpus): + raise PermissionDenied(detail='You do not have an admin access to the corpus of this process.') + try: - return dataimport.list_elements().values('id', 'type__slug', 'name') + return process.list_elements().values('id', 'type__slug', 'name') except AssertionError as e: raise ValidationError({'__all__': [str(e)]}) diff --git a/arkindex/dataimport/serializers/imports.py b/arkindex/dataimport/serializers/imports.py index 64603c2509..455d67d35c 100644 --- a/arkindex/dataimport/serializers/imports.py +++ b/arkindex/dataimport/serializers/imports.py @@ -3,6 +3,7 @@ from uuid import UUID from django.conf import settings from django.db.models import Q from rest_framework import serializers +from rest_framework.exceptions import ValidationError from arkindex.dataimport.models import DataFile, DataImport, DataImportMode, WorkerRun from arkindex.dataimport.serializers.git import RevisionSerializer @@ -10,6 +11,8 @@ from arkindex.dataimport.serializers.workers import WorkerLightSerializer from arkindex.documents.models import Classification, ClassificationState, Corpus, Element, ElementType from arkindex.documents.serializers.elements import ElementSlimSerializer from arkindex.project.serializer_fields import BestClassField, EnumField +from arkindex.users.models import Role +from arkindex.users.utils import get_max_level from ponos.models import State from transkribus import TranskribusAPI @@ -192,10 +195,10 @@ class StartProcessSerializer(serializers.Serializer): class ElementsWorkflowSerializer(serializers.Serializer): - corpus = serializers.PrimaryKeyRelatedField(queryset=Corpus.objects.none()) + corpus = serializers.UUIDField() # Elements filtering options - element = serializers.PrimaryKeyRelatedField(queryset=Element.objects.none(), required=False) + element = serializers.UUIDField(required=False) process_name = serializers.CharField(required=False, max_length=150) name = serializers.CharField(required=False, max_length=250) type = serializers.SlugField(required=False, max_length=50) @@ -203,18 +206,27 @@ class ElementsWorkflowSerializer(serializers.Serializer): selection = serializers.BooleanField(default=False) load_children = serializers.BooleanField(default=False) - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - if not self.context.get('request'): - # Do not raise Error in order to create OpenAPI schema - return - corpora = Corpus.objects.writable(self.context['request'].user) - self.fields['corpus'].queryset = corpora - # Only elements representing a folder or containing a zone can be imported - self.fields['element'].queryset = Element.objects.filter( - Q(type__folder=True) | Q(zone__isnull=False), - corpus__in=corpora - ) + def validate_corpus(self, corpus_id): + try: + corpus = Corpus.objects.get(id=corpus_id) + except Corpus.DoesNotExist: + raise ValidationError(['Corpus with this ID does not exist.']) + + # Assert user has an admin access to the corpus + access_level = get_max_level(self.context['request'].user, corpus) + if not access_level: + raise ValidationError(['Corpus with this ID does not exist.']) + elif access_level < Role.Admin.value: + raise ValidationError(['You do not have an admin access to this corpus.']) + return corpus + + def validate_element(self, element_id): + if not element_id: + return None + try: + return Element.objects.select_related('type').get(id=element_id) + except Element.DoesNotExist: + raise ValidationError(['Element with this ID does not exist.']) def validate(self, data): """ @@ -229,28 +241,32 @@ class ElementsWorkflowSerializer(serializers.Serializer): best_class = data.get('best_class') selection = data.get('selection') + # Ensure element is a folder or has a zone + if element and element.type.folder is False and element.zone_id is None: + raise ValidationError({'element': ['Element has to be a folder or an element with a zone.']}) + if selection and not settings.ARKINDEX_FEATURES['selection']: - raise serializers.ValidationError({'selection': ['Selection is not available on this instance.']}) + raise ValidationError({'selection': ['Selection is not available on this instance.']}) if selection and (element or name or elt_type or best_class is not None): - raise serializers.ValidationError({'__all__': [ + raise ValidationError({'__all__': [ 'Filtering parameters (element, name, type, best_class) cannot be used with selection parameter' ]}) if element and (name or elt_type or best_class is not None): - raise serializers.ValidationError({ + raise ValidationError({ '__all__': ['Filtering parameters (name, type, best_class) cannot be used with element parameter'] }) - if element: - # Assert element is in the right corpus - if corpus.id != element.corpus.id: - raise serializers.ValidationError({ - 'element': ['Element is not part of corpus {}'.format(corpus.name)] - }) + + # If element is defined ensure it is part of the right corpus + if element and element.corpus_id != corpus.id: + raise ValidationError({ + 'element': ['Element is not part of corpus {}'.format(corpus.name)] + }) if elt_type: # Check type filter is valid if not corpus.types.filter(slug=elt_type).exists(): - raise serializers.ValidationError({ + raise ValidationError({ 'type': ['Unrecognized element type filter'] }) targeted_elts = targeted_elts.filter(type__slug=elt_type) @@ -263,7 +279,7 @@ class ElementsWorkflowSerializer(serializers.Serializer): if isinstance(best_class, UUID): if not corpus.ml_classes.filter(id=best_class).exists(): - raise serializers.ValidationError({ + raise ValidationError({ 'best_class': [f'MLClass with ID {best_class} does not exist'] }) targeted_elts = targeted_elts.filter( @@ -281,7 +297,7 @@ class ElementsWorkflowSerializer(serializers.Serializer): # Assert filters match Arkindex elements # This check will not properly work when using the selection; this case is handled in the API endpoint. if not targeted_elts.exists(): - raise serializers.ValidationError({ + raise ValidationError({ '__all__': ['No element match filtering'] }) return data diff --git a/arkindex/dataimport/tests/test_imports.py b/arkindex/dataimport/tests/test_imports.py index 377af5a1f1..e41846f05c 100644 --- a/arkindex/dataimport/tests/test_imports.py +++ b/arkindex/dataimport/tests/test_imports.py @@ -53,8 +53,36 @@ class TestImports(FixtureAPITestCase): cls.page_type = ElementType.objects.get(corpus=cls.corpus, slug='page') def setUp(self): - super().setUp() - self.dataimport = self.corpus.imports.create(creator=self.user, mode=DataImportMode.Images) + # Create multiple processes the user can access + self.user2 = User.objects.create_user('user2@test.test', display_name='Process creator') + # Guest access (A user own a process on a corpus they are not a member anymore) + self.user_img_process = DataImport.objects.create( + mode=DataImportMode.Images, + creator=self.user, + corpus=Corpus.objects.create(name='Private'), + ) + # Contributor access + self.repo.memberships.create(user=self.user, level=Role.Contributor.value) + self.repository_process = DataImport.objects.create( + mode=DataImportMode.Repository, + creator=self.user2, + revision=self.rev, + ) + # Admin access + self.elts_process = self.corpus.imports.create(mode=DataImportMode.Workers, creator=self.user2) + self.processes = DataImport.objects.filter( + id__in=[self.user_img_process.id, self.repository_process.id, self.elts_process.id] + ) + + def _serialize_process(self, process): + return { + 'name': process.name, + 'id': str(process.id), + 'state': process.state.value, + 'mode': process.mode.value, + 'corpus': process.corpus_id and str(process.corpus.id), + 'workflow': process.workflow and str(process.workflow.id), + } def build_task(self, workflow_id, run, state, depth=1): """ @@ -71,56 +99,63 @@ class TestImports(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_list(self): + """ + A user can list processes they have access to + """ self.client.force_login(self.user) - response = self.client.get(reverse('api:import-list'), {'with_workflow': False}) + response = self.client.get(reverse('api:import-list')) self.assertEqual(response.status_code, status.HTTP_200_OK) - data = response.json() - self.assertEqual(len(data['results']), 1) - result = data['results'][0] - self.assertDictEqual(result, { - 'name': None, - 'id': str(self.dataimport.id), - 'state': State.Unscheduled.value, - 'mode': DataImportMode.Images.value, - 'corpus': str(self.corpus.id), - 'workflow': None, + self.assertDictEqual(response.json(), { + 'count': 3, + 'number': 1, + 'next': None, + 'previous': None, + 'results': [ + self._serialize_process(p) + for p in self.processes.order_by('id') + ] }) - def test_list_workflow_link(self): - dataimport2 = self.corpus.imports.create(creator=self.user, mode=DataImportMode.Workers) - self.assertIsNone(dataimport2.workflow) - + def test_list_with_workflow(self): + """ + Filter processes that have a workflow i.e. that have been started + """ + self.user_img_process.start() self.client.force_login(self.user) - self.client.post( - reverse('api:process-start', kwargs={'pk': str(dataimport2.id)}) - ) - dataimport2.refresh_from_db() - - response = self.client.get(reverse('api:import-list')) + with self.assertNumQueries(8): + response = self.client.get(reverse('api:import-list'), {'with_workflow': 'true'}) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() self.assertEqual(len(data['results']), 1) results = data['results'] self.assertListEqual(results, [{ 'name': None, - 'id': str(dataimport2.id), + 'id': str(self.user_img_process.id), 'state': State.Unscheduled.value, - 'mode': DataImportMode.Workers.value, - 'corpus': str(self.corpus.id), - 'workflow': f"http://testserver/ponos/v1/workflow/{dataimport2.workflow.id}/", + 'mode': DataImportMode.Images.value, + 'corpus': str(self.user_img_process.corpus.id), + 'workflow': f'http://testserver/ponos/v1/workflow/{self.user_img_process.workflow.id}/', }]) - response = self.client.get(reverse('api:import-list'), {'with_workflow': False}) + def test_list_exclude_workflow(self): + """ + Filter process that have not been started yet + """ + self.repository_process.start() + self.elts_process.start() + self.client.force_login(self.user) + with self.assertNumQueries(7): + response = self.client.get(reverse('api:import-list'), {'with_workflow': 'false'}) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() self.assertEqual(len(data['results']), 1) results = data['results'] self.assertListEqual(results, [{ 'name': None, - 'id': str(self.dataimport.id), + 'id': str(self.user_img_process.id), 'state': State.Unscheduled.value, 'mode': DataImportMode.Images.value, - 'corpus': str(self.corpus.id), + 'corpus': str(self.user_img_process.corpus.id), 'workflow': None, }]) @@ -139,7 +174,7 @@ class TestImports(FixtureAPITestCase): self.client.force_login(self.user) response = self.client.get(reverse('api:import-list'), {'corpus': 'oh-no'}) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'corpus': "'oh-no' is not a valid UUID"}) + self.assertEqual(response.json(), {'corpus': ["'oh-no' is not a valid UUID"]}) def test_list_filter_mode(self): self.client.force_login(self.user) @@ -157,49 +192,39 @@ class TestImports(FixtureAPITestCase): self.client.force_login(self.user) response = self.client.get(reverse('api:import-list'), {'mode': 'unexisting_mode'}) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'mode': "Mode 'unexisting_mode' does not exist"}) + self.assertEqual(response.json(), {'mode': ["Mode 'unexisting_mode' does not exist"]}) - def test_list_filter_creator_by_email(self): + def test_list_filter_created(self): + """ + Display processes that have been created by the user + """ self.client.force_login(self.user) - self.corpus.imports.create( - creator=User.objects.create_user(email='another_user@user.fr'), - mode=DataImportMode.PDF, - ) - response = self.client.get(reverse('api:import-list'), {'creator': 'user@user.fr', 'with_workflow': False}) + response = self.client.get(reverse('api:import-list'), {'created': 'true'}) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() - self.assertEqual(len(data['results']), 1) - self.assertEqual(data['results'][0]['id'], str(self.dataimport.id)) + self.assertEqual(data['count'], 1) + self.assertEqual(data['results'][0]['id'], str(self.user_img_process.id)) - def test_list_filter_creator_by_id(self): + def test_list_filter_not_created(self): + """ + Display processes that have not been created by the user + """ self.client.force_login(self.user) - self.corpus.imports.create( - creator=User.objects.create_user(email='another_user@user.fr'), - mode=DataImportMode.PDF, - ) - response = self.client.get(reverse('api:import-list'), {'creator': self.user.id, 'with_workflow': False}) + response = self.client.get(reverse('api:import-list'), {'created': 'false'}) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() - self.assertEqual(len(data['results']), 1) - self.assertEqual(data['results'][0]['id'], str(self.dataimport.id)) - - def test_list_filter_unexisting_creator(self): - self.client.force_login(self.user) - response = self.client.get(reverse('api:import-list'), {'creator': 'blabla@blabla.fr'}) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'creator': "User with email or id 'blabla@blabla.fr' does not exist"}) - - self.client.force_login(self.user) - response = self.client.get(reverse('api:import-list'), {'creator': '1337'}) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'creator': "User with email or id '1337' does not exist"}) + self.assertEqual(data['count'], 2) + self.assertCountEqual( + [p['id'] for p in data['results']], + [str(p.id) for p in self.processes.exclude(creator=self.user)] + ) def test_list_filter_id(self): self.client.force_login(self.user) # Generate a UUID whose first characters are not the same as the first DataImport dataimport_id = str(uuid.uuid4()) - while dataimport_id[:10] == str(self.dataimport.id)[:10]: + while dataimport_id[:10] == str(self.user_img_process.id)[:10]: dataimport_id = str(uuid.uuid4()) dataimport2 = self.corpus.imports.create( @@ -217,12 +242,12 @@ class TestImports(FixtureAPITestCase): """ Ensure the DataImport reports an Unscheduled state when there are no tasks in its workflow """ - self.assertIsNone(self.dataimport.workflow) - self.dataimport.start() - self.dataimport.workflow.tasks.all().delete() + self.assertIsNone(self.user_img_process.workflow) + self.user_img_process.start() + self.user_img_process.workflow.tasks.all().delete() self.client.force_login(self.user) - response = self.client.get(reverse('api:import-list')) + response = self.client.get(reverse('api:import-list'), {'id': str(self.user_img_process.id)}) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() self.assertEqual(len(data['results']), 1) @@ -232,7 +257,7 @@ class TestImports(FixtureAPITestCase): self.client.force_login(self.user) response = self.client.get(reverse('api:import-list'), {'state': 'spain'}) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertDictEqual(response.json(), {'state': "State 'spain' does not exist"}) + self.assertDictEqual(response.json(), {'state': ["State 'spain' does not exist"]}) def test_list_process_state_filter(self): """ @@ -303,25 +328,26 @@ class TestImports(FixtureAPITestCase): self.build_task(*task) processes = [ DataImport.objects.get(workflow_id=workflow_id), - DataImport.objects.get(workflow=None) + *self.processes ] - # Wirkflows with no tasks are considered unscheduled + # Workflows with no tasks are considered unscheduled self.assertListEqual([p for p in processes if p.state.value == 'unscheduled'], processes) self.client.force_login(self.user) with self.assertNumQueries(8): response = self.client.get(reverse('api:import-list'), {'state': 'unscheduled', 'with_workflow': True}) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual([r['id'] for r in response.json()['results']], [str(processes[0].id)]) + self.assertCountEqual([r['id'] for r in response.json()['results']], [str(processes[0].id)]) self.client.force_login(self.user) with self.assertNumQueries(5): response = self.client.get(reverse('api:import-list'), {'state': 'unscheduled', 'with_workflow': False}) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual([r['id'] for r in response.json()['results']], [str(processes[1].id)]) + self.assertEqual(response.json()['count'], len(self.processes)) + self.assertCountEqual([r['id'] for r in response.json()['results']], [str(p.id) for p in processes[1:]]) def test_details_requires_login(self): - response = self.client.get(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + response = self.client.get(reverse('api:import-details', kwargs={'pk': self.user_img_process.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertDictEqual(response.json(), {'detail': 'Authentication credentials were not provided.'}) @@ -329,89 +355,88 @@ class TestImports(FixtureAPITestCase): """ Ensure the DataImport reports an Unscheduled state when there are no tasks in its workflow """ - self.assertIsNone(self.dataimport.workflow) - self.dataimport.start() - self.dataimport.workflow.tasks.all().delete() + self.assertIsNone(self.user_img_process.workflow) + self.user_img_process.start() + self.user_img_process.workflow.tasks.all().delete() self.client.force_login(self.user) - with self.assertNumQueries(9): - response = self.client.get(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + with self.assertNumQueries(11): + response = self.client.get(reverse('api:import-details', kwargs={'pk': self.user_img_process.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() self.assertEqual(data['state'], State.Unscheduled.value) def test_details(self): self.client.force_login(self.user) - with self.assertNumQueries(7): - response = self.client.get(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + with self.assertNumQueries(9): + response = self.client.get(reverse('api:import-details', kwargs={'pk': self.user_img_process.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() - self.assertEqual(data['id'], str(self.dataimport.id)) + self.assertEqual(data['id'], str(self.user_img_process.id)) - def test_details_destroy_verified(self): + def test_delete_requires_verified(self): user = User.objects.create(email='AAAAAAA') self.client.force_login(user) - response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + response = self.client.delete(reverse('api:import-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 permission to perform this action.'}) - def test_details_destroy_running(self): + def test_delete_running(self): + """ + It is not possible to delete a runnin import + """ self.client.force_login(self.user) - self.dataimport.start() - self.dataimport.workflow.tasks.update(state=State.Running) - response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + self.elts_process.start() + self.elts_process.workflow.tasks.update(state=State.Running) + response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.json(), {'__all__': ['Cannot delete a workflow while it is running']}) - def test_details_destroy_no_permission(self): + def test_delete_repository_import_no_permission(self): """ - A user cannot delete a dataimport linked to a corpus he has no write access to + Deletion of a repository import requires to be admin on the repository """ self.client.force_login(self.user) - self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value) - response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.repository_process.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertDictEqual(response.json(), {'detail': f'You have no write access to corpus "{self.corpus.name}".'}) + self.assertDictEqual(response.json(), {'detail': 'You do not have a sufficient access level to this process.'}) - def test_details_destroy_no_corpus_forbidden(self): + def test_delete_corpus_import_no_permission(self): """ - A user cannot delete a dataimport with a null corpus if he is not its creator nor an admin + A user cannot delete a dataimport linked to a corpus he has no admin access to """ self.client.force_login(self.user) - self.assertFalse(self.user.is_admin) - dataimport = DataImport.objects.create(mode=DataImportMode.Repository, creator=User.objects.create()) - response = self.client.delete(reverse('api:import-details', kwargs={'pk': dataimport.id})) + self.assertFalse(self.user_img_process.corpus.memberships.filter(user=self.user).exists()) + response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.user_img_process.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertDictEqual(response.json(), {'detail': 'You are not the creator of this process nor an admin.'}) + self.assertDictEqual(response.json(), {'detail': 'You do not have a sufficient access level to this process.'}) - def test_details_destroy_no_corpus_is_creator(self): + def test_delete_corpus_import_admin(self): """ - A user is allowed to delete a dataimport if he is the creator + A user is allowed to delete a dataimport if he has an admin right to its corpus """ self.client.force_login(self.user) - self.assertFalse(self.user.is_admin) - dataimport = DataImport.objects.create(mode=DataImportMode.Repository, creator=self.user) - response = self.client.delete(reverse('api:import-details', kwargs={'pk': dataimport.id})) + with self.assertNumQueries(10): + response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) - def test_details_destroy_no_corpus_is_admin(self): + def test_delete_superuser(self): """ - A user is allowed to delete a dataimport he has not created if he is an admin + A superuser is allowed to delete any dataimport """ - user = User.objects.create(email='admin', is_admin=True) - self.client.force_login(user) - dataimport = DataImport.objects.create(mode=DataImportMode.Repository, creator=User.objects.create()) - response = self.client.delete(reverse('api:import-details', kwargs={'pk': dataimport.id})) + self.client.force_login(self.superuser) + with self.assertNumQueries(7): + response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.user_img_process.id})) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) def test_update_process_requires_login(self): - response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.user_img_process.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertDictEqual(response.json(), {'detail': 'Authentication credentials were not provided.'}) def test_update_process_requires_verified(self): user = User.objects.create(email="unchecked") self.client.force_login(user) - response = self.client.delete(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + response = self.client.delete(reverse('api:import-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 permission to perform this action.'}) @@ -420,49 +445,52 @@ class TestImports(FixtureAPITestCase): A running process cannot be updated... (see following test) """ self.client.force_login(self.user) - self.dataimport.start() - self.dataimport.workflow.tasks.update(state=State.Running) - response = self.client.patch(reverse('api:import-details', kwargs={'pk': self.dataimport.id})) + self.elts_process.start() + self.elts_process.workflow.tasks.update(state=State.Running) + with self.assertNumQueries(10): + response = self.client.patch(reverse('api:import-details', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.json(), {'__all__': ['Cannot edit a workflow while it is running']}) def test_update_process_running_only_name(self): """ - Unless only name attribute is to be patched + Only name attribute of a running process can be patched """ self.client.force_login(self.user) - self.dataimport.start() - self.dataimport.workflow.tasks.update(state=State.Running) - self.assertEqual(self.dataimport.name, None) - response = self.client.patch( - reverse('api:import-details', kwargs={'pk': self.dataimport.id}), - { - 'name': 'newName' - }, - format='json' - ) + self.elts_process.start() + self.elts_process.workflow.tasks.update(state=State.Running) + self.assertEqual(self.elts_process.name, None) + with self.assertNumQueries(12): + response = self.client.patch( + reverse('api:import-details', kwargs={'pk': self.elts_process.id}), + {'name': 'newName'}, + format='json' + ) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.dataimport.refresh_from_db() - self.assertEqual(self.dataimport.name, 'newName') + self.elts_process.refresh_from_db() + self.assertEqual(self.elts_process.name, 'newName') def test_update_process_no_permission(self): """ - A user cannot update a dataimport linked to a corpus he has no write access to + A user cannot update a dataimport linked to a corpus he has no admin access to """ self.client.force_login(self.user) self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value) - dataimport = DataImport.objects.create(mode=DataImportMode.Workers, corpus=self.corpus, creator=self.user) - response = self.client.patch(reverse('api:import-details', kwargs={'pk': dataimport.id})) + response = self.client.patch(reverse('api:import-details', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertDictEqual(response.json(), {'detail': f'You have no write access to corpus "{self.corpus.name}".'}) + self.assertDictEqual(response.json(), {'detail': 'You do not have a sufficient access level to this process.'}) - def test_update_process_no_corpus_forbidden(self): + def test_update_process_repository_requires_admin(self): + """ + Edition of a repository import requires to be admin on the repository + """ self.client.force_login(self.user) - self.assertFalse(self.user.is_admin) - dataimport = DataImport.objects.create(mode=DataImportMode.Repository, creator=User.objects.create()) - response = self.client.patch(reverse('api:import-details', kwargs={'pk': dataimport.id})) + response = self.client.patch( + reverse('api:import-details', kwargs={'pk': self.repository_process.id}), + {'name': 'newName'} + ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertDictEqual(response.json(), {'detail': 'You are not the creator of this process nor an admin.'}) + self.assertDictEqual(response.json(), {'detail': 'You do not have a sufficient access level to this process.'}) def test_update_process_wrong_mode(self): """ @@ -511,13 +539,9 @@ class TestImports(FixtureAPITestCase): def test_update_process_corpus_no_write_right(self): self.client.force_login(self.user) self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value) - dataimport = DataImport.objects.create(mode=DataImportMode.Workers, corpus=self.corpus, creator=self.user) - response = self.client.patch(reverse('api:import-details', kwargs={'pk': dataimport.id})) + response = self.client.patch(reverse('api:import-details', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertDictEqual( - response.json(), - {'detail': f'You have no write access to corpus "{self.corpus.name}".'} - ) + self.assertDictEqual(response.json(), {'detail': 'You do not have a sufficient access level to this process.'}) def test_update_process(self): """ @@ -529,16 +553,17 @@ class TestImports(FixtureAPITestCase): (dataimport.name, dataimport.name_contains, dataimport.element_type, dataimport.load_children), (None, None, None, False) ) - response = self.client.patch( - reverse('api:import-details', kwargs={'pk': dataimport.id}), - { - 'name': 'newName', - 'name_contains': 'AAA', - 'element_type': 'page', - 'load_children': True - }, - format='json' - ) + with self.assertNumQueries(11): + response = self.client.patch( + reverse('api:import-details', kwargs={'pk': dataimport.id}), + { + 'name': 'newName', + 'name_contains': 'AAA', + 'element_type': 'page', + 'load_children': True + }, + format='json' + ) self.assertEqual(response.status_code, status.HTTP_200_OK) dataimport.refresh_from_db() self.assertTupleEqual( @@ -630,70 +655,71 @@ class TestImports(FixtureAPITestCase): }) def test_retry_requires_login(self): - response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.dataimport.id})) + response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.user_img_process.id})) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_retry_repository_process_no_right(self): + """ + A user that is not the creator nor admin cannot restart a dataimport that is not linked to any corpus + """ + self.client.force_login(self.user) + self.repository_process.start() + self.repository_process.workflow.tasks.all().update(state=State.Error) + self.assertEqual(self.repository_process.state, State.Error) + response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.repository_process.id})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual(response.json(), {'detail': 'You do not have an admin access to this process.'}) def test_retry_only_final(self): self.client.force_login(self.user) - self.dataimport.start() - self.assertEqual(self.dataimport.state, State.Unscheduled) - response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.dataimport.id})) + self.elts_process.start() + self.assertEqual(self.elts_process.state, State.Unscheduled) + response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) def test_retry_running(self): self.client.force_login(self.user) - self.dataimport.start() - task = self.dataimport.workflow.tasks.all().last() + self.elts_process.start() + task = self.elts_process.workflow.tasks.all().last() task.state = State.Running task.save() - response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.dataimport.id})) + response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) def test_retry(self): self.client.force_login(self.user) - self.dataimport.start() - self.dataimport.workflow.tasks.all().update(state=State.Error) - self.assertEqual(self.dataimport.state, State.Error) - response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.dataimport.id})) + self.elts_process.start() + self.elts_process.workflow.tasks.all().update(state=State.Error) + self.assertEqual(self.elts_process.state, State.Error) + with self.assertNumQueries(28): + response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.dataimport.refresh_from_db() - self.assertEqual(self.dataimport.state, State.Unscheduled) + self.elts_process.refresh_from_db() + self.assertEqual(self.elts_process.state, State.Unscheduled) def test_retry_no_workflow(self): self.client.force_login(self.user) - self.assertIsNone(self.dataimport.workflow) - response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.dataimport.id})) + self.assertIsNone(self.elts_process.workflow) + with self.assertNumQueries(20): + response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.elts_process.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.dataimport.refresh_from_db() - self.assertEqual(self.dataimport.state, State.Unscheduled) - self.assertIsNotNone(self.dataimport.workflow) + self.elts_process.refresh_from_db() + self.assertEqual(self.elts_process.state, State.Unscheduled) + self.assertIsNotNone(self.elts_process.workflow) def test_retry_repo_disabled(self): - self.client.force_login(self.user) - self.dataimport.mode = DataImportMode.Repository - self.dataimport.revision = self.rev - self.dataimport.save() - self.dataimport.start() - self.dataimport.workflow.tasks.all().update(state=State.Error) - self.assertEqual(self.dataimport.state, State.Error) + self.repository_process.revision = self.rev + self.repository_process.save() + self.repository_process.start() + self.repository_process.workflow.tasks.all().update(state=State.Error) + self.assertEqual(self.repository_process.state, State.Error) self.creds.delete() - response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.dataimport.id})) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.creds.save() - - def test_retry_no_corpus_not_creator_nor_admin(self): - """ - A user that is not the creator nor admin cannot restart a dataimport that is not linked to any corpus - """ + # Allow the user to do the retry + self.repo.memberships.filter(user=self.user).update(level=Role.Admin.value) self.client.force_login(self.user) - self.assertFalse(self.user.is_admin) - creator = User.objects.create() - dataimport = DataImport.objects.create(mode=DataImportMode.Repository, revision=self.rev, creator=creator) - dataimport.start() - dataimport.workflow.tasks.all().update(state=State.Error) - self.assertEqual(dataimport.state, State.Error) - response = self.client.post(reverse('api:import-retry', kwargs={'pk': dataimport.id})) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.repository_process.id})) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json(), ['Git repository does not have any valid credentials']) def test_from_files_requires_login(self): response = self.client.post(reverse('api:import-from-files'), { @@ -871,7 +897,7 @@ class TestImports(FixtureAPITestCase): def test_start_process_requires_login(self): response = self.client.post( - reverse('api:process-start', kwargs={'pk': str(self.dataimport.id)}) + reverse('api:process-start', kwargs={'pk': str(self.user_img_process.id)}) ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) @@ -884,14 +910,18 @@ class TestImports(FixtureAPITestCase): self.assertEqual(response.json(), {'detail': 'Not found.'}) def test_start_process_wrong_dataimport_mode(self): - self.assertNotEqual(self.dataimport.mode, DataImportMode.Workers) - + self.assertNotEqual(self.user_img_process.mode, DataImportMode.Workers) + # grant an admin access to this process + self.user_img_process.corpus.memberships.create(user=self.user, level=Role.Admin.value) self.client.force_login(self.user) response = self.client.post( - reverse('api:process-start', kwargs={'pk': str(self.dataimport.id)}) + reverse('api:process-start', kwargs={'pk': str(self.user_img_process.id)}) ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), ['Only a DataImport with Workers mode and not already launched can be started later on']) + self.assertEqual( + response.json(), + {'__all__': ['Only a DataImport with Workers mode and not already launched can be started later on']} + ) def test_start_process_dataimport_already_started(self): dataimport2 = self.corpus.imports.create(creator=self.user, mode=DataImportMode.Workers) @@ -904,16 +934,20 @@ class TestImports(FixtureAPITestCase): reverse('api:process-start', kwargs={'pk': str(dataimport2.id)}) ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), ['Only a DataImport with Workers mode and not already launched can be started later on']) + self.assertEqual( + response.json(), + {'__all__': ['Only a DataImport with Workers mode and not already launched can be started later on']} + ) def test_start_process(self): dataimport2 = self.corpus.imports.create(creator=self.user, mode=DataImportMode.Workers) self.assertIsNone(dataimport2.workflow) self.client.force_login(self.user) - response = self.client.post( - reverse('api:process-start', kwargs={'pk': str(dataimport2.id)}) - ) + with self.assertNumQueries(20): + response = self.client.post( + reverse('api:process-start', kwargs={'pk': str(dataimport2.id)}) + ) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json()['id'], str(dataimport2.id)) dataimport2.refresh_from_db() diff --git a/arkindex/dataimport/tests/test_process_elements.py b/arkindex/dataimport/tests/test_process_elements.py index 65204bc8f1..9ce2f638ac 100644 --- a/arkindex/dataimport/tests/test_process_elements.py +++ b/arkindex/dataimport/tests/test_process_elements.py @@ -6,6 +6,7 @@ from rest_framework import status from arkindex.dataimport.models import DataImport, DataImportMode, WorkerVersion from arkindex.documents.models import Classification, ClassificationState, Corpus, Element, MLClass from arkindex.project.tests import FixtureAPITestCase +from arkindex.users.models import User class TestProcessElements(FixtureAPITestCase): @@ -186,10 +187,12 @@ class TestProcessElements(FixtureAPITestCase): def test_no_access(self): self.dataimport.corpus = Corpus.objects.create(name='private') + self.corpus.creator = User.objects.create_user('John Doe') self.dataimport.save() self.client.force_login(self.user) response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertDictEqual(response.json(), {'detail': 'You do not have an admin access to the corpus of this process.'}) def test_filter_elements_wrong_corpus(self): """ @@ -200,7 +203,7 @@ class TestProcessElements(FixtureAPITestCase): self.dataimport.elements.add(self.page_1.id, self.folder_2.id) self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertDictEqual(response.json(), { '__all__': [f'Some elements on this process are not part of corpus {self.dataimport.corpus_id}'] @@ -220,7 +223,7 @@ class TestProcessElements(FixtureAPITestCase): self.dataimport.elements.add(element.id, self.page_1.id) self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertDictEqual(response.json(), { '__all__': [f'Some elements on this process are not part of corpus {self.dataimport.corpus_id}'] @@ -236,7 +239,7 @@ class TestProcessElements(FixtureAPITestCase): self.dataimport.save() self.client.force_login(self.superuser) - with self.assertNumQueries(4): + with self.assertNumQueries(5): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertDictEqual(response.json(), { '__all__': [f'Element {self.page_1.id} is not part of corpus {self.dataimport.corpus_id}'] @@ -248,7 +251,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_1, self.page_1, self.page_5] self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -271,7 +274,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_1, self.page_1] self.client.force_login(self.superuser) - with self.assertNumQueries(6): + with self.assertNumQueries(7): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -292,7 +295,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_1, self.folder_2] self.client.force_login(self.superuser) - with self.assertNumQueries(6): + with self.assertNumQueries(7): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -315,7 +318,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.line_1, self.line_2, self.line_3] self.client.force_login(self.superuser) - with self.assertNumQueries(7): + with self.assertNumQueries(8): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -336,7 +339,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.page_5, self.page_3, self.folder_2, self.page_2] self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -357,7 +360,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.page_1, self.page_5, self.page_3, self.folder_2, self.page_2] self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -378,7 +381,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_1, self.line_1, self.line_2, self.line_3, self.line_4, self.line_5, self.page_4] self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -398,7 +401,7 @@ class TestProcessElements(FixtureAPITestCase): self.dataimport.save() self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -416,7 +419,7 @@ class TestProcessElements(FixtureAPITestCase): self.dataimport.elements.add(self.page_1.id, self.folder_2.id) self.client.force_login(self.superuser) - with self.assertNumQueries(6): + with self.assertNumQueries(7): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -442,7 +445,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_1, self.page_1, self.page_5] self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -464,7 +467,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_1, self.folder_2] self.client.force_login(self.superuser) - with self.assertNumQueries(6): + with self.assertNumQueries(7): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -486,7 +489,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_2, self.page_2, self.page_3, self.page_5] self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -508,7 +511,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.page_1, self.page_5, self.page_3, self.folder_2, self.page_2] self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get( reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -531,7 +534,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_1, self.line_1, self.line_2, self.line_3, self.line_4, self.line_5, self.page_4] self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -553,7 +556,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.folder_1, self.page_1, self.page_3, self.line_1, self.line_2, self.line_3, self.page_2] self.client.force_login(self.superuser) - with self.assertNumQueries(6): + with self.assertNumQueries(7): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -575,7 +578,7 @@ class TestProcessElements(FixtureAPITestCase): elements = [self.page_1, self.page_5, self.page_3, self.line_1, self.line_3, self.line_4, self.line_5, self.folder_2, self.page_4] self.client.force_login(self.superuser) - with self.assertNumQueries(7): + with self.assertNumQueries(8): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -596,7 +599,7 @@ class TestProcessElements(FixtureAPITestCase): elements = Element.objects.filter(corpus=self.private_corpus).order_by('name', 'type__slug') self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -618,7 +621,7 @@ class TestProcessElements(FixtureAPITestCase): for mode in (DataImportMode.Images, DataImportMode.PDF, DataImportMode.Repository, DataImportMode.IIIF, DataImportMode.Transkribus): self.dataimport.mode = mode self.dataimport.save() - with self.assertNumQueries(3): + with self.assertNumQueries(4): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -629,7 +632,7 @@ class TestProcessElements(FixtureAPITestCase): for mode in (DataImportMode.Elements, DataImportMode.Workers): self.dataimport.mode = mode self.dataimport.save() - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -663,12 +666,12 @@ class TestProcessElements(FixtureAPITestCase): self.dataimport.save() self.client.force_login(self.superuser) - with self.assertNumQueries(6): + with self.assertNumQueries(7): page_1 = self.client.get(reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id})) self.assertEqual(len(page_1.json()['results']), 20) next_page = page_1.json().get('next') self.assertIsNotNone(next_page) - with self.assertNumQueries(6): + with self.assertNumQueries(7): page_2 = self.client.get(next_page) self.assertIsNone(page_2.json()['next']) qs_1 = Element.objects.filter(id__in=[elt['id'] for elt in page_1.json()['results']]) @@ -691,7 +694,7 @@ class TestProcessElements(FixtureAPITestCase): Elements count can be retrieved with with_count parameter """ self.client.force_login(self.superuser) - with self.assertNumQueries(6): + with self.assertNumQueries(7): response = self.client.get( reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id}), {'page_size': 6, 'with_count': True} @@ -726,7 +729,7 @@ class TestProcessElements(FixtureAPITestCase): self.dataimport.save() self.client.force_login(self.superuser) - with self.assertNumQueries(5): + with self.assertNumQueries(6): response = self.client.get( reverse('api:process-elements-list', kwargs={'pk': self.dataimport.id}), {'page_size': 50} diff --git a/arkindex/dataimport/tests/test_workerruns.py b/arkindex/dataimport/tests/test_workerruns.py index 017deb88c3..c4b9512949 100644 --- a/arkindex/dataimport/tests/test_workerruns.py +++ b/arkindex/dataimport/tests/test_workerruns.py @@ -54,7 +54,7 @@ class TestWorkerRuns(FixtureAPITestCase): def test_runs_list(self): self.client.force_login(self.user) - with self.assertNumQueries(4): + with self.assertNumQueries(9): response = self.client.get(reverse('api:worker-run-list', kwargs={'pk': str(self.dataimport_1.id)})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -153,7 +153,7 @@ class TestWorkerRuns(FixtureAPITestCase): def test_runs_post_create_worker_run(self): self.client.force_login(self.user) - with self.assertNumQueries(11): + with self.assertNumQueries(14): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(self.dataimport_2.id)}), data={'version_id': str(self.version_1.id), 'parents': []}, format='json' @@ -173,6 +173,17 @@ class TestWorkerRuns(FixtureAPITestCase): } }) + def test_runs_post_via_repository_right(self): + self.worker_1.memberships.filter(user=self.user).update(level=Role.Guest.value) + self.worker_1.repository.memberships.create(user=self.user, level=Role.Contributor.value) + self.client.force_login(self.user) + with self.assertNumQueries(15): + response = self.client.post( + reverse('api:worker-run-list', kwargs={'pk': str(self.dataimport_2.id)}), + data={'version_id': str(self.version_1.id), 'parents': []}, format='json' + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + def test_runs_post_empty(self): """ Parents and version_id are required to create a worker run @@ -191,17 +202,17 @@ class TestWorkerRuns(FixtureAPITestCase): response = self.client.get(reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_retrieve_run_no_execution_right(self): + def test_retrieve_run_no_worker_execution_right(self): """ - A user cannot retrieve a worker run if he has no execution access on its worker + A user can retrieve any worker run if they have an admin access to the process project """ self.worker_1.memberships.update(level=Role.Guest.value) self.client.force_login(self.user) - response = self.client.get( - reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) - ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertEqual(response.json(), {'detail': 'You do not have an execution access to this worker.'}) + with self.assertNumQueries(6): + response = self.client.get( + reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) def test_retrieve_run_invalid_id(self): self.client.force_login(self.user) @@ -252,17 +263,17 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_update_run_no_execution_right(self): + def test_update_run_no_project_admin_right(self): """ - A user cannot update a worker run if he has no execution access on its worker + A user cannot update a worker run if he has no admin access on its process project """ - self.worker_1.memberships.update(level=Role.Guest.value) + self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value) self.client.force_login(self.user) response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertEqual(response.json(), {'detail': 'You do not have an execution access to this worker.'}) + self.assertEqual(response.json(), {'detail': 'You do not have an admin access to the process project.'}) def test_update_run_invalid_id(self): rev_2 = self.repo.revisions.create( @@ -420,18 +431,17 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_delete_run_no_execution_right(self): + def test_delete_run_no_worker_execution_right(self): """ - A user cannot delete a worker run if he has no execution access on its worker + A user can delete a worker run with no right on its worker but an admin access to the process project """ self.worker_1.memberships.update(level=Role.Guest.value) self.client.force_login(self.user) response = self.client.delete( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertEqual(response.json(), {'detail': 'You do not have an execution access to this worker.'}) - self.assertTrue(WorkerRun.objects.exists()) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertFalse(WorkerRun.objects.exists()) def test_delete_run(self): self.client.force_login(self.user) diff --git a/arkindex/dataimport/tests/test_workers.py b/arkindex/dataimport/tests/test_workers.py index af5810fb6e..b8c4b936a0 100644 --- a/arkindex/dataimport/tests/test_workers.py +++ b/arkindex/dataimport/tests/test_workers.py @@ -219,7 +219,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): with self.assertNumQueries(9): response = self.client.get(reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_1.id)})) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertEqual(response.json(), {'detail': 'You do not have an execution access to this worker.'}) + self.assertEqual(response.json(), {'detail': 'You do not have a guest access to this worker.'}) def test_worker_create_repository_not_uid(self): self.client.force_login(self.user) diff --git a/arkindex/dataimport/tests/test_workflows_api.py b/arkindex/dataimport/tests/test_workflows_api.py index 6fd28c4b3a..9647822dbc 100644 --- a/arkindex/dataimport/tests/test_workflows_api.py +++ b/arkindex/dataimport/tests/test_workflows_api.py @@ -6,6 +6,7 @@ from rest_framework.reverse import reverse from arkindex.dataimport.models import DataImport, DataImportMode, RepositoryType, WorkerVersion from arkindex.documents.models import Corpus, Element from arkindex.project.tests import FixtureAPITestCase +from arkindex.users.models import Role from ponos.models import State, Workflow RECIPE = ''' @@ -140,7 +141,6 @@ class TestWorkflows(FixtureAPITestCase): { 'corpus': str(self.corpus.id), 'element': str(self.pages.first().id), - 'mode': 'elements' }, format='json' ) @@ -150,34 +150,50 @@ class TestWorkflows(FixtureAPITestCase): dataimport = DataImport.objects.get(id=data['id']) self.assertEqual(dataimport.element, self.pages.first()) - def test_element_workflow_requires_login(self): + def test_create_element_workflow_requires_login(self): + response = self.client.post(reverse('api:corpus-workflow'), {}, format='json') + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_create_element_workflow_required_fields(self): + self.client.force_login(self.user) + response = self.client.post(reverse('api:corpus-workflow'), {}, format='json') + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual( + response.json(), + {'corpus': ['This field is required.']} + ) + + def test_create_element_workflow_requires_corpus_admin(self): + """ + Only an admin of the target corpus can create a workers process + """ + self.client.force_login(self.user) + self.corpus.memberships.filter(user=self.user).update(level=Role.Contributor.value) response = self.client.post( reverse('api:corpus-workflow'), { - 'element': str(self.volume.id) + 'corpus': str(self.corpus.id), + 'name': 'Test', }, format='json' ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual(response.json(), {'corpus': ['You do not have an admin access to this corpus.']}) - def test_element_workflow_acl(self): + def test_create_element_workflow_non_readable_corpus(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), { - 'element': str(self.private_volume.id), + 'corpus': str(self.private_corpus.id), + 'name': 'Test', }, format='json' ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - data = response.json() - self.assertIn('element', data) - self.assertListEqual( - data['element'], - ['Invalid pk "{}" - object does not exist.'.format(self.private_volume.id)] - ) + self.assertDictEqual(response.json(), {'corpus': ['Corpus with this ID does not exist.']}) - def test_elements_name_filter(self): + def test_create_element_workflow_name_filter(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -192,7 +208,7 @@ class TestWorkflows(FixtureAPITestCase): dataimport = DataImport.objects.get(id=data['id']) self.assertEqual(dataimport.name_contains, self.pages.first().name[2:5]) - def test_elements_name_filter_max_length(self): + def test_create_element_workflow_name_filter_max_length(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -205,7 +221,7 @@ class TestWorkflows(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.json(), {'name': ['Ensure this field has no more than 250 characters.']}) - def test_elements_type_filter(self): + def test_create_element_workflow_type_filter(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -220,7 +236,7 @@ class TestWorkflows(FixtureAPITestCase): dataimport = DataImport.objects.get(id=data['id']) self.assertEqual(dataimport.element_type.slug, 'page') - def test_elements_empty_queryset(self): + def test_create_element_workflow_empty_queryset(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -239,7 +255,7 @@ class TestWorkflows(FixtureAPITestCase): ['No element match filtering'] ) - def test_elements_empty_selection(self): + def test_create_element_workflow_empty_selection(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -257,7 +273,7 @@ class TestWorkflows(FixtureAPITestCase): ['No element match filtering'] ) - def test_wrong_type(self): + def test_create_element_workflow_wrong_type(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -275,7 +291,10 @@ class TestWorkflows(FixtureAPITestCase): ['Unrecognized element type filter'] ) - def test_mixed_element_filters(self): + def test_create_element_workflow_mixed_element_filters(self): + """ + Element parameter cannot be set with filtering parameters + """ self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -287,33 +306,46 @@ class TestWorkflows(FixtureAPITestCase): format='json' ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - data = response.json() - self.assertIn('__all__', data) - self.assertListEqual( - data['__all__'], - ['Filtering parameters (name, type, best_class) cannot be used with element parameter'] + self.assertDictEqual( + response.json(), + {'__all__': ['Filtering parameters (name, type, best_class) cannot be used with element parameter']} ) - def test_mixed_selection_filters(self,): + def test_create_element_workflow_non_folder_no_zone(self): self.client.force_login(self.user) + element = self.corpus.elements.create(type=self.pages.first().type, name='Kill me please') response = self.client.post( reverse('api:corpus-workflow'), { 'corpus': str(self.corpus.id), - 'element': str(self.pages.first().id), + 'element': str(element.id), + }, + format='json' + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual( + response.json(), + {'element': ['Element has to be a folder or an element with a zone.']} + ) + + def test_create_element_workflow_mixed_selection_filters(self): + self.client.force_login(self.user) + response = self.client.post( + reverse('api:corpus-workflow'), + { + 'corpus': str(self.corpus.id), + 'element': str(self.volume.id), 'selection': True, }, format='json' ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - data = response.json() - self.assertIn('__all__', data) - self.assertListEqual( - data['__all__'], - ['Filtering parameters (element, name, type, best_class) cannot be used with selection parameter'] + self.assertDictEqual( + response.json(), + {'__all__': ['Filtering parameters (element, name, type, best_class) cannot be used with selection parameter']} ) - def test_elements_selection_filter(self): + def test_create_element_workflow_selection_filter(self): self.client.force_login(self.user) page = self.pages.first() self.client.post( @@ -357,7 +389,7 @@ class TestWorkflows(FixtureAPITestCase): self.assertEqual(data['results'][0]['id'], str(page.id)) @override_settings(ARKINDEX_FEATURES={'selection': False}) - def test_elements_selection_no_selection(self): + def test_create_element_workflow_selection_no_selection(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -370,7 +402,7 @@ class TestWorkflows(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.json(), {'selection': ['Selection is not available on this instance.']}) - def test_process_name(self): + def test_create_element_workflow_process_name(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -385,7 +417,7 @@ class TestWorkflows(FixtureAPITestCase): dataimport = DataImport.objects.get(id=data['id']) self.assertEqual(dataimport.name, 'blah') - def test_process_name_max_length(self): + def test_create_element_workflow_process_name_max_length(self): self.client.force_login(self.user) response = self.client.post( reverse('api:corpus-workflow'), @@ -482,7 +514,7 @@ class TestWorkflows(FixtureAPITestCase): self.assertIsNone(dataimport_2.workflow) self.client.force_login(self.user) - with self.assertNumQueries(35): + with self.assertNumQueries(39): response = self.client.post( reverse('api:process-start', kwargs={'pk': str(dataimport_2.id)}) ) diff --git a/arkindex/project/mixins.py b/arkindex/project/mixins.py index 9b93929887..59eafc4e53 100644 --- a/arkindex/project/mixins.py +++ b/arkindex/project/mixins.py @@ -7,7 +7,7 @@ from drf_spectacular.utils import extend_schema, extend_schema_view from rest_framework.exceptions import APIException, ValidationError from rest_framework.serializers import CharField, Serializer -from arkindex.dataimport.models import Repository, Worker +from arkindex.dataimport.models import DataImport, DataImportMode, Repository, Worker from arkindex.documents.models import Corpus from arkindex.documents.serializers.search import SearchQuerySerializer from arkindex.project.elastic import ESQuerySet @@ -156,6 +156,39 @@ class CorpusACLMixin(ACLMixin): return self.has_access(corpus, Role.Admin.value) +class ProcessACLMixin(ACLMixin): + """ + Rights on processes are determined depending on its nature. + The process creator always has a read access. + If the process is attached to a project, the project right apply. + If the process is a version build, the repository right apply. + """ + + @property + def readable_processes(self): + # Return a queryset of accessible process + return DataImport.objects.filter( + Q(creator_id=self.user.id) + | Q(corpus_id__in=filter_rights(self.user, Corpus, Role.Guest.value).values('id')) + | Q(revision__repo_id__in=filter_rights(self.user, Repository, Role.Guest.value).values('id')) + ).distinct() + + def process_access_level(self, process): + # Return the access level on a single process + access_levels = [] + if process.corpus_id: + # Use project right. Covers Images, IIIF, PDF, Repository (IIIF), Elements (Workers) and Transkribus process modes + access_levels.append(get_max_level(self.user, process.corpus)) + elif process.mode == DataImportMode.Repository and process.revision_id: + # Use repository right in case of a workers docker build + access_levels.append(get_max_level(self.user, process.revision.repo)) + if process.creator_id == self.user.id: + # A process is always visible by its creator. This case only occurs when a user loses + # their access to the project or repository after they have created the process. + access_levels.append(Role.Guest.value) + return max(filter(None, access_levels), default=None) + + class SearchAPIMixin(CorpusACLMixin): query_serializer_class = SearchQuerySerializer search = None diff --git a/arkindex/project/tests/test_acl_mixin.py b/arkindex/project/tests/test_acl_mixin.py index 52708a8834..388b6cfb33 100644 --- a/arkindex/project/tests/test_acl_mixin.py +++ b/arkindex/project/tests/test_acl_mixin.py @@ -3,9 +3,9 @@ import uuid from django.contrib.auth.models import AnonymousUser from django.contrib.contenttypes.models import ContentType -from arkindex.dataimport.models import Repository, RepositoryType +from arkindex.dataimport.models import DataImport, DataImportMode, Repository, RepositoryType, Revision from arkindex.documents.models import Corpus -from arkindex.project.mixins import ACLMixin, CorpusACLMixin, RepositoryACLMixin, WorkerACLMixin +from arkindex.project.mixins import ACLMixin, CorpusACLMixin, ProcessACLMixin, RepositoryACLMixin, WorkerACLMixin from arkindex.project.tests import FixtureTestCase from arkindex.users.models import Group, Right, Role, User from arkindex.users.utils import filter_rights, get_max_level @@ -283,3 +283,61 @@ class TestACLMixin(FixtureTestCase): def test_max_level_public(self): with self.assertNumQueries(3): self.assertEqual(get_max_level(self.user1, self.corpus), Role.Guest.value) + + def test_process_access_creator(self): + """ + A process creator always has a read access + """ + process = DataImport.objects.create(mode=DataImportMode.Workers, creator=self.user1, corpus=self.corpus2) + with self.assertNumQueries(3): + self.assertEqual(ProcessACLMixin(user=self.user1).process_access_level(process), Role.Guest.value) + + def test_process_access_project(self): + """ + A project attached to a process defines its access rights + """ + process = DataImport.objects.create(mode=DataImportMode.Workers, creator=self.user2, corpus=self.corpus1) + with self.assertNumQueries(3): + self.assertEqual(ProcessACLMixin(user=self.user1).process_access_level(process), 75) + + def test_process_access_docker_build(self): + """ + Access to a docker build process is possible for someone with an access to the origin repository + """ + process = DataImport.objects.create( + mode=DataImportMode.Repository, + creator=self.user2, + revision=self.repo1.revisions.create(hash='42', message='Message', author='User 1') + ) + with self.assertNumQueries(3): + self.assertEqual(ProcessACLMixin(user=self.user1).process_access_level(process), 80) + + def test_process_access_list(self): + """ + A user can list process either if they are creator, or have rights on its corresponding project or repository + """ + # Readable processes + process_1 = DataImport.objects.create(mode=DataImportMode.PDF, creator=self.user1) + process_2 = DataImport.objects.create(mode=DataImportMode.Workers, creator=self.user2, corpus=self.corpus1) + process_3 = DataImport.objects.create( + mode=DataImportMode.Repository, + creator=self.user2, + revision=self.repo1.revisions.create(hash='42', message='Message', author='User 1') + ) + # Non readable processes + DataImport.objects.create(mode=DataImportMode.Workers, creator=self.user2) + DataImport.objects.create(mode=DataImportMode.Workers, creator=self.user2, corpus=self.corpus2) + DataImport.objects.create( + mode=DataImportMode.Repository, + creator=self.user2, + revision=Revision.objects.get(message='My w0rk3r') + ) + + with self.assertNumQueries(4): + readable_process_ids = list( + ProcessACLMixin(user=self.user1).readable_processes.values_list('id', flat=True) + ) + self.assertCountEqual( + readable_process_ids, + [process_1.id, process_2.id, process_3.id] + ) diff --git a/arkindex/project/tests/test_ponos_view.py b/arkindex/project/tests/test_ponos_view.py index 3580b93057..e97ae28c29 100644 --- a/arkindex/project/tests/test_ponos_view.py +++ b/arkindex/project/tests/test_ponos_view.py @@ -10,7 +10,7 @@ from arkindex.documents.models import Corpus from arkindex.project.tests import FixtureAPITestCase from arkindex.users.models import Role, User from ponos.authentication import AgentUser -from ponos.models import Agent, Artifact, Farm, Secret, encrypt +from ponos.models import Agent, Artifact, Farm, Secret, State, encrypt @override_settings(PONOS_PRIVATE_KEY='staging') @@ -165,3 +165,41 @@ class TestPonosView(FixtureAPITestCase): HTTP_AUTHORIZATION="Bearer {}".format(agent_user.token.access_token), ) self.assertEqual(response.status_code, status.HTTP_302_FOUND) + + def test_retrieve_workflow(self): + """ + A user with a guest right on a process cannot retrieve its workflow + """ + self.client.force_login(self.user) + self.dataimport_corpus.memberships.create(user=self.user, level=Role.Guest.value) + with self.assertNumQueries(12): + 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): + """ + A user cannot retry a workflow if they have no admin access to their process + """ + self.task.state = State.Running + self.task.save() + self.client.force_login(self.user) + self.dataimport_corpus.memberships.create(user=self.user, level=Role.Guest.value) + with self.assertNumQueries(7): + response = self.client.patch( + reverse('ponos-workflow-details', kwargs={'pk': str(self.workflow.id)}), + {'status': 'stopped'} + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.json(), {'detail': 'You do not have an admin access to this process.'}) + + def test_retry_workflow(self): + self.task.state = State.Running + self.task.save() + self.client.force_login(self.user) + self.dataimport_corpus.memberships.create(user=self.user, level=Role.Admin.value) + with self.assertNumQueries(17): + response = self.client.patch( + 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 220771de82..1fd7321057 100644 --- a/arkindex/project/urls.py +++ b/arkindex/project/urls.py @@ -13,6 +13,7 @@ from arkindex.project.views import ( PonosArtifactDownload, PonosSecretDetails, PonosTaskUpdate, + PonosWorkflowDetails, ) # Fallback to the dummy frontend view when CDN_ASSETS_URL is not set @@ -31,6 +32,7 @@ urlpatterns = [ 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('admin/', admin.site.urls), path('rq/', include('django_rq.urls')), diff --git a/arkindex/project/views.py b/arkindex/project/views.py index 833268b32d..660f9cb208 100644 --- a/arkindex/project/views.py +++ b/arkindex/project/views.py @@ -1,11 +1,16 @@ 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.dataimport.models import DataImport from arkindex.dataimport.permissions import IsArtifactAdminOrCreator, IsTaskAdminOrCreator -from arkindex.project.mixins import CachedViewMixin +from arkindex.project.mixins import CachedViewMixin, ProcessACLMixin from arkindex.project.permissions import IsVerified -from ponos.api import AgentDetails, AgentsState, SecretDetails, TaskArtifactDownload, TaskUpdate +from arkindex.users.models import Role +from ponos.api import AgentDetails, AgentsState, SecretDetails, TaskArtifactDownload, TaskUpdate, WorkflowDetails class FrontendView(View): @@ -81,3 +86,32 @@ class PonosArtifactDownload(TaskArtifactDownload): Allow users with an admin privilege and creators to download an artifact of a dataimport task """ permission_classes = (IsArtifactAdminOrCreator, ) + + +@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( + DataImport, + 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.') -- GitLab