diff --git a/wagtail/wagtailcore/permission_policies/collections.py b/wagtail/wagtailcore/permission_policies/collections.py index 98d1ecf009..ded790e428 100644 --- a/wagtail/wagtailcore/permission_policies/collections.py +++ b/wagtail/wagtailcore/permission_policies/collections.py @@ -44,7 +44,9 @@ class CollectionPermissionLookupMixin(object): return collection_permissions.exists() - def _collection_permission_filter(self, user, actions): + def _collection_permission_filter( + self, user, actions, path_filter_param='collection__path__startswith' + ): """ Return a filter object that can be applied to a queryset of this model so that it only includes instances in collections for which the user has @@ -64,11 +66,13 @@ class CollectionPermissionLookupMixin(object): if collection_root_paths: # build a filter expression that will filter our model to just those # instances in collections with a path that starts with one of the above - collection_path_filter = Q(collection__path__startswith=collection_root_paths[0]) + collection_path_filter = Q(**{ + path_filter_param: collection_root_paths[0] + }) for path in collection_root_paths[1:]: - collection_path_filter = collection_path_filter | Q( - collection__path__startswith=path - ) + collection_path_filter = collection_path_filter | Q(**{ + path_filter_param: path + }) return collection_path_filter else: # no matching collections @@ -109,6 +113,13 @@ class CollectionPermissionLookupMixin(object): self._users_with_perm_filter(actions, collection=collection) ).distinct() + def collections_user_has_permission_for(self, user, action): + """ + Return a queryset of all collections in which the given user has + permission to perform the given action + """ + return self.collections_user_has_any_permission_for(user, [action]) + class CollectionPermissionPolicy(CollectionPermissionLookupMixin, BaseDjangoAuthPermissionPolicy): """ @@ -174,6 +185,26 @@ class CollectionPermissionPolicy(CollectionPermissionLookupMixin, BaseDjangoAuth """ return self._users_with_perm(actions, collection=instance.collection) + def collections_user_has_any_permission_for(self, user, actions): + """ + Return a queryset of all collections in which the given user has + permission to perform any of the given actions + """ + if user.is_active and user.is_superuser: + # active superusers can perform any action (including unrecognised ones) + # in any collection + return Collection.objects.all() + + elif not user.is_authenticated(): + return Collection.objects.none() + + else: + return Collection.objects.filter( + self._collection_permission_filter( + user, actions, path_filter_param='path__startswith' + ) + ) + class CollectionOwnershipPermissionPolicy( CollectionPermissionLookupMixin, BaseDjangoAuthPermissionPolicy @@ -295,3 +326,39 @@ class CollectionOwnershipPermissionPolicy( # not meaningful for existing instances. As such, the action is only # available to superusers return get_user_model().objects.filter(is_active=True, is_superuser=True) + + def collections_user_has_any_permission_for(self, user, actions): + """ + Return a queryset of all collections in which the given user has + permission to perform any of the given actions + """ + if user.is_active and user.is_superuser: + # active superusers can perform any action (including unrecognised ones) + # in any collection + return Collection.objects.all() + + elif not user.is_authenticated(): + return Collection.objects.none() + + elif 'change' in actions or 'delete' in actions: + # return collections which are covered by either 'add' or 'change' permissions + # (since collections with 'add' permissions can *potentially* contain instances + # they own and can therefore edit) + + return Collection.objects.filter( + self._collection_permission_filter( + user, ['add', 'change'], path_filter_param='path__startswith' + ) + ) + + elif 'add' in actions: + return Collection.objects.filter( + self._collection_permission_filter( + user, ['add'], path_filter_param='path__startswith' + ) + ) + + else: + # action is not recognised, and so non-superusers + # cannot perform it on any existing collections + return Collection.objects.none() diff --git a/wagtail/wagtailcore/tests/test_collection_permission_policies.py b/wagtail/wagtailcore/tests/test_collection_permission_policies.py index 115c6a2d68..a5231db8a1 100644 --- a/wagtail/wagtailcore/tests/test_collection_permission_policies.py +++ b/wagtail/wagtailcore/tests/test_collection_permission_policies.py @@ -23,28 +23,28 @@ class PermissionPolicyTestCase(PermissionPolicyTestUtils, TestCase): ) # Collections - root_collection = Collection.get_first_root_node() - reports_collection = root_collection.add_child(name="Reports") + self.root_collection = Collection.get_first_root_node() + self.reports_collection = self.root_collection.add_child(name="Reports") # Groups doc_changers_group = Group.objects.create(name="Document changers") GroupCollectionPermission.objects.create( group=doc_changers_group, - collection=root_collection, + collection=self.root_collection, permission=change_doc_permission ) report_changers_group = Group.objects.create(name="Report changers") GroupCollectionPermission.objects.create( group=report_changers_group, - collection=reports_collection, + collection=self.reports_collection, permission=change_doc_permission ) report_adders_group = Group.objects.create(name="Report adders") GroupCollectionPermission.objects.create( group=report_adders_group, - collection=reports_collection, + collection=self.reports_collection, permission=add_doc_permission ) @@ -93,31 +93,31 @@ class PermissionPolicyTestCase(PermissionPolicyTestUtils, TestCase): # a document in the root owned by 'reportchanger' self.changer_doc = Document.objects.create( - title="reportchanger's document", collection=root_collection, + title="reportchanger's document", collection=self.root_collection, uploaded_by_user=self.report_changer ) # a document in reports owned by 'reportchanger' self.changer_report = Document.objects.create( - title="reportchanger's report", collection=reports_collection, + title="reportchanger's report", collection=self.reports_collection, uploaded_by_user=self.report_changer ) # a document in reports owned by 'reportadder' self.adder_report = Document.objects.create( - title="reportadder's report", collection=reports_collection, + title="reportadder's report", collection=self.reports_collection, uploaded_by_user=self.report_adder ) # a document in reports owned by 'uselessuser' self.useless_report = Document.objects.create( - title="uselessuser's report", collection=reports_collection, + title="uselessuser's report", collection=self.reports_collection, uploaded_by_user=self.useless_user ) # a document with no owner self.anonymous_report = Document.objects.create( - title="anonymous report", collection=reports_collection + title="anonymous report", collection=self.reports_collection ) @@ -390,6 +390,120 @@ class TestCollectionPermissionPolicy(PermissionPolicyTestCase): [self.superuser] ) + def test_collections_user_has_permission_for(self): + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.superuser, 'change', + ), + [self.root_collection, self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.inactive_superuser, 'change', + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.doc_changer, 'change', + ), + [self.root_collection, self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.report_changer, 'change', + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.report_adder, 'change', + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.report_adder, 'add', + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.useless_user, 'change', + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.anonymous_user, 'change', + ), + [] + ) + + def test_collections_user_has_any_permission_for(self): + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.superuser, ['change', 'delete'] + ), + [self.root_collection, self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.inactive_superuser, ['change', 'delete'] + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.doc_changer, ['change', 'delete'] + ), + [self.root_collection, self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.report_changer, ['change', 'delete'] + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.report_adder, ['change', 'delete'] + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.report_adder, ['add', 'delete'] + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.useless_user, ['change', 'delete'] + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.anonymous_user, ['change', 'delete'] + ), + [] + ) + class TestCollectionOwnershipPermissionPolicy(PermissionPolicyTestCase): def setUp(self): @@ -707,3 +821,117 @@ class TestCollectionOwnershipPermissionPolicy(PermissionPolicyTestCase): ), [self.superuser, self.doc_changer, self.report_changer] ) + + def test_collections_user_has_permission_for(self): + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.superuser, 'change', + ), + [self.root_collection, self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.inactive_superuser, 'change', + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.doc_changer, 'change', + ), + [self.root_collection, self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.report_changer, 'change', + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.report_adder, 'change', + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.report_adder, 'add', + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.useless_user, 'change', + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_permission_for( + self.anonymous_user, 'change', + ), + [] + ) + + def test_collections_user_has_any_permission_for(self): + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.superuser, ['change', 'delete'] + ), + [self.root_collection, self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.inactive_superuser, ['change', 'delete'] + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.doc_changer, ['change', 'delete'] + ), + [self.root_collection, self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.report_changer, ['change', 'delete'] + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.report_adder, ['change', 'delete'] + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.report_adder, ['add', 'delete'] + ), + [self.reports_collection] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.useless_user, ['change', 'delete'] + ), + [] + ) + + self.assertResultSetEqual( + self.policy.collections_user_has_any_permission_for( + self.anonymous_user, ['change', 'delete'] + ), + [] + )