prevent users from navigating privileged pages

This change prevents non-admins from navigating around the Wagtail page
tree for pages that lie outside of their explorable root. Currently,
non-admins can hit any page in the tree using a URL like

/admin/pages/123/

even if they don't have any permissions over that page or its part of
the page tree.

This change adds a (temporary) redirect to requests like this, so that
users may not navigate to parts of the tree that lie outside outside of
their explorable site root, as determined by the page privileges they
have. If they try to hit a URL like the one above, they get redirected
to their explorable site root navigation page instead.

Relevant unit tests have been modified to incorporate this change.

CHANGELOG.txt

@@ -48,6 +48,7 @@ Changelog
  * Fix: Menu icon no longer overlaps with title in Modeladmin on mobile (Coen van der Kamp)
  * Fix: Background color overflow within the Wagtail documentation (Sergey Fedoseev)
  * Fix: Page count on homepage summary panel now takes account of user permissions (Andy Chosak)
+ * Fix: Explorer view now prevents navigating outside of the common ancestor of the user's permissions (Andy Chosak)
 
 
 2.0.1 (04.04.2018)

docs/releases/2.1.rst

@@ -71,6 +71,7 @@ Bug fixes
  * Menu icon no longer overlaps with title in Modeladmin on mobile (Coen van der Kamp)
  * Background color overflow within the Wagtail documentation (Sergey Fedoseev)
  * Page count on homepage summary panel now takes account of user permissions (Andy Chosak)
+ * Explorer view now prevents navigating outside of the common ancestor of the user's permissions (Andy Chosak)
 
 
 Upgrade considerations

wagtail/admin/tests/test_pages_views.py

@@ -336,6 +336,9 @@ class TestPageExplorerSignposting(TestCase, WagtailTestUtils):
         )
         self.root_page.add_child(instance=self.no_site_page)
 
+    # Tests for users that have both add-site permission, and explore permission at the given view;
+    # warning messages should include advice re configuring sites
+
     def test_admin_at_root(self):
         self.assertTrue(self.client.login(username='superuser', password='password'))
         response = self.client.get(reverse('wagtailadmin_explore_root'))
@@ -373,9 +376,19 @@ class TestPageExplorerSignposting(TestCase, WagtailTestUtils):
         # There should be no warning message here
         self.assertNotContains(response, "Pages created here will not be accessible")
 
+    # Tests for standard users that have explore permission at the given view;
+    # warning messages should omit advice re configuring sites
+
     def test_nonadmin_at_root(self):
+        # Assign siteeditor permission over no_site_page, so that the deepest-common-ancestor
+        # logic allows them to explore root
+        GroupPagePermission.objects.create(
+            group=Group.objects.get(name="Site-wide editors"),
+            page=self.no_site_page, permission_type='add'
+        )
         self.assertTrue(self.client.login(username='siteeditor', password='password'))
         response = self.client.get(reverse('wagtailadmin_explore_root'))
+
         self.assertEqual(response.status_code, 200)
         # Non-admin should get a simple "create pages as children of the homepage" prompt
         self.assertContains(
@@ -385,8 +398,14 @@ class TestPageExplorerSignposting(TestCase, WagtailTestUtils):
         )
 
     def test_nonadmin_at_non_site_page(self):
+        # Assign siteeditor permission over no_site_page
+        GroupPagePermission.objects.create(
+            group=Group.objects.get(name="Site-wide editors"),
+            page=self.no_site_page, permission_type='add'
+        )
         self.assertTrue(self.client.login(username='siteeditor', password='password'))
         response = self.client.get(reverse('wagtailadmin_explore', args=(self.no_site_page.id, )))
+
         self.assertEqual(response.status_code, 200)
         # Non-admin should get a warning about unroutable pages
         self.assertContains(
@@ -404,6 +423,43 @@ class TestPageExplorerSignposting(TestCase, WagtailTestUtils):
         # There should be no warning message here
         self.assertNotContains(response, "Pages created here will not be accessible")
 
+    # Tests for users that have explore permission *somewhere*, but not at the view being tested;
+    # in all cases, they should be redirected to their explorable root
+
+    def test_bad_permissions_at_root(self):
+        # 'siteeditor' does not have permission to explore the root
+        self.assertTrue(self.client.login(username='siteeditor', password='password'))
+        response = self.client.get(reverse('wagtailadmin_explore_root'))
+
+        # Users without permission to explore here should be redirected to their explorable root.
+        self.assertEqual(
+            (response.status_code, response['Location']),
+            (302, reverse('wagtailadmin_explore', args=(self.site_page.pk, )))
+        )
+
+    def test_bad_permissions_at_non_site_page(self):
+        # 'siteeditor' does not have permission to explore no_site_page
+        self.assertTrue(self.client.login(username='siteeditor', password='password'))
+        response = self.client.get(reverse('wagtailadmin_explore', args=(self.no_site_page.id, )))
+
+        # Users without permission to explore here should be redirected to their explorable root.
+        self.assertEqual(
+            (response.status_code, response['Location']),
+            (302, reverse('wagtailadmin_explore', args=(self.site_page.pk, )))
+        )
+
+    def test_bad_permissions_at_site_page(self):
+        # Adjust siteeditor's permission so that they have permission over no_site_page
+        # instead of site_page
+        Group.objects.get(name="Site-wide editors").page_permissions.update(page_id=self.no_site_page.id)
+        self.assertTrue(self.client.login(username='siteeditor', password='password'))
+        response = self.client.get(reverse('wagtailadmin_explore', args=(self.site_page.id, )))
+        # Users without permission to explore here should be redirected to their explorable root.
+        self.assertEqual(
+            (response.status_code, response['Location']),
+            (302, reverse('wagtailadmin_explore', args=(self.no_site_page.pk, )))
+        )
+
 
 class TestExplorablePageVisibility(TestCase, WagtailTestUtils):
     """

wagtail/admin/views/pages.py

@@ -18,6 +18,7 @@ from django.views.generic import View
 
 from wagtail.admin import messages, signals
 from wagtail.admin.forms import CopyForm, SearchForm
+from wagtail.admin.navigation import get_explorable_root_page
 from wagtail.admin.utils import send_notification, user_has_any_page_permission, user_passes_test
 from wagtail.core import hooks
 from wagtail.core.models import Page, PageRevision, UserPagePermissionsProxy
@@ -34,9 +35,22 @@ def get_valid_next_url_from_request(request):
 @user_passes_test(user_has_any_page_permission)
 def index(request, parent_page_id=None):
     if parent_page_id:
-        parent_page = get_object_or_404(Page, id=parent_page_id).specific
+        parent_page = get_object_or_404(Page, id=parent_page_id)
     else:
-        parent_page = Page.get_first_root_node().specific
+        parent_page = Page.get_first_root_node()
+
+    # This will always succeed because of the @user_passes_test above.
+    root_page = get_explorable_root_page(request.user)
+
+    # If this page isn't a descendant of the user's explorable root page,
+    # then redirect to that explorable root page instead.
+    if not (
+        parent_page.pk == root_page.pk or
+        parent_page.is_descendant_of(root_page)
+    ):
+        return redirect('wagtailadmin_explore', root_page.pk)
+
+    parent_page = parent_page.specific
 
     pages = parent_page.get_children().prefetch_related('content_type', 'sites_rooted_here')