Properly check permissions when viewing revisions (#5490)

Currently any user with any page permission can view any page revision.
This commit fixes that, and ensures that the user has publish or edit
permission on the page before showing a page revision. If not, the user
is presented with a 403.

This maintains the current behavior if the user has no page permissions,
which is to redirect to the admin home page.

New tests have been added to cover these changes.

Fixes issue 5426.

CHANGELOG.txt

@@ -15,6 +15,7 @@ Changelog
  * Fix: Added labels to snippet bulk edit checkboxes for screen reader users (Martey Dodoo)
  * Fix: Middleware responses during page preview are now properly returned to the user (Matt Westcott)
  * Fix: Default text of page links in rich text uses the public page title rather than the admin display title (Andy Chosak)
+ * Fix: Specific page permission checks are now enforced when viewing a page revision (Andy Chosak)
 
 
 2.6.1 (05.08.2019)

docs/releases/2.7.rst

@@ -33,6 +33,7 @@ Bug fixes
  * Added labels to snippet bulk edit checkboxes for screen reader users (Martey Dodoo)
  * Middleware responses during page preview are now properly returned to the user (Matt Westcott)
  * Default text of page links in rich text uses the public page title rather than the admin display title (Andy Chosak)
+ * Specific page permission checks are now enforced when viewing a page revision (Andy Chosak)
 
 
 Upgrade considerations

wagtail/admin/tests/test_pages_views.py

@@ -4229,16 +4229,52 @@ class TestRevisions(TestCase, WagtailTestUtils):
         self.assertContains(response, this_christmas_preview_url)
         self.assertContains(response, this_christmas_revert_url)
 
-    def test_preview_revision(self):
+    def request_preview_revision(self):
         last_christmas_preview_url = reverse(
             'wagtailadmin_pages:revisions_view',
             args=(self.christmas_event.id, self.last_christmas_revision.id)
         )
-        response = self.client.get(last_christmas_preview_url)
-        self.assertEqual(response.status_code, 200)
+        return self.client.get(last_christmas_preview_url)
 
+    def test_preview_revision(self):
+        response = self.request_preview_revision()
+
+        self.assertEqual(response.status_code, 200)
         self.assertContains(response, "Last Christmas I gave you my heart")
 
+    def test_preview_revision_with_no_page_permissions_redirects_to_admin(self):
+        admin_only_user = get_user_model().objects.create_user(
+            username='admin_only',
+            email='admin_only@email.com',
+            password='password'
+        )
+        admin_only_user.user_permissions.add(
+            Permission.objects.get_by_natural_key(
+                codename='access_admin',
+                app_label='wagtailadmin',
+                model='admin'
+            )
+        )
+
+        self.login(user=admin_only_user)
+        response = self.request_preview_revision()
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response['Location'], reverse('wagtailadmin_home'))
+
+    def test_preview_revision_forbidden_without_permission(self):
+        # Alter the editors group so it has no permissions for Christmas page.
+        st_patricks = Page.objects.get(slug='saint-patrick')
+        editors_group = Group.objects.get(name='Site-wide editors')
+        editors_group.page_permissions.update(page_id=st_patricks.id)
+
+        editor = get_user_model().objects.get(username='siteeditor')
+
+        self.login(editor)
+        response = self.request_preview_revision()
+
+        self.assertEqual(response.status_code, 403)
+
     def test_revert_revision(self):
         last_christmas_preview_url = reverse(
             'wagtailadmin_pages:revisions_revert',

wagtail/admin/views/pages.py

@@ -1174,6 +1174,11 @@ def revisions_revert(request, page_id, revision_id):
 @user_passes_test(user_has_any_page_permission)
 def revisions_view(request, page_id, revision_id):
     page = get_object_or_404(Page, id=page_id).specific
+
+    perms = page.permissions_for_user(request.user)
+    if not (perms.can_publish() or perms.can_edit()):
+        raise PermissionDenied
+
     revision = get_object_or_404(page.revisions, id=revision_id)
     revision_page = revision.as_page_object()