From 11e92ab7f65ff722859726cb638fefa67fdcc022 Mon Sep 17 00:00:00 2001 From: Matheus Bratfisch Date: Wed, 21 Dec 2016 16:35:41 +1300 Subject: [PATCH] Fix #3240 - Implement is_descendant_of on copy to check to node --- .../wagtailadmin/tests/test_pages_views.py | 13 ++++++++++ wagtail/wagtailadmin/views/pages.py | 4 +-- wagtail/wagtailcore/models.py | 26 +++++++++++++++++++ wagtail/wagtailcore/tests/test_page_model.py | 11 ++++++++ 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 3e395bf35..785814e97 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -2198,6 +2198,19 @@ class TestPageCopy(TestCase, WagtailTestUtils): "This slug is already in use within the context of its parent page \"Welcome to your new Wagtail site!\"" ) + def test_page_copy_post_and_subpages_to_same_tree_branch(self): + # This tests the existing slug checking on page copy when changing the parent page + + # Attempt to copy the page and changed the parent page + post_data = { + 'new_title': "Hello world 2", + 'new_slug': 'hello-world', + 'new_parent_page': str(self.test_child_page.id), + 'copy_subpages': True, + } + response = self.client.post(reverse('wagtailadmin_pages:copy', args=(self.test_page.id,)), post_data) + self.assertEqual(response.status_code, 403) + def test_page_copy_post_existing_slug_to_another_parent_page(self): # This tests the existing slug checking on page copy when changing the parent page diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 9a4a65b5a..a070ad02e 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -811,8 +811,8 @@ def copy(request, page_id): if form.cleaned_data['new_parent_page']: parent_page = form.cleaned_data['new_parent_page'] - # Make sure this user has permission to add subpages on the parent - if not parent_page.permissions_for_user(request.user).can_add_subpage(): + if not page.permissions_for_user(request.user).can_copy_to(parent_page, + form.cleaned_data.get('copy_subpages')): raise PermissionDenied # Re-check if the user has permission to publish subpages on the new parent diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 4cef026b7..a7707f4e1 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -1094,6 +1094,8 @@ class Page(six.with_metaclass(PageBase, AbstractPage, index.Indexed, Clusterable setattr(page_copy, field, value) if to: + if recursive and (to == self or to.is_descendant_of(self)): + raise Exception("You cannot copy a tree branch recursively into itself") page_copy = to.add_child(instance=page_copy) else: page_copy = self.add_sibling(instance=page_copy) @@ -1814,6 +1816,30 @@ class PagePermissionTester(object): # no publishing required, so the already-tested 'add' permission is sufficient return True + def can_copy_to(self, destination, recursive=False): + # reject the logically impossible cases first + # recursive can't copy to the same tree otherwise it will be on infinite loop + if recursive and (self.page == destination or destination.is_descendant_of(self.page)): + return False + + # shortcut the trivial 'everything' / 'nothing' permissions + if not self.user.is_active: + return False + if self.user.is_superuser: + return True + + # Inspect permissions on the destination + destination_perms = self.user_perms.for_page(destination) + + if not destination.specific_class.creatable_subpage_models(): + return False + + # we always need at least add permission in the target + if 'add' not in destination_perms.permissions: + return False + + return True + class PageViewRestriction(models.Model): NONE = 'none' diff --git a/wagtail/wagtailcore/tests/test_page_model.py b/wagtail/wagtailcore/tests/test_page_model.py index dea43c1ab..3e54b0c29 100644 --- a/wagtail/wagtailcore/tests/test_page_model.py +++ b/wagtail/wagtailcore/tests/test_page_model.py @@ -772,6 +772,17 @@ class TestCopyPage(TestCase): old_christmas_event.specific.revisions.count(), 1, "Revisions were removed from the original page" ) + def test_copy_page_copies_recursively_to_the_same_tree(self): + events_index = EventIndex.objects.get(url_path='/home/events/') + old_christmas_event = events_index.get_children().filter(slug='christmas').first().specific + old_christmas_event.save_revision() + + with self.assertRaises(Exception) as exception: + events_index.copy( + recursive=True, update_attrs={'title': "New events index", 'slug': 'new-events-index'}, to=events_index + ) + self.assertEqual(str(exception.exception), "You cannot copy a tree branch recursively into itself") + def test_copy_page_updates_user(self): event_moderator = get_user_model().objects.get(username='eventmoderator') christmas_event = EventPage.objects.get(url_path='/home/events/christmas/')