From 48aad4f356b92777aed8c6b851f0df1326be5a66 Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Sat, 2 Mar 2019 01:49:30 -0400 Subject: [PATCH 1/3] Add mixin to provide temporary test models Signed-off-by: Roberto Rosario --- mayan/apps/common/tests/mixins.py | 50 +++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/mayan/apps/common/tests/mixins.py b/mayan/apps/common/tests/mixins.py index ad3764ecd7..b17166d5a7 100644 --- a/mayan/apps/common/tests/mixins.py +++ b/mayan/apps/common/tests/mixins.py @@ -6,14 +6,17 @@ import random from furl import furl +from django.apps import apps from django.conf import settings from django.conf.urls import url from django.core import management +from django.db import connection from django.db import models from django.http import HttpResponse from django.template import Context, Template from django.test.utils import ContextList from django.urls import clear_url_caches, reverse +from django.utils.encoding import force_bytes from mayan.apps.storage.settings import setting_temporary_directory @@ -229,6 +232,53 @@ class TempfileCheckTestCaseMixin(object): super(TempfileCheckTestCaseMixin, self).tearDown() +class TestModelTestMixin(object): + _test_models = [] + + def _create_test_model(self, fields=None, model_name='TestModel', options=None): + class Meta: + pass + + if options is not None: + for key, value in options.items(): + setattr(Meta, key, value) + + attrs = {'__module__': self.__class__.__module__, 'Meta': Meta} + + if fields: + attrs.update(fields) + + TestModel = type( + force_bytes(model_name), (models.Model,), attrs + ) + + setattr(self, model_name, TestModel) + + with connection.schema_editor() as schema_editor: + schema_editor.create_model(model=TestModel) + + self.__class__._test_models.append(model_name) + + def _delete_test_model(self, model_name='TestModel'): + TestModel = getattr(self, model_name) + + with connection.schema_editor() as schema_editor: + schema_editor.delete_model(model=TestModel) + + del TestModel._meta.app_config.models[model_name.lower()] + apps.clear_cache() + + def _delete_test_models(self): + for test_model in self.__class__._test_models: + self._delete_test_model(model_name=test_model) + + self.__class__._test_models = [] + + def tearDown(self): + self._delete_test_models() + super(TestModelTestMixin, self).tearDown() + + class TestViewTestCaseMixin(object): has_test_view = False From 0cbd9e0d45a169d6e7676bc67014801887f09403 Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Sat, 2 Mar 2019 01:51:23 -0400 Subject: [PATCH 2/3] ACLs: Make get_inherited_permissions recursive Update .get_inherited_permissions() to grab the permissions of an object up the parent tree. Also add the role permissions. Finally filter all the permissions by those that apply to the object. Signed-off-by: Roberto Rosario --- mayan/apps/acls/classes.py | 14 +-- mayan/apps/acls/managers.py | 49 +++++---- mayan/apps/acls/tests/mixins.py | 9 +- mayan/apps/acls/tests/test_models.py | 145 +++++++++++++++++++++++++++ 4 files changed, 183 insertions(+), 34 deletions(-) diff --git a/mayan/apps/acls/classes.py b/mayan/apps/acls/classes.py index 94ee4c6f69..c95ab4e562 100644 --- a/mayan/apps/acls/classes.py +++ b/mayan/apps/acls/classes.py @@ -40,20 +40,10 @@ class ModelPermission(object): app_label='permissions', model_name='StoredPermission' ) - permissions = [] - - class_permissions = cls.get_for_class(klass=type(instance)) - - if class_permissions: - permissions.extend(class_permissions) - - proxy = cls._proxies.get(type(instance)) - - if proxy: - permissions.extend(cls._registry.get(proxy)) + permissions = cls.get_for_class(klass=type(instance)) pks = [ - permission.stored_permission.pk for permission in set(permissions) + permission.stored_permission.pk for permission in permissions ] return StoredPermission.objects.filter(pk__in=pks) diff --git a/mayan/apps/acls/managers.py b/mayan/apps/acls/managers.py index d113be9254..64bb239dd3 100644 --- a/mayan/apps/acls/managers.py +++ b/mayan/apps/acls/managers.py @@ -40,9 +40,9 @@ class AccessControlListManager(models.Manager): # 2: Related field # 3: Related field that is Generic Foreign Key # 4: No related field, but has an inherited related field, solved by - # 5: Inherited field of a related field # recursion, branches to #2 or #3. - # Not addressed yet + # 5: Inherited field of a related field + # -- Not addressed yet -- # 6: Inherited field of a related field that is Generic Foreign Key result = [] @@ -152,41 +152,56 @@ class AccessControlListManager(models.Manager): raise PermissionDenied def get_inherited_permissions(self, obj, role): - try: - instance = obj.first() - except AttributeError: - instance = obj - else: - if not instance: - return StoredPermission.objects.none() + queryset = self._get_inherited_object_permissions(obj=obj, role=role) + + queryset = queryset | role.permissions.all() + + # Filter the permissions to the ones that apply to the model + queryset = ModelPermission.get_for_instance( + instance=obj + ).filter( + pk__in=queryset + ) + + return queryset + + def _get_inherited_object_permissions(self, obj, role): + queryset = StoredPermission.objects.none() + + if not obj: + return queryset try: parent_accessor = ModelPermission.get_inheritance( - model=type(instance) + model=type(obj) ) except KeyError: - return StoredPermission.objects.none() + pass else: try: parent_object = resolve_attribute( - obj=instance, attribute=parent_accessor + obj=obj, attribute=parent_accessor ) except AttributeError: # Parent accessor is not an attribute, try it as a related # field. parent_object = return_related( - instance=instance, related_field=parent_accessor + instance=obj, related_field=parent_accessor ) content_type = ContentType.objects.get_for_model(model=parent_object) try: - return self.get( + queryset = queryset | self.get( content_type=content_type, object_id=parent_object.pk, role=role ).permissions.all() except self.model.DoesNotExist: - return StoredPermission.objects.none() + pass - def grant(self, permission, role, obj): + queryset = queryset | self._get_inherited_object_permissions(obj=parent_object, role=role) + + return queryset + + def grant(self, obj, permission, role): class_permissions = ModelPermission.get_for_class(klass=obj.__class__) if permission not in class_permissions: raise PermissionNotValidForClass @@ -224,7 +239,7 @@ class AccessControlListManager(models.Manager): # is staff. Return the entire queryset. return queryset - def revoke(self, permission, role, obj): + def revoke(self, obj, permission, role): content_type = ContentType.objects.get_for_model(model=obj) acl, created = self.get_or_create( content_type=content_type, object_id=obj.pk, diff --git a/mayan/apps/acls/tests/mixins.py b/mayan/apps/acls/tests/mixins.py index 7ea4237294..1f7186b4bd 100644 --- a/mayan/apps/acls/tests/mixins.py +++ b/mayan/apps/acls/tests/mixins.py @@ -40,12 +40,11 @@ class ACLTestMixin(object): if self.auto_create_test_role: self._create_test_role() - self.test_object = self.document - - content_type = ContentType.objects.get_for_model(self.test_object) + def _inject_test_object_content_type(self): + self.test_object_content_type = ContentType.objects.get_for_model(self.test_object) self.test_content_object_view_kwargs = { - 'app_label': content_type.app_label, - 'model': content_type.model, + 'app_label': self.test_object_content_type.app_label, + 'model_name': self.test_object_content_type.model, 'object_id': self.test_object.pk } diff --git a/mayan/apps/acls/tests/test_models.py b/mayan/apps/acls/tests/test_models.py index e7657094a1..ab2c8e144b 100644 --- a/mayan/apps/acls/tests/test_models.py +++ b/mayan/apps/acls/tests/test_models.py @@ -1,16 +1,22 @@ from __future__ import absolute_import, unicode_literals from django.core.exceptions import PermissionDenied +from django.db import models from mayan.apps.common.tests import BaseTestCase +from mayan.apps.common.tests.mixins import TestModelTestMixin from mayan.apps.documents.models import Document, DocumentType from mayan.apps.documents.permissions import permission_document_view from mayan.apps.documents.tests import ( DocumentTestMixin, TEST_DOCUMENT_TYPE_2_LABEL, TEST_DOCUMENT_TYPE_LABEL ) +from mayan.apps.permissions.tests.mixins import PermissionTestMixin, RoleTestMixin +from ..classes import ModelPermission from ..models import AccessControlList +from .mixins import ACLTestMixin + class PermissionTestCase(DocumentTestMixin, BaseTestCase): auto_create_document_type = False @@ -150,3 +156,142 @@ class PermissionTestCase(DocumentTestMixin, BaseTestCase): self.assertTrue(self.test_document_1 in result) self.assertTrue(self.test_document_2 in result) self.assertTrue(self.test_document_3 in result) + + +class InheritedPermissionTestCase(TestModelTestMixin, PermissionTestMixin, RoleTestMixin, ACLTestMixin, BaseTestCase): + def test_retrieve_inherited_role_permission_not_model_applicable(self): + self._create_test_model() + self.test_object = self.TestModel.objects.create() + self._create_test_acl() + self._create_test_permission() + + self.test_role.grant(permission=self.test_permission) + + queryset = AccessControlList.objects.get_inherited_permissions( + obj=self.test_object, role=self.test_role + ) + self.assertTrue(self.test_permission.stored_permission not in queryset) + + def test_retrieve_inherited_role_permission_model_applicable(self): + self._create_test_model() + self.test_object = self.TestModel.objects.create() + self._create_test_acl() + self._create_test_permission() + + ModelPermission.register( + model=self.test_object._meta.model, permissions=( + self.test_permission, + ) + ) + self.test_role.grant(permission=self.test_permission) + + queryset = AccessControlList.objects.get_inherited_permissions( + obj=self.test_object, role=self.test_role + ) + self.assertTrue(self.test_permission.stored_permission in queryset) + + def test_retrieve_inherited_related_parent_child_permission(self): + self._create_test_permission() + + self._create_test_model(model_name='TestModelParent') + self._create_test_model( + fields={ + 'parent': models.ForeignKey( + on_delete=models.CASCADE, related_name='children', + to='TestModelParent', + ) + }, model_name='TestModelChild' + ) + + ModelPermission.register( + model=self.TestModelParent, permissions=( + self.test_permission, + ) + ) + ModelPermission.register( + model=self.TestModelChild, permissions=( + self.test_permission, + ) + ) + ModelPermission.register_inheritance( + model=self.TestModelChild, related='parent', + ) + + parent = self.TestModelParent.objects.create() + child = self.TestModelChild.objects.create(parent=parent) + + AccessControlList.objects.grant( + obj=parent, permission=self.test_permission, role=self.test_role + ) + + queryset = AccessControlList.objects.get_inherited_permissions( + obj=child, role=self.test_role + ) + + self.assertTrue(self.test_permission.stored_permission in queryset) + + #self._delete_test_model(model_name='TestModelParent') + #elf._delete_test_model(model_name='TestModelChild') + + def test_retrieve_inherited_related_grandparent_parent_child_permission(self): + self._create_test_permission() + + self._create_test_model(model_name='TestModelGrandParent') + self._create_test_model( + fields={ + 'parent': models.ForeignKey( + on_delete=models.CASCADE, related_name='children', + to='TestModelGrandParent', + ) + }, model_name='TestModelParent' + ) + self._create_test_model( + fields={ + 'parent': models.ForeignKey( + on_delete=models.CASCADE, related_name='children', + to='TestModelParent', + ) + }, model_name='TestModelChild' + ) + + ModelPermission.register( + model=self.TestModelGrandParent, permissions=( + self.test_permission, + ) + ) + ModelPermission.register( + model=self.TestModelParent, permissions=( + self.test_permission, + ) + ) + ModelPermission.register( + model=self.TestModelChild, permissions=( + self.test_permission, + ) + ) + + ModelPermission.register_inheritance( + model=self.TestModelChild, related='parent', + ) + ModelPermission.register_inheritance( + model=self.TestModelParent, related='parent', + ) + + grandparent = self.TestModelGrandParent.objects.create() + parent = self.TestModelParent.objects.create(parent=grandparent) + child = self.TestModelChild.objects.create(parent=parent) + + AccessControlList.objects.grant( + obj=grandparent, permission=self.test_permission, + role=self.test_role + ) + + queryset = AccessControlList.objects.get_inherited_permissions( + obj=child, role=self.test_role + ) + + self.assertTrue(self.test_permission.stored_permission in queryset) + + #self._delete_test_model(model_name='TestModelGrandParent') + #self._delete_test_model(model_name='TestModelParent') + #self._delete_test_model(model_name='TestModelChild') From 2a67cf271ed19dc928f72f900c1b1c8e0b36d0fe Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Sat, 2 Mar 2019 16:03:29 -0400 Subject: [PATCH 3/3] Refactor ACL app API Signed-off-by: Roberto Rosario --- mayan/apps/acls/api_views.py | 288 ++++++++++--------------- mayan/apps/acls/serializers.py | 300 +++++++++++---------------- mayan/apps/acls/tests/test_api.py | 125 ++++++----- mayan/apps/acls/tests/test_models.py | 8 - mayan/apps/acls/urls.py | 14 +- mayan/apps/acls/views.py | 22 +- mayan/apps/common/tests/mixins.py | 5 + 7 files changed, 324 insertions(+), 438 deletions(-) diff --git a/mayan/apps/acls/api_views.py b/mayan/apps/acls/api_views.py index 989d147717..ad0754bb82 100644 --- a/mayan/apps/acls/api_views.py +++ b/mayan/apps/acls/api_views.py @@ -3,201 +3,125 @@ from __future__ import absolute_import, unicode_literals from django.contrib.contenttypes.models import ContentType from django.shortcuts import get_object_or_404 -from rest_framework import generics +from rest_framework import generics, status, viewsets +from rest_framework.decorators import action +from rest_framework.response import Response + +from mayan.apps.common.mixins import ContentTypeViewMixin, ExternalObjectMixin +from mayan.apps.rest_api.viewsets import ( + MayanAPIGenericViewSet, MayanAPIModelViewSet, MayanAPIReadOnlyModelViewSet +) +from mayan.apps.permissions.serializers import ( + PermissionSerializer, RolePermissionAddRemoveSerializer +) from .models import AccessControlList from .permissions import permission_acl_edit, permission_acl_view -from .serializers import ( - AccessControlListPermissionSerializer, AccessControlListSerializer, - WritableAccessControlListPermissionSerializer, - WritableAccessControlListSerializer -) +from .serializers import AccessControlListSerializer -class APIObjectACLListView(generics.ListCreateAPIView): - """ - get: Returns a list of all the object's access control lists - post: Create a new access control list for the selected object. - """ - def get_content_object(self): - content_type = get_object_or_404( - klass=ContentType, app_label=self.kwargs['app_label'], - model=self.kwargs['model'] - ) - - content_object = get_object_or_404( - klass=content_type.model_class(), pk=self.kwargs['object_id'] - ) - - if self.request.method == 'GET': - permission_required = permission_acl_view - else: - permission_required = permission_acl_edit - - AccessControlList.objects.check_access( - permissions=permission_required, user=self.request.user, - obj=content_object - ) - - return content_object - - def get_queryset(self): - return self.get_content_object().acls.all() - - def get_serializer_context(self): - """ - Extra context provided to the serializer class. - """ - context = super(APIObjectACLListView, self).get_serializer_context() - if self.kwargs: - context.update( - { - 'content_object': self.get_content_object(), - } - ) - - return context - - def get_serializer(self, *args, **kwargs): - if not self.request: - return None - - return super(APIObjectACLListView, self).get_serializer(*args, **kwargs) - - def get_serializer_class(self): - if self.request.method == 'GET': - return AccessControlListSerializer - else: - return WritableAccessControlListSerializer - - -class APIObjectACLView(generics.RetrieveDestroyAPIView): - """ - delete: Delete the selected access control list. - get: Returns the details of the selected access control list. - """ +class ObjectACLAPIViewSet(ContentTypeViewMixin, ExternalObjectMixin, MayanAPIModelViewSet): + content_type_url_kw_args = { + 'app_label': 'app_label', + 'model': 'model_name' + } + external_object_pk_url_kwarg = 'object_id' + lookup_url_kwarg = 'acl_id' serializer_class = AccessControlListSerializer - def get_content_object(self): - if self.request.method == 'GET': - permission_required = permission_acl_view - else: - permission_required = permission_acl_edit - - content_type = get_object_or_404( - klass=ContentType, app_label=self.kwargs['app_label'], - model=self.kwargs['model'] + def create(self, request, *args, **kwargs): + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + serializer.validated_data.update( + { + 'object_id': self.external_object.pk, + 'content_type': self.get_content_type(), + } ) + self.perform_create(serializer) + headers = self.get_success_headers(serializer.data) + return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) - content_object = get_object_or_404( - klass=content_type.model_class(), pk=self.kwargs['object_id'] - ) - - AccessControlList.objects.check_access( - permissions=permission_required, user=self.request.user, - obj=content_object - ) - - return content_object - - def get_queryset(self): - return self.get_content_object().acls.all() - - -class APIObjectACLPermissionListView(generics.ListCreateAPIView): - """ - get: Returns the access control list permission list. - post: Add a new permission to the selected access control list. - """ - def get_acl(self): - return get_object_or_404( - klass=self.get_content_object().acls, pk=self.kwargs['acl_pk'] - ) - - def get_content_object(self): - content_type = get_object_or_404( - klass=ContentType, app_label=self.kwargs['app_label'], - model=self.kwargs['model'] - ) - - content_object = get_object_or_404( - klass=content_type.model_class(), pk=self.kwargs['object_id'] - ) - - AccessControlList.objects.check_access( - permissions=permission_acl_view, user=self.request.user, - obj=content_object - ) - - return content_object - - def get_queryset(self): - return self.get_acl().permissions.all() - - def get_serializer(self, *args, **kwargs): - if not self.request: + def get_external_object_permission(self): + action = getattr(self, 'action', None) + if action is None: return None - - return super(APIObjectACLPermissionListView, self).get_serializer(*args, **kwargs) - - def get_serializer_class(self): - if self.request.method == 'GET': - return AccessControlListPermissionSerializer + elif action in ['list', 'retrieve', 'permission_list', 'permission_inherited_list']: + return permission_acl_view else: - return WritableAccessControlListPermissionSerializer + return permission_acl_edit - def get_serializer_context(self): - context = super(APIObjectACLPermissionListView, self).get_serializer_context() - if self.kwargs: - context.update( - { - 'acl': self.get_acl(), - } - ) - - return context - - -class APIObjectACLPermissionView(generics.RetrieveDestroyAPIView): - """ - delete: Remove the permission from the selected access control list. - get: Returns the details of the selected access control list permission. - """ - lookup_url_kwarg = 'permission_pk' - serializer_class = AccessControlListPermissionSerializer - - def get_acl(self): - return get_object_or_404( - klass=self.get_content_object().acls, pk=self.kwargs['acl_pk'] - ) - - def get_content_object(self): - content_type = get_object_or_404( - klass=ContentType, app_label=self.kwargs['app_label'], - model=self.kwargs['model'] - ) - - content_object = get_object_or_404( - klass=content_type.model_class(), pk=self.kwargs['object_id'] - ) - - AccessControlList.objects.check_access( - permissions=permission_acl_view, user=self.request.user, - obj=content_object - ) - - return content_object + def get_external_object_queryset(self): + # Here we get a queryset the object model for which the event + # will be accessed. + return self.get_content_type().get_all_objects_for_this_type() def get_queryset(self): - return self.get_acl().permissions.all() + obj = self.get_external_object() + return obj.acls.all() - def get_serializer_context(self): - context = super(APIObjectACLPermissionView, self).get_serializer_context() - if self.kwargs: - context.update( - { - 'acl': self.get_acl(), - } - ) + @action( + detail=True, lookup_url_kwarg='acl_id', methods=('post',), + serializer_class=RolePermissionAddRemoveSerializer, + url_name='permission-add', url_path='permissions/add' + ) + def permission_add(self, request, *args, **kwargs): + instance = self.get_object() + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + serializer.permissions_add(instance=instance) + headers = self.get_success_headers(data=serializer.data) + return Response( + serializer.data, headers=headers, status=status.HTTP_200_OK + ) - return context + @action( + detail=True, lookup_url_kwarg='acl_id', + serializer_class=PermissionSerializer, url_name='permission-list', + url_path='permissions' + ) + def permission_list(self, request, *args, **kwargs): + queryset = self.get_object().permissions.all() + page = self.paginate_queryset(queryset) + + serializer = self.get_serializer( + queryset, many=True, context={'request': request} + ) + + if page is not None: + return self.get_paginated_response(serializer.data) + + return Response(serializer.data) + + @action( + detail=True, lookup_url_kwarg='acl_id', + serializer_class=PermissionSerializer, + url_name='permission-inherited-list', url_path='permissions/inherited' + ) + def permission_inherited_list(self, request, *args, **kwargs): + queryset = self.get_object().get_inherited_permissions() + page = self.paginate_queryset(queryset) + + serializer = self.get_serializer( + queryset, many=True, context={'request': request} + ) + + if page is not None: + return self.get_paginated_response(serializer.data) + + return Response(serializer.data) + + @action( + detail=True, lookup_url_kwarg='acl_id', + methods=('post',), serializer_class=RolePermissionAddRemoveSerializer, + url_name='permission-remove', url_path='permissions/remove' + ) + def permission_remove(self, request, *args, **kwargs): + instance = self.get_object() + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + serializer.permissions_remove(instance=instance) + headers = self.get_success_headers(data=serializer.data) + return Response( + serializer.data, headers=headers, status=status.HTTP_200_OK + ) diff --git a/mayan/apps/acls/serializers.py b/mayan/apps/acls/serializers.py index 5a8a02157b..116d1c6151 100644 --- a/mayan/apps/acls/serializers.py +++ b/mayan/apps/acls/serializers.py @@ -12,205 +12,143 @@ from rest_framework.reverse import reverse from mayan.apps.common.serializers import ContentTypeSerializer from mayan.apps.permissions import Permission from mayan.apps.permissions.models import Role, StoredPermission +from mayan.apps.permissions.permissions import permission_role_edit from mayan.apps.permissions.serializers import ( PermissionSerializer, RoleSerializer ) +from mayan.apps.rest_api.mixins import ( + ExternalObjectListSerializerMixin, ExternalObjectSerializerMixin +) +from mayan.apps.rest_api.relations import MultiKwargHyperlinkedIdentityField from .models import AccessControlList -class AccessControlListSerializer(serializers.ModelSerializer): +#TODO: Inherited permissions +class AccessControlListSerializer(ExternalObjectSerializerMixin, serializers.ModelSerializer): content_type = ContentTypeSerializer(read_only=True) - permissions_url = serializers.SerializerMethodField( - help_text=_( - 'API URL pointing to the list of permissions for this access ' - 'control list.' - ) - ) role = RoleSerializer(read_only=True) - url = serializers.SerializerMethodField() + permission_add_url = MultiKwargHyperlinkedIdentityField( + view_kwargs=( + { + 'lookup_field': 'content_type__app_label', 'lookup_url_kwarg': 'app_label', + }, + { + 'lookup_field': 'content_type__model', 'lookup_url_kwarg': 'model_name', + }, + { + 'lookup_field': 'object_id', 'lookup_url_kwarg': 'object_id', + }, + { + 'lookup_field': 'pk', 'lookup_url_kwarg': 'acl_id', + } + ), + view_name='rest_api:object-acl-permission-add' + ) + permission_list_url = MultiKwargHyperlinkedIdentityField( + view_kwargs=( + { + 'lookup_field': 'content_type__app_label', 'lookup_url_kwarg': 'app_label', + }, + { + 'lookup_field': 'content_type__model', 'lookup_url_kwarg': 'model_name', + }, + { + 'lookup_field': 'object_id', 'lookup_url_kwarg': 'object_id', + }, + { + 'lookup_field': 'pk', 'lookup_url_kwarg': 'acl_id', + } + ), + view_name='rest_api:object-acl-permission-list' + ) + permission_list_inherited_url = MultiKwargHyperlinkedIdentityField( + view_kwargs=( + { + 'lookup_field': 'content_type__app_label', 'lookup_url_kwarg': 'app_label', + }, + { + 'lookup_field': 'content_type__model', 'lookup_url_kwarg': 'model_name', + }, + { + 'lookup_field': 'object_id', 'lookup_url_kwarg': 'object_id', + }, + { + 'lookup_field': 'pk', 'lookup_url_kwarg': 'acl_id', + } + ), + view_name='rest_api:object-acl-permission-inherited-list' + ) + permission_remove_url = MultiKwargHyperlinkedIdentityField( + view_kwargs=( + { + 'lookup_field': 'content_type__app_label', 'lookup_url_kwarg': 'app_label', + }, + { + 'lookup_field': 'content_type__model', 'lookup_url_kwarg': 'model_name', + }, + { + 'lookup_field': 'object_id', 'lookup_url_kwarg': 'object_id', + }, + { + 'lookup_field': 'pk', 'lookup_url_kwarg': 'acl_id', + } + ), + view_name='rest_api:object-acl-permission-remove' + ) + role_id = serializers.CharField( + label=_('Role ID'), + help_text=_( + 'Primary key of the role of the ACL that will be created or edited.' + ), required=False, write_only=True + ) + url = MultiKwargHyperlinkedIdentityField( + view_kwargs=( + { + 'lookup_field': 'content_type__app_label', 'lookup_url_kwarg': 'app_label', + }, + { + 'lookup_field': 'content_type__model', 'lookup_url_kwarg': 'model_name', + }, + { + 'lookup_field': 'object_id', 'lookup_url_kwarg': 'object_id', + }, + { + 'lookup_field': 'pk', 'lookup_url_kwarg': 'acl_id', + } + ), + view_name='rest_api:object-acl-detail' + ) class Meta: + external_object_model = Role + external_object_pk_field = 'role_id' + external_object_permission = permission_role_edit fields = ( - 'content_type', 'id', 'object_id', 'permissions_url', 'role', 'url' + 'content_type', 'id', 'object_id', 'permission_add_url', + 'permission_list_url', 'permission_list_inherited_url', + 'permission_remove_url', 'role', 'role_id', + 'url' ) model = AccessControlList - - def get_permissions_url(self, instance): - return reverse( - viewname='rest_api:accesscontrollist-permission-list', kwargs={ - 'app_label': instance.content_type.app_label, - 'model': instance.content_type.model, - 'object_id': instance.object_id, - 'acl_pk': instance.pk - }, request=self.context['request'], format=self.context['format'] - ) - - def get_url(self, instance): - return reverse( - 'rest_api:accesscontrollist-detail', kwargs={ - 'app_label': instance.content_type.app_label, - 'model': instance.content_type.model, - 'object_id': instance.object_id, - 'acl_pk': instance.pk - }, request=self.context['request'], format=self.context['format'] - ) - - -class AccessControlListPermissionSerializer(PermissionSerializer): - acl_permission_url = serializers.SerializerMethodField( - help_text=_( - 'API URL pointing to a permission in relation to the ' - 'access control list to which it is attached. This URL is ' - 'different than the canonical workflow URL.' - ) - ) - acl_url = serializers.SerializerMethodField() - - def get_acl_permission_url(self, instance): - return reverse( - 'rest_api:accesscontrollist-permission-detail', kwargs={ - 'app_label': self.context['acl'].content_type.app_label, - 'model': self.context['acl'].content_type.model, - 'object_id': self.context['acl'].object_id, - 'acl_pk': self.context['acl'].pk, - 'permission_pk': instance.stored_permission.pk - }, request=self.context['request'], format=self.context['format'] - ) - - def get_acl_url(self, instance): - return reverse( - 'rest_api:accesscontrollist-detail', kwargs={ - 'app_label': self.context['acl'].content_type.app_label, - 'model': self.context['acl'].content_type.model, - 'object_id': self.context['acl'].object_id, - 'acl_pk': self.context['acl'].pk - }, request=self.context['request'], format=self.context['format'] - ) - - -class WritableAccessControlListPermissionSerializer(AccessControlListPermissionSerializer): - permission_pk = serializers.CharField( - help_text=_( - 'Primary key of the new permission to grant to the access control ' - 'list.' - ), write_only=True - ) - - class Meta: - fields = ('namespace',) - read_only_fields = ('namespace',) + read_only_fields = ('object_id',) def create(self, validated_data): - for permission in validated_data['permissions']: - self.context['acl'].permissions.add(permission) + role = self.get_external_object() - return validated_data['permissions'][0] + if role: + validated_data['role'] = role - def validate(self, attrs): - permissions_pk_list = attrs.pop('permission_pk', None) - permissions_result = [] - - if permissions_pk_list: - for pk in permissions_pk_list.split(','): - try: - permission = Permission.get(pk=pk) - except KeyError: - raise ValidationError(_('No such permission: %s') % pk) - else: - # Accumulate valid stored permission pks - permissions_result.append(permission.pk) - - attrs['permissions'] = StoredPermission.objects.filter( - pk__in=permissions_result - ) - return attrs - - -class WritableAccessControlListSerializer(serializers.ModelSerializer): - content_type = ContentTypeSerializer(read_only=True) - permissions_pk_list = serializers.CharField( - help_text=_( - 'Comma separated list of permission primary keys to grant to this ' - 'access control list.' - ), required=False - ) - permissions_url = serializers.SerializerMethodField( - help_text=_( - 'API URL pointing to the list of permissions for this access ' - 'control list.' - ), read_only=True - ) - role_pk = serializers.IntegerField( - help_text=_( - 'Primary keys of the role to which this access control list ' - 'binds to.' - ), write_only=True - ) - url = serializers.SerializerMethodField() - - class Meta: - fields = ( - 'content_type', 'id', 'object_id', 'permissions_pk_list', - 'permissions_url', 'role_pk', 'url' - ) - model = AccessControlList - read_only_fields = ('content_type', 'object_id') - - def get_permissions_url(self, instance): - return reverse( - 'rest_api:accesscontrollist-permission-list', kwargs={ - 'app_label': instance.content_type.app_label, - 'model': instance.content_type.model, - 'object_id': instance.object_id, - 'acl_pk': instance.pk - }, request=self.context['request'], format=self.context['format'] + return super(AccessControlListSerializer, self).create( + validated_data=validated_data ) - def get_url(self, instance): - return reverse( - 'rest_api:accesscontrollist-detail', kwargs={ - 'app_label': instance.content_type.app_label, - 'model': instance.content_type.model, - 'object_id': instance.object_id, - 'acl_pk': instance.pk - }, request=self.context['request'], format=self.context['format'] + def update(self, instance, validated_data): + role = self.get_external_object() + + if role: + validated_data['role'] = role + + return super(AccessControlListSerializer, self).update( + instance=instance, validated_data=validated_data ) - - def validate(self, attrs): - attrs['content_type'] = ContentType.objects.get_for_model( - self.context['content_object'] - ) - attrs['object_id'] = self.context['content_object'].pk - - try: - attrs['role'] = Role.objects.get(pk=attrs.pop('role_pk')) - except Role.DoesNotExist as exception: - raise ValidationError(force_text(exception)) - - permissions_pk_list = attrs.pop('permissions_pk_list', None) - permissions_result = [] - - if permissions_pk_list: - for pk in permissions_pk_list.split(','): - try: - permission = Permission.get(pk=pk) - except KeyError: - raise ValidationError(_('No such permission: %s') % pk) - else: - # Accumulate valid stored permission pks - permissions_result.append(permission.pk) - - instance = AccessControlList(**attrs) - - try: - instance.full_clean() - except DjangoValidationError as exception: - raise ValidationError(exception) - - # Add a queryset of valid stored permissions so that they get added - # after the ACL gets created. - attrs['permissions'] = StoredPermission.objects.filter( - pk__in=permissions_result - ) - return attrs diff --git a/mayan/apps/acls/tests/test_api.py b/mayan/apps/acls/tests/test_api.py index 55433b2fb3..8768d3e69e 100644 --- a/mayan/apps/acls/tests/test_api.py +++ b/mayan/apps/acls/tests/test_api.py @@ -4,72 +4,93 @@ from django.contrib.contenttypes.models import ContentType from rest_framework import status +from mayan.apps.common.tests.mixins import TestModelTestMixin from mayan.apps.documents.permissions import permission_document_view from mayan.apps.documents.tests import DocumentTestMixin from mayan.apps.permissions.tests.literals import TEST_ROLE_LABEL +from mayan.apps.permissions.tests.mixins import PermissionTestMixin, RoleTestMixin from mayan.apps.rest_api.tests import BaseAPITestCase +from ..classes import ModelPermission from ..models import AccessControlList -from ..permissions import permission_acl_view +from ..permissions import permission_acl_edit, permission_acl_view + +from .mixins import ACLTestMixin -class ACLAPITestCase(DocumentTestMixin, BaseAPITestCase): +class ACLAPITestCase(ACLTestMixin, RoleTestMixin, PermissionTestMixin, TestModelTestMixin, BaseAPITestCase): def setUp(self): super(ACLAPITestCase, self).setUp() - self.login_admin_user() - self.document_content_type = ContentType.objects.get_for_model( - self.document + self._create_test_model() + self._create_test_object() + self._create_test_acl() + ModelPermission.register( + model=self.test_object._meta.model, permissions=( + permission_acl_edit, permission_acl_view, + ) ) - def _create_acl(self): - self.acl = AccessControlList.objects.create( - content_object=self.document, - role=self.role + self._create_test_permission() + ModelPermission.register( + model=self.test_object._meta.model, permissions=( + self.test_permission, + ) + ) + self.test_acl.permissions.add(self.test_permission.stored_permission) + self._inject_test_object_content_type() + + def _request_object_acl_list_api_view(self): + return self.get( + viewname='rest_api:object-acl-list', + kwargs=self.test_content_object_view_kwargs ) - self.acl.permissions.add(permission_document_view.stored_permission) + def test_object_acl_list_api_view_no_permission(self): + response = self._request_object_acl_list_api_view() + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_object_acl_list_view(self): - self._create_acl() + def test_object_acl_list_api_view_with_access(self): + self.grant_access(obj=self.test_object, permission=permission_acl_view) - response = self.get( - viewname='rest_api:accesscontrollist-list', - kwargs={ - 'app_label': self.document_content_type.app_label, - 'model': self.document_content_type.model, - 'object_id': self.document.pk - } - ) + response = self._request_object_acl_list_api_view() + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual( response.data['results'][0]['content_type']['app_label'], - self.document_content_type.app_label + self.test_object_content_type.app_label ) self.assertEqual( - response.data['results'][0]['role']['label'], TEST_ROLE_LABEL + response.data['results'][0]['role']['label'], + self.test_acl.role.label ) - def test_object_acl_delete_view(self): - self._create_acl() + def _request_acl_delete_api_view(self): + kwargs = self.test_content_object_view_kwargs.copy() + kwargs['acl_id'] = self.test_acl.pk - response = self.delete( - viewname='rest_api:accesscontrollist-detail', - kwargs={ - 'app_label': self.document_content_type.app_label, - 'model': self.document_content_type.model, - 'object_id': self.document.pk, - 'acl_pk': self.acl.pk - } + return self.delete( + viewname='rest_api:object-acl-detail', + kwargs=kwargs ) + def test_object_acl_delete_api_view_with_access(self): + self.grant_access(obj=self.test_object, permission=permission_acl_edit) + response = self._request_acl_delete_api_view() + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) - self.assertEqual(AccessControlList.objects.count(), 0) + self.assertTrue(self.test_acl not in AccessControlList.objects.all()) + + def test_object_acl_delete_api_view_no_permission(self): + response = self._request_acl_delete_api_view() + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertTrue(self.test_acl in AccessControlList.objects.all()) def test_object_acl_detail_view(self): self._create_acl() response = self.get( - viewname='rest_api:accesscontrollist-detail', + viewname='rest_api:object-acl-detail', kwargs={ 'app_label': self.document_content_type.app_label, 'model': self.document_content_type.model, @@ -90,12 +111,12 @@ class ACLAPITestCase(DocumentTestMixin, BaseAPITestCase): permission = self.acl.permissions.first() response = self.delete( - viewname='rest_api:accesscontrollist-permission-detail', + viewname='rest_api:object-acl-permission-detail', kwargs={ 'app_label': self.document_content_type.app_label, - 'model': self.document_content_type.model, + 'model_name': self.document_content_type.model, 'object_id': self.document.pk, - 'acl_pk': self.acl.pk, 'permission_pk': permission.pk + 'acl_id': self.acl.pk, 'permission_id': permission.pk } ) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) @@ -106,10 +127,10 @@ class ACLAPITestCase(DocumentTestMixin, BaseAPITestCase): permission = self.acl.permissions.first() response = self.get( - viewname='rest_api:accesscontrollist-permission-detail', + viewname='rest_api:object-acl-permission-detail', kwargs={ 'app_label': self.document_content_type.app_label, - 'model': self.document_content_type.model, + 'model_name': self.document_content_type.model, 'object_id': self.document.pk, 'acl_pk': self.acl.pk, 'permission_pk': permission.pk } @@ -123,12 +144,12 @@ class ACLAPITestCase(DocumentTestMixin, BaseAPITestCase): self._create_acl() response = self.get( - viewname='rest_api:accesscontrollist-permission-list', + viewname='rest_api:object-acl-permission-list', kwargs={ 'app_label': self.document_content_type.app_label, - 'model': self.document_content_type.model, + 'model_name': self.document_content_type.model, 'object_id': self.document.pk, - 'acl_pk': self.acl.pk + 'acl_id': self.acl.pk } ) @@ -141,12 +162,12 @@ class ACLAPITestCase(DocumentTestMixin, BaseAPITestCase): self._create_acl() response = self.post( - viewname='rest_api:accesscontrollist-permission-list', + viewname='rest_api:object-acl-permission-list', kwargs={ 'app_label': self.document_content_type.app_label, - 'model': self.document_content_type.model, + 'model_name': self.document_content_type.model, 'object_id': self.document.pk, 'acl_pk': self.acl.pk - }, data={'permission_pk': permission_acl_view.pk} + }, data={'permission_id': permission_acl_view.pk} ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) @@ -159,17 +180,17 @@ class ACLAPITestCase(DocumentTestMixin, BaseAPITestCase): def test_object_acl_post_no_permissions_added_view(self): response = self.post( - viewname='rest_api:accesscontrollist-list', + viewname='rest_api:object-acl-list', kwargs={ 'app_label': self.document_content_type.app_label, - 'model': self.document_content_type.model, + 'model_name': self.document_content_type.model, 'object_id': self.document.pk - }, data={'role_pk': self.role.pk} + }, data={'role_id': self.test_role.pk} ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual( - self.document.acls.first().role, self.role + self.document.acls.first().role, self.test_role ) self.assertEqual( self.document.acls.first().content_object, self.document @@ -180,13 +201,13 @@ class ACLAPITestCase(DocumentTestMixin, BaseAPITestCase): def test_object_acl_post_with_permissions_added_view(self): response = self.post( - viewname='rest_api:accesscontrollist-list', + viewname='rest_api:object-acl-list', kwargs={ 'app_label': self.document_content_type.app_label, 'model': self.document_content_type.model, 'object_id': self.document.pk }, data={ - 'role_pk': self.role.pk, + 'role_pk': self.test_role.pk, 'permissions_pk_list': permission_acl_view.pk } @@ -197,7 +218,7 @@ class ACLAPITestCase(DocumentTestMixin, BaseAPITestCase): self.document.acls.first().content_object, self.document ) self.assertEqual( - self.document.acls.first().role, self.role + self.document.acls.first().role, self.test_role ) self.assertEqual( self.document.acls.first().permissions.first(), diff --git a/mayan/apps/acls/tests/test_models.py b/mayan/apps/acls/tests/test_models.py index ab2c8e144b..6d6a6683d3 100644 --- a/mayan/apps/acls/tests/test_models.py +++ b/mayan/apps/acls/tests/test_models.py @@ -129,7 +129,6 @@ class PermissionTestCase(DocumentTestMixin, BaseTestCase): # Since document_1 and document_2 are of document_type_1 # they are the only ones that should be returned - self.assertTrue(self.test_document_1 in result) self.assertTrue(self.test_document_2 in result) self.assertTrue(self.test_document_3 not in result) @@ -230,9 +229,6 @@ class InheritedPermissionTestCase(TestModelTestMixin, PermissionTestMixin, RoleT self.assertTrue(self.test_permission.stored_permission in queryset) - #self._delete_test_model(model_name='TestModelParent') - #elf._delete_test_model(model_name='TestModelChild') - def test_retrieve_inherited_related_grandparent_parent_child_permission(self): self._create_test_permission() @@ -291,7 +287,3 @@ class InheritedPermissionTestCase(TestModelTestMixin, PermissionTestMixin, RoleT ) self.assertTrue(self.test_permission.stored_permission in queryset) - - #self._delete_test_model(model_name='TestModelGrandParent') - #self._delete_test_model(model_name='TestModelParent') - #self._delete_test_model(model_name='TestModelChild') diff --git a/mayan/apps/acls/urls.py b/mayan/apps/acls/urls.py index d172bb3bdc..640d9c510a 100644 --- a/mayan/apps/acls/urls.py +++ b/mayan/apps/acls/urls.py @@ -2,10 +2,7 @@ from __future__ import unicode_literals from django.conf.urls import url -from .api_views import ( - APIObjectACLListView, APIObjectACLPermissionListView, - APIObjectACLPermissionView, APIObjectACLView -) +from .api_views import ObjectACLAPIViewSet from .views import ( ACLCreateView, ACLDeleteView, ACLListView, ACLPermissionsView ) @@ -29,6 +26,14 @@ urlpatterns = [ ), ] +api_router_entries = ( + { + 'prefix': r'apps/(?P[^/.]+)/models/(?P[^/.]+)/objects/(?P[^/.]+)/acls', + 'viewset': ObjectACLAPIViewSet, 'basename': 'object-acl' + }, +) + +''' api_urls = [ url( regex=r'^objects/(?P[-\w]+)/(?P[-\w]+)/(?P\d+)/acls/$', @@ -49,3 +54,4 @@ api_urls = [ view=APIObjectACLPermissionView.as_view() ), ] +''' diff --git a/mayan/apps/acls/views.py b/mayan/apps/acls/views.py index b1c11c7d4c..fe3d2ad95e 100644 --- a/mayan/apps/acls/views.py +++ b/mayan/apps/acls/views.py @@ -58,7 +58,7 @@ class ACLCreateView(ContentTypeViewMixin, ExternalObjectMixin, SingleObjectCreat pk__in=self.get_external_object().acls.values('role') ), 'widget_attributes': {'class': 'select2'}, - '_user': self.request.user + 'user': self.request.user } def get_instance_extra_data(self): @@ -174,12 +174,10 @@ class ACLPermissionsView(AddRemoveView): def get_disabled_choices(self): """ - Get permissions from a parent's ACLs. We return a list since that is - what the form widget's can process. + Get permissions from a parent's ACLs or directly granted to the role. + We return a list since that is what the form widget's can process. """ - return self.main_object.get_inherited_permissions().values_list( - 'pk', flat=True - ) + return self.main_object.get_inherited_permissions().values_list('pk', flat=True) def get_extra_context(self): return { @@ -195,9 +193,10 @@ class ACLPermissionsView(AddRemoveView): def get_list_added_help_text(self): if self.main_object.get_inherited_permissions(): return _( - 'Disabled permissions are inherited from a parent object and ' - 'can\'t be removed from this view, they need to be removed ' - 'from the parent object\'s ACL view.' + 'Disabled permissions are inherited from a parent object or ' + 'directly granted to the role and can\'t be removed from this ' + 'view. Inherited permissions need to be removed from the ' + 'parent object\'s ACL or from them role via the Setup menu.' ) def get_list_added_queryset(self): @@ -210,9 +209,10 @@ class ACLPermissionsView(AddRemoveView): remove from the parent first to enable the choice in the form, remove it from the ACL and then re-add it to the parent ACL. """ - queryset = super(ACLPermissionsView, self).get_list_added_queryset() + queryset_acl = super(ACLPermissionsView, self).get_list_added_queryset() + return ( - queryset | self.main_object.get_inherited_permissions() + queryset_acl | self.main_object.get_inherited_permissions() ).distinct() def get_secondary_object_source_queryset(self): diff --git a/mayan/apps/common/tests/mixins.py b/mayan/apps/common/tests/mixins.py index b17166d5a7..16401b70c7 100644 --- a/mayan/apps/common/tests/mixins.py +++ b/mayan/apps/common/tests/mixins.py @@ -259,6 +259,11 @@ class TestModelTestMixin(object): self.__class__._test_models.append(model_name) + def _create_test_object(self, model_name='TestModel', **kwargs): + TestModel = getattr(self, model_name) + + self.test_object = TestModel.objects.create(**kwargs) + def _delete_test_model(self, model_name='TestModel'): TestModel = getattr(self, model_name)