From 354ea434aeb186b74df8ef692cca78d9026398de Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Sat, 19 Jan 2019 00:09:09 -0400 Subject: [PATCH] Add keyword arguments to the ACLs app code Rename all instance of `pk` or `acl_pk` to `acl_id` to match the preferred URL parameter naming conventions of using `id` instead of `pk`. Signed-off-by: Roberto Rosario --- mayan/apps/acls/links.py | 7 +- mayan/apps/acls/models.py | 2 +- mayan/apps/acls/tests/mixins.py | 1 - mayan/apps/acls/tests/test_models.py | 109 +++++++++++++-------------- mayan/apps/acls/tests/test_views.py | 4 +- mayan/apps/acls/urls.py | 10 +-- mayan/apps/acls/views.py | 36 +++------ mayan/apps/acls/workflow_actions.py | 3 +- 8 files changed, 77 insertions(+), 95 deletions(-) diff --git a/mayan/apps/acls/links.py b/mayan/apps/acls/links.py index da2f289e10..adebdea8fb 100644 --- a/mayan/apps/acls/links.py +++ b/mayan/apps/acls/links.py @@ -29,13 +29,14 @@ def get_kwargs_factory(variable_name): link_acl_delete = Link( - args='resolved_object.pk', icon_class=icon_acl_delete, + icon_class=icon_acl_delete, kwargs={'acl_id': 'resolved_object.pk'}, permissions=(permission_acl_edit,), permissions_related='content_object', tags='dangerous', text=_('Delete'), view='acls:acl_delete', ) link_acl_list = Link( - icon_class=icon_acl_list, kwargs=get_kwargs_factory('resolved_object'), - permissions=(permission_acl_view,), text=_('ACLs'), view='acls:acl_list' + icon_class=icon_acl_list, kwargs=get_kwargs_factory( + variable_name='resolved_object' + ), permissions=(permission_acl_view,), text=_('ACLs'), view='acls:acl_list' ) link_acl_create = Link( icon_class=icon_acl_new, kwargs=get_kwargs_factory('resolved_object'), diff --git a/mayan/apps/acls/models.py b/mayan/apps/acls/models.py index e93f00f486..4a7fb10726 100644 --- a/mayan/apps/acls/models.py +++ b/mayan/apps/acls/models.py @@ -67,7 +67,7 @@ class AccessControlList(models.Model): def get_absolute_url(self): return reverse( - viewname='acls:acl_permissions', kwargs={'acl_pk': self.pk} + viewname='acls:acl_permissions', kwargs={'acl_id': self.pk} ) def get_inherited_permissions(self): diff --git a/mayan/apps/acls/tests/mixins.py b/mayan/apps/acls/tests/mixins.py index 7c32cb7287..6f4c9af305 100644 --- a/mayan/apps/acls/tests/mixins.py +++ b/mayan/apps/acls/tests/mixins.py @@ -9,7 +9,6 @@ from ..models import AccessControlList class ACLTestCaseMixin(RoleTestCaseMixin, UserTestCaseMixin): - def setUp(self): super(ACLTestCaseMixin, self).setUp() if hasattr(self, '_test_case_user'): diff --git a/mayan/apps/acls/tests/test_models.py b/mayan/apps/acls/tests/test_models.py index 1eaeb57e24..f113a4ff43 100644 --- a/mayan/apps/acls/tests/test_models.py +++ b/mayan/apps/acls/tests/test_models.py @@ -6,154 +6,147 @@ from mayan.apps.common.tests import BaseTestCase from mayan.apps.documents.models import Document, DocumentType from mayan.apps.documents.permissions import permission_document_view from mayan.apps.documents.tests import ( - TEST_DOCUMENT_TYPE_2_LABEL, TEST_DOCUMENT_TYPE_LABEL, - TEST_SMALL_DOCUMENT_PATH + DocumentTestMixin, TEST_DOCUMENT_TYPE_2_LABEL, TEST_DOCUMENT_TYPE_LABEL ) from ..models import AccessControlList -class PermissionTestCase(BaseTestCase): +class PermissionTestCase(DocumentTestMixin, BaseTestCase): + auto_create_document_type = False + def setUp(self): super(PermissionTestCase, self).setUp() - self.document_type_1 = DocumentType.objects.create( + self.test_document_type_1 = DocumentType.objects.create( label=TEST_DOCUMENT_TYPE_LABEL ) - self.document_type_2 = DocumentType.objects.create( + self.test_document_type_2 = DocumentType.objects.create( label=TEST_DOCUMENT_TYPE_2_LABEL ) - with open(TEST_SMALL_DOCUMENT_PATH, mode='rb') as file_object: - self.document_1 = self.document_type_1.new_document( - file_object=file_object - ) - - with open(TEST_SMALL_DOCUMENT_PATH, mode='rb') as file_object: - self.document_2 = self.document_type_1.new_document( - file_object=file_object - ) - - with open(TEST_SMALL_DOCUMENT_PATH, mode='rb') as file_object: - self.document_3 = self.document_type_2.new_document( - file_object=file_object - ) - - def tearDown(self): - for document_type in DocumentType.objects.all(): - document_type.delete() - super(PermissionTestCase, self).tearDown() + self.test_document_1 = self.upload_document( + document_type=self.test_document_type_1 + ) + self.test_document_2 = self.upload_document( + document_type=self.test_document_type_1 + ) + self.test_document_3 = self.upload_document( + document_type=self.test_document_type_2 + ) def test_check_access_without_permissions(self): with self.assertRaises(PermissionDenied): AccessControlList.objects.check_access( - permissions=(permission_document_view,), - user=self.user, obj=self.document_1 + obj=self.test_document_1, permissions=(permission_document_view,), + user=self._test_case_user ) def test_filtering_without_permissions(self): - self.assertQuerysetEqual( + self.assertEqual( AccessControlList.objects.filter_by_access( - permission=permission_document_view, user=self.user, - queryset=Document.objects.all() - ), [] + permission=permission_document_view, + queryset=Document.objects.all(), user=self._test_case_user, + ).count(), 0 ) def test_check_access_with_acl(self): acl = AccessControlList.objects.create( - content_object=self.document_1, role=self.role + content_object=self.test_document_1, role=self._test_case_role ) acl.permissions.add(permission_document_view.stored_permission) try: AccessControlList.objects.check_access( - permissions=(permission_document_view,), user=self.user, - obj=self.document_1 + obj=self.test_document_1, permissions=(permission_document_view,), + user=self._test_case_user ) except PermissionDenied: self.fail('PermissionDenied exception was not expected.') def test_filtering_with_permissions(self): acl = AccessControlList.objects.create( - content_object=self.document_1, role=self.role + content_object=self.test_document_1, role=self._test_case_role ) acl.permissions.add(permission_document_view.stored_permission) self.assertQuerysetEqual( AccessControlList.objects.filter_by_access( - permission=permission_document_view, user=self.user, - queryset=Document.objects.all() - ), (repr(self.document_1),) + permission=permission_document_view, + queryset=Document.objects.all(), user=self._test_case_user + ), (repr(self.test_document_1),) ) def test_check_access_with_inherited_acl(self): acl = AccessControlList.objects.create( - content_object=self.document_type_1, role=self.role + content_object=self.test_document_type_1, role=self._test_case_role ) acl.permissions.add(permission_document_view.stored_permission) try: AccessControlList.objects.check_access( - permissions=(permission_document_view,), user=self.user, - obj=self.document_1 + obj=self.test_document_1, permissions=(permission_document_view,), + user=self._test_case_user ) except PermissionDenied: self.fail('PermissionDenied exception was not expected.') def test_check_access_with_inherited_acl_and_local_acl(self): acl = AccessControlList.objects.create( - content_object=self.document_type_1, role=self.role + content_object=self.test_document_type_1, role=self._test_case_role ) acl.permissions.add(permission_document_view.stored_permission) acl = AccessControlList.objects.create( - content_object=self.document_3, role=self.role + content_object=self.test_document_3, role=self._test_case_role ) acl.permissions.add(permission_document_view.stored_permission) try: AccessControlList.objects.check_access( - permissions=(permission_document_view,), user=self.user, - obj=self.document_3 + obj=self.test_document_3, permissions=(permission_document_view,), + user=self._test_case_user ) except PermissionDenied: self.fail('PermissionDenied exception was not expected.') def test_filtering_with_inherited_permissions(self): acl = AccessControlList.objects.create( - content_object=self.document_type_1, role=self.role + content_object=self.test_document_type_1, role=self._test_case_role ) acl.permissions.add(permission_document_view.stored_permission) result = AccessControlList.objects.filter_by_access( - permission=permission_document_view, user=self.user, - queryset=Document.objects.all() + permission=permission_document_view, queryset=Document.objects.all(), + user=self._test_case_user ) # Since document_1 and document_2 are of document_type_1 # they are the only ones that should be returned - self.assertTrue(self.document_1 in result) - self.assertTrue(self.document_2 in result) - self.assertTrue(self.document_3 not in result) + 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) def test_filtering_with_inherited_permissions_and_local_acl(self): - self.role.permissions.add(permission_document_view.stored_permission) + self._test_case_role.permissions.add( + permission_document_view.stored_permission + ) acl = AccessControlList.objects.create( - content_object=self.document_type_1, role=self.role + content_object=self.test_document_type_1, role=self._test_case_role ) acl.permissions.add(permission_document_view.stored_permission) acl = AccessControlList.objects.create( - content_object=self.document_3, role=self.role + content_object=self.test_document_3, role=self._test_case_role ) acl.permissions.add(permission_document_view.stored_permission) result = AccessControlList.objects.filter_by_access( - permission=permission_document_view, user=self.user, - queryset=Document.objects.all() + permission=permission_document_view, queryset=Document.objects.all(), + user=self._test_case_user, ) - self.assertTrue(self.document_1 in result) - self.assertTrue(self.document_2 in result) - self.assertTrue(self.document_3 in result) + self.assertTrue(self.test_document_1 in result) + self.assertTrue(self.test_document_2 in result) + self.assertTrue(self.test_document_3 in result) diff --git a/mayan/apps/acls/tests/test_views.py b/mayan/apps/acls/tests/test_views.py index 05a839f298..a41ca75164 100644 --- a/mayan/apps/acls/tests/test_views.py +++ b/mayan/apps/acls/tests/test_views.py @@ -99,7 +99,7 @@ class AccessControlListViewTestCase(RoleTestMixin, GenericDocumentViewTestCase): def _request_acl_delete_view(self): return self.post( - viewname='acls:acl_delete', kwargs={'acl_pk': self.test_acl.pk} + viewname='acls:acl_delete', kwargs={'acl_id': self.test_acl.pk} ) def test_acl_delete_view_no_permission(self): @@ -154,7 +154,7 @@ class AccessControlListViewTestCase(RoleTestMixin, GenericDocumentViewTestCase): def _request_get_acl_permissions_view(self): return self.get( viewname='acls:acl_permissions', - kwargs={'acl_pk': self.test_acl.pk} + kwargs={'acl_id': self.test_acl.pk} ) def test_acl_permissions_view_get_no_permission(self): diff --git a/mayan/apps/acls/urls.py b/mayan/apps/acls/urls.py index dc73f8a5b7..d172bb3bdc 100644 --- a/mayan/apps/acls/urls.py +++ b/mayan/apps/acls/urls.py @@ -20,11 +20,11 @@ urlpatterns = [ name='acl_list', view=ACLListView.as_view() ), url( - regex=r'^acls/(?P\d+)/delete/$', name='acl_delete', + regex=r'^acls/(?P\d+)/delete/$', name='acl_delete', view=ACLDeleteView.as_view() ), url( - regex=r'^acls/(?P\d+)/permissions/$', name='acl_permissions', + regex=r'^acls/(?P\d+)/permissions/$', name='acl_permissions', view=ACLPermissionsView.as_view() ), ] @@ -35,16 +35,16 @@ api_urls = [ name='accesscontrollist-list', view=APIObjectACLListView.as_view() ), url( - regex=r'^objects/(?P[-\w]+)/(?P[-\w]+)/(?P\d+)/acls/(?P\d+)/$', + regex=r'^objects/(?P[-\w]+)/(?P[-\w]+)/(?P\d+)/acls/(?P\d+)/$', name='accesscontrollist-detail', view=APIObjectACLView.as_view() ), url( - regex=r'^objects/(?P[-\w]+)/(?P[-\w]+)/(?P\d+)/acls/(?P\d+)/permissions/$', + regex=r'^objects/(?P[-\w]+)/(?P[-\w]+)/(?P\d+)/acls/(?P\d+)/permissions/$', name='accesscontrollist-permission-list', view=APIObjectACLPermissionListView.as_view() ), url( - regex=r'^objects/(?P[-\w]+)/(?P[-\w]+)/(?P\d+)/acls/(?P\d+)/permissions/(?P\d+)/$', + regex=r'^objects/(?P[-\w]+)/(?P[-\w]+)/(?P\d+)/acls/(?P\d+)/permissions/(?P\d+)/$', name='accesscontrollist-permission-detail', view=APIObjectACLPermissionView.as_view() ), diff --git a/mayan/apps/acls/views.py b/mayan/apps/acls/views.py index 410f75b006..2028cc93a0 100644 --- a/mayan/apps/acls/views.py +++ b/mayan/apps/acls/views.py @@ -3,7 +3,6 @@ from __future__ import absolute_import, unicode_literals import itertools import logging -from django.core.exceptions import PermissionDenied from django.shortcuts import get_object_or_404 from django.template import RequestContext from django.urls import reverse @@ -11,7 +10,7 @@ from django.utils.encoding import force_text from django.utils.translation import ugettext_lazy as _ from mayan.apps.common.mixins import ( - ContentTypeViewMixin, ExternalObjectViewMixin + ContentTypeViewMixin, ExternalObjectMixin ) from mayan.apps.common.views import ( AssignRemoveView, SingleObjectCreateView, SingleObjectDeleteView, @@ -30,7 +29,7 @@ from .permissions import permission_acl_edit, permission_acl_view logger = logging.getLogger(__name__) -class ACLCreateView(ContentTypeViewMixin, ExternalObjectViewMixin, SingleObjectCreateView): +class ACLCreateView(ContentTypeViewMixin, ExternalObjectMixin, SingleObjectCreateView): external_object_permission = permission_acl_edit external_object_pk_url_kwarg = 'object_id' form_class = ACLCreateForm @@ -77,11 +76,9 @@ class ACLCreateView(ContentTypeViewMixin, ExternalObjectViewMixin, SingleObjectC class ACLDeleteView(SingleObjectDeleteView): - object_permission = permission_acl_edit - object_permission_related = 'content_object' - object_permission_raise_404 = True model = AccessControlList - pk_url_kwarg = 'acl_pk' + object_permission = permission_acl_edit + pk_url_kwarg = 'acl_id' def get_extra_context(self): return { @@ -100,7 +97,7 @@ class ACLDeleteView(SingleObjectDeleteView): ) -class ACLListView(ContentTypeViewMixin, ExternalObjectViewMixin, SingleObjectListView): +class ACLListView(ContentTypeViewMixin, ExternalObjectMixin, SingleObjectListView): external_object_permission = permission_acl_view external_object_pk_url_kwarg = 'object_id' @@ -211,24 +208,15 @@ class ACLPermissionsView(AssignRemoveView): return StoredPermission.objects.filter(pk__in=merged_pks) def get_object(self): - acl = get_object_or_404( - klass=AccessControlList, pk=self.kwargs['acl_pk'] + return get_object_or_404( + klass=self.get_queryset(), pk=self.kwargs['acl_id'] ) - # Get the ACL, from this get the object of the ACL, from the object - # get all ACLs it holds as a filtered queryset by access. - - try: - AccessControlList.objects.check_access( - permissions=(permission_acl_edit,), obj=acl.content_object, - user=self.request.user - ) - except PermissionDenied: - queryset = AccessControlList.objects.none() - else: - queryset = acl.content_object.acls.all() - - return get_object_or_404(klass=queryset, pk=self.kwargs['acl_pk']) + def get_queryset(self): + return AccessControlList.objects.restrict_queryset( + permission=permission_acl_edit, + queryset=AccessControlList.objects.all(), user=self.request.user + ) def get_right_list_help_text(self): if self.get_object().get_inherited_permissions(): diff --git a/mayan/apps/acls/workflow_actions.py b/mayan/apps/acls/workflow_actions.py index 0d333914dd..8da80d1c18 100644 --- a/mayan/apps/acls/workflow_actions.py +++ b/mayan/apps/acls/workflow_actions.py @@ -89,7 +89,8 @@ class GrantAccessAction(WorkflowAction): try: AccessControlList.objects.check_access( - permissions=permission_acl_edit, user=request.user, obj=obj + obj=obj, permissions=permission_acl_edit, + user=request.user ) except Exception as exception: raise ValidationError(exception)