Fix PagePermissionPolicy to work with Django's Permission model

wagtail/admin/forms/account.py

@@ -26,7 +26,7 @@ class NotificationPreferencesForm(forms.ModelForm):
         permission_policy = PagePermissionPolicy()
         if not permission_policy.user_has_permission(self.instance.user, "publish"):
             del self.fields["submitted_notifications"]
-        if not permission_policy.user_has_permission(self.instance.user, "edit"):
+        if not permission_policy.user_has_permission(self.instance.user, "change"):
             del self.fields["approved_notifications"]
             del self.fields["rejected_notifications"]
             del self.fields["updated_comments_notifications"]

wagtail/admin/views/reports/locked_pages.py

@@ -48,7 +48,7 @@ class LockedPagesView(PageReportView):
         pages = (
             (
                 PagePermissionPolicy().instances_user_has_permission_for(
-                    self.request.user, "edit"
+                    self.request.user, "change"
                 )
                 | Page.objects.filter(locked_by=self.request.user)
             )

wagtail/admin/views/reports/workflows.py

@@ -37,7 +37,7 @@ def get_requested_by_queryset(request):
 
 def get_editable_page_ids_query(request):
     pages = PagePermissionPolicy().instances_user_has_permission_for(
-        request.user, "edit"
+        request.user, "change"
     )
     # Need to cast the page ids to string because Postgres doesn't support
     # implicit type casts when querying on GenericRelations

wagtail/admin/views/workflows.py

@@ -308,7 +308,7 @@ def usage(request, pk):
     workflow = get_object_or_404(Workflow, id=pk)
 
     editable_pages = PagePermissionPolicy().instances_user_has_permission_for(
-        request.user, "edit"
+        request.user, "change"
     )
     pages = workflow.all_pages() & editable_pages
     paginator = Paginator(pages, per_page=10)

wagtail/contrib/forms/utils.py

@@ -36,7 +36,7 @@ def get_forms_for_user(user):
     Return a queryset of form pages that this user is allowed to access the submissions for
     """
     editable_forms = PagePermissionPolicy().instances_user_has_permission_for(
-        user, "edit"
+        user, "change"
     )
     editable_forms = editable_forms.filter(content_type__in=get_form_types())
 

wagtail/contrib/modeladmin/helpers/permission.py

@@ -139,7 +139,7 @@ class PagePermissionHelper(PermissionHelper):
             perms = {
                 perm
                 for perm in PagePermissionPolicy().get_cached_permissions_for_user(user)
-                if perm.permission_type == "add"
+                if perm.permission.codename == "add_page"
             }
             for perm in perms:
                 # user has add permission on any subpage of perm.page

wagtail/models/__init__.py

@@ -2989,7 +2989,7 @@ class UserPagePermissionsProxy:
             stacklevel=2,
         )
         return self.permission_policy.instances_user_has_permission_for(
-            self.user, "edit"
+            self.user, "change"
         )
 
     def can_edit_pages(self):
@@ -3050,7 +3050,9 @@ class PagePermissionTester:
 
         if self.user.is_active and not self.user.is_superuser:
             self.permissions = {
-                perm.permission_type
+                # Get the 'action' part of the permission codename, e.g.
+                # 'add' instead of 'add_page'
+                perm.permission.codename.rsplit("_", maxsplit=1)[0]
                 for perm in self.permission_policy.get_cached_permissions_for_user(user)
                 if self.page.path.startswith(perm.page.path)
             }
@@ -3082,7 +3084,7 @@ class PagePermissionTester:
         if self.user.is_superuser:
             return True
 
-        if "edit" in self.permissions:
+        if "change" in self.permissions:
             return True
 
         if "add" in self.permissions and self.page.owner_id == self.user.pk:
@@ -3116,7 +3118,7 @@ class PagePermissionTester:
         ):
             return False
 
-        if "edit" in self.permissions:
+        if "change" in self.permissions:
             # if the user does not have publish permission, we also need to confirm that there
             # are no published pages here
             if "publish" not in self.permissions:

wagtail/permission_policies/pages.py

@@ -1,4 +1,4 @@
-from django.contrib.auth import get_user_model
+from django.contrib.auth import get_permission_codename, get_user_model
 from django.db.models import CharField, Q
 from django.db.models.functions import Cast
 
@@ -17,9 +17,12 @@ class PagePermissionPolicy(BasePermissionPolicy):
         if not user.is_active or user.is_anonymous or user.is_superuser:
             return GroupPagePermission.objects.none()
         return GroupPagePermission.objects.filter(group__user=user).select_related(
-            "page"
+            "page", "permission"
         )
 
+    def _get_permission_codenames(self, actions):
+        return {get_permission_codename(action, self.model._meta) for action in actions}
+
     def _base_user_has_permission(self, user):
         if not user.is_active:
             return False
@@ -44,22 +47,23 @@ class PagePermissionPolicy(BasePermissionPolicy):
 
         # User with only "add" permission can still edit their own pages
         actions = set(actions)
-        if "edit" in actions:
+        if "change" in actions:
             actions.add("add")
 
         permissions = {
-            perm.permission_type for perm in self.get_cached_permissions_for_user(user)
+            perm.permission.codename
+            for perm in self.get_cached_permissions_for_user(user)
         }
-        return bool(actions & permissions)
+        return bool(self._get_permission_codenames(actions) & permissions)
 
     def users_with_any_permission(self, actions, include_superusers=True):
         # User with only "add" permission can still edit their own pages
         actions = set(actions)
-        if "edit" in actions:
+        if "change" in actions:
             actions.add("add")
 
         groups = GroupPagePermission.objects.filter(
-            permission_type__in=actions
+            permission__codename__in=self._get_permission_codenames(actions)
         ).values_list("group", flat=True)
 
         q = Q(groups__in=groups)
@@ -87,11 +91,15 @@ class PagePermissionPolicy(BasePermissionPolicy):
         permissions = set()
         for perm in self.get_cached_permissions_for_user(user):
             if instance.pk == perm.page_id or instance.is_descendant_of(perm.page):
-                permissions.add(perm.permission_type)
-                if perm.permission_type == "add" and instance.owner_id == user.pk:
-                    permissions.add("edit")
+                permissions.add(perm.permission.codename)
+                if (
+                    perm.permission.codename
+                    == get_permission_codename("add", self.model._meta)
+                    and instance.owner_id == user.pk
+                ):
+                    permissions.add(get_permission_codename("change", self.model._meta))
 
-        return bool(set(actions) & permissions)
+        return bool(self._get_permission_codenames(actions) & permissions)
 
     def instances_user_has_any_permission_for(self, user, actions):
         base_queryset = self._base_queryset_for_user(user)
@@ -101,14 +109,15 @@ class PagePermissionPolicy(BasePermissionPolicy):
         pages = self.model._default_manager.none()
         for perm in self.get_cached_permissions_for_user(user):
             if (
-                perm.permission_type == "add"
+                perm.permission.codename
+                == get_permission_codename("add", self.model._meta)
                 and "add" not in actions
-                and "edit" in actions
+                and "change" in actions
             ):
                 pages |= self.model._default_manager.descendant_of(
                     perm.page, inclusive=True
                 ).filter(owner=user)
-            elif perm.permission_type in actions:
+            elif perm.permission.codename in self._get_permission_codenames(actions):
                 pages |= self.model._default_manager.descendant_of(
                     perm.page, inclusive=True
                 )
@@ -120,7 +129,8 @@ class PagePermissionPolicy(BasePermissionPolicy):
         # Find permissions for all ancestors that match any of the actions
         ancestors = instance.get_ancestors(inclusive=True)
         groups = GroupPagePermission.objects.filter(
-            permission_type__in=actions, page__in=ancestors
+            permission__codename__in=self._get_permission_codenames(actions),
+            page__in=ancestors,
         ).values_list("group", flat=True)
 
         q = Q(groups__in=groups)
@@ -128,12 +138,13 @@ class PagePermissionPolicy(BasePermissionPolicy):
         if include_superusers:
             q |= Q(is_superuser=True)
 
-        # If "edit" is in actions but "add" is not, then we need to check for
+        # If "change" is in actions but "add" is not, then we need to check for
         # cases where the user has "add" permission on an ancestor, and is the
         # owner of the instance
-        if "edit" in actions and "add" not in actions:
+        if "change" in actions and "add" not in actions:
             add_groups = GroupPagePermission.objects.filter(
-                permission_type="add", page__in=ancestors
+                permission__codename=get_permission_codename("add", self.model._meta),
+                page__in=ancestors,
             ).values_list("group", flat=True)
 
             q |= Q(groups__in=add_groups) & Q(pk=instance.owner_id)
@@ -153,15 +164,18 @@ class PagePermissionPolicy(BasePermissionPolicy):
         )
 
     def instances_with_direct_explore_permission(self, user):
-        # Get all pages that the user has direct add/edit/publish/lock permission on
+        # Get all pages that the user has direct add/change/publish/lock permission on
         if user.is_superuser:
             # superuser has implicit permission on the root node
             return Page.objects.filter(depth=1)
         else:
+            codenames = self._get_permission_codenames(
+                {"add", "change", "publish", "lock"}
+            )
             return [
                 perm.page
                 for perm in self.get_cached_permissions_for_user(user)
-                if perm.permission_type in {"add", "edit", "publish", "lock"}
+                if perm.permission.codename in codenames
             ]
 
     def explorable_root_instance(self, user):
@@ -185,13 +199,13 @@ class PagePermissionPolicy(BasePermissionPolicy):
             return base_queryset
 
         explorable_pages = self.instances_user_has_any_permission_for(
-            user, {"add", "edit", "publish", "lock"}
+            user, {"add", "change", "publish", "lock"}
         )
 
         # For all pages with specific permissions, add their ancestors as
         # explorable. This will allow deeply nested pages to be accessed in the
         # explorer. For example, in the hierarchy A>B>C>D where the user has
-        # 'edit' access on D, they will be able to navigate to D without having
+        # 'change' access on D, they will be able to navigate to D without having
         # explicit access to A, B or C.
         page_permissions = [
             perm.page for perm in self.get_cached_permissions_for_user(user)

wagtail/tests/test_page_permission_policies.py

@@ -1,7 +1,7 @@
-from django.contrib.auth.models import AnonymousUser, Group
+from django.contrib.auth.models import AnonymousUser, Group, Permission
 from django.test import TestCase
 
-from wagtail.models import GroupPagePermission, Page
+from wagtail.models import GroupPagePermission, Page, get_default_page_content_type
 from wagtail.permission_policies.pages import PagePermissionPolicy
 from wagtail.test.utils import WagtailTestUtils
 from wagtail.tests.test_permission_policies import PermissionPolicyTestUtils
@@ -9,6 +9,8 @@ from wagtail.tests.test_permission_policies import PermissionPolicyTestUtils
 
 class PermissionPolicyTestCase(PermissionPolicyTestUtils, WagtailTestUtils, TestCase):
     def setUp(self):
+        page_type = get_default_page_content_type()
+
         self.root_page = Page.objects.get(id=2)
 
         self.reports_page = self.root_page.add_child(
@@ -22,21 +24,27 @@ class PermissionPolicyTestCase(PermissionPolicyTestUtils, WagtailTestUtils, Test
         self.root_edit_perm = GroupPagePermission.objects.create(
             group=root_editors_group,
             page=self.root_page,
-            permission_type="edit",
+            permission=Permission.objects.get(
+                content_type=page_type, codename="change_page"
+            ),
         )
 
         report_editors_group = Group.objects.create(name="Report editors")
         self.report_edit_perm = GroupPagePermission.objects.create(
             group=report_editors_group,
             page=self.reports_page,
-            permission_type="edit",
+            permission=Permission.objects.get(
+                content_type=page_type, codename="change_page"
+            ),
         )
 
         report_adders_group = Group.objects.create(name="Report adders")
         self.report_add_perm = GroupPagePermission.objects.create(
             group=report_adders_group,
             page=self.reports_page,
-            permission_type="add",
+            permission=Permission.objects.get(
+                content_type=page_type, codename="add_page"
+            ),
         )
 
         # Users
@@ -183,34 +191,34 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
                 (self.useless_user, False, False, False, False),
                 (self.anonymous_user, False, False, False, False),
             ],
-            ["add", "edit", "delete", "frobnicate"],
+            ["add", "change", "delete", "frobnicate"],
         )
 
     def test_user_has_any_permission(self):
         self.assertTrue(
-            self.policy.user_has_any_permission(self.superuser, ["add", "edit"])
+            self.policy.user_has_any_permission(self.superuser, ["add", "change"])
         )
         self.assertFalse(
             self.policy.user_has_any_permission(
-                self.inactive_superuser, ["add", "edit"]
+                self.inactive_superuser, ["add", "change"]
             )
         )
         self.assertTrue(
-            self.policy.user_has_any_permission(self.report_editor, ["add", "edit"])
+            self.policy.user_has_any_permission(self.report_editor, ["add", "change"])
         )
         self.assertTrue(
-            self.policy.user_has_any_permission(self.report_adder, ["add", "edit"])
+            self.policy.user_has_any_permission(self.report_adder, ["add", "change"])
         )
         self.assertFalse(
-            self.policy.user_has_any_permission(self.anonymous_user, ["add", "edit"])
+            self.policy.user_has_any_permission(self.anonymous_user, ["add", "change"])
         )
         self.assertTrue(
-            self.policy.user_has_any_permission(self.report_adder, ["edit"])
+            self.policy.user_has_any_permission(self.report_adder, ["change"])
         )
 
     def test_users_with_any_permission(self):
         users_with_add_or_change_permission = self.policy.users_with_any_permission(
-            ["add", "edit"]
+            ["add", "change"]
         )
 
         self.assertResultSetEqual(
@@ -236,7 +244,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
         )
 
         users_with_edit_or_frobnicate_permission = (
-            self.policy.users_with_any_permission(["edit", "frobnicate"])
+            self.policy.users_with_any_permission(["change", "frobnicate"])
         )
 
         self.assertResultSetEqual(
@@ -250,7 +258,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
         )
 
     def test_users_with_permission(self):
-        users_with_change_permission = self.policy.users_with_permission("edit")
+        users_with_change_permission = self.policy.users_with_permission("change")
 
         self.assertResultSetEqual(
             users_with_change_permission,
@@ -286,7 +294,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
                 (self.useless_user, False, False, False),
                 (self.anonymous_user, False, False, False),
             ],
-            ["edit", "delete", "frobnicate"],
+            ["change", "delete", "frobnicate"],
         )
 
         # page in 'reports' is editable by users with permissions
@@ -303,31 +311,31 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
                 (self.useless_user, False, False, False),
                 (self.anonymous_user, False, False, False),
             ],
-            ["edit", "delete", "frobnicate"],
+            ["change", "delete", "frobnicate"],
         )
 
     def test_user_has_any_permission_for_instance(self):
         self.assertTrue(
             self.policy.user_has_any_permission_for_instance(
-                self.report_editor, ["edit", "delete"], self.useless_report
+                self.report_editor, ["change", "delete"], self.useless_report
             )
         )
 
         self.assertFalse(
             self.policy.user_has_any_permission_for_instance(
-                self.report_editor, ["edit", "delete"], self.editor_page
+                self.report_editor, ["change", "delete"], self.editor_page
             )
         )
 
         self.assertTrue(
             self.policy.user_has_any_permission_for_instance(
-                self.report_adder, ["edit", "delete"], self.adder_report
+                self.report_adder, ["change", "delete"], self.adder_report
             )
         )
 
         self.assertFalse(
             self.policy.user_has_any_permission_for_instance(
-                self.anonymous_user, ["edit", "delete"], self.editor_page
+                self.anonymous_user, ["change", "delete"], self.editor_page
             )
         )
 
@@ -335,7 +343,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
         self.assertResultSetEqual(
             self.policy.instances_user_has_permission_for(
                 self.superuser,
-                "edit",
+                "change",
             ),
             Page.objects.all(),
         )
@@ -343,7 +351,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
         self.assertResultSetEqual(
             self.policy.instances_user_has_permission_for(
                 self.inactive_superuser,
-                "edit",
+                "change",
             ),
             [],
         )
@@ -351,7 +359,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
         self.assertResultSetEqual(
             self.policy.instances_user_has_permission_for(
                 self.root_editor,
-                "edit",
+                "change",
             ),
             [
                 self.root_page,
@@ -367,7 +375,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
         self.assertResultSetEqual(
             self.policy.instances_user_has_permission_for(
                 self.report_editor,
-                "edit",
+                "change",
             ),
             [
                 self.reports_page,
@@ -381,7 +389,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
         self.assertResultSetEqual(
             self.policy.instances_user_has_permission_for(
                 self.useless_user,
-                "edit",
+                "change",
             ),
             [],
         )
@@ -389,7 +397,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
         self.assertResultSetEqual(
             self.policy.instances_user_has_permission_for(
                 self.anonymous_user,
-                "edit",
+                "change",
             ),
             [],
         )
@@ -397,21 +405,21 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
     def test_instances_user_has_any_permission_for(self):
         self.assertResultSetEqual(
             self.policy.instances_user_has_any_permission_for(
-                self.superuser, ["edit", "delete"]
+                self.superuser, ["change", "delete"]
             ),
             Page.objects.all(),
         )
 
         self.assertResultSetEqual(
             self.policy.instances_user_has_any_permission_for(
-                self.inactive_superuser, ["edit", "delete"]
+                self.inactive_superuser, ["change", "delete"]
             ),
             [],
         )
 
         self.assertResultSetEqual(
             self.policy.instances_user_has_any_permission_for(
-                self.root_editor, ["edit", "delete"]
+                self.root_editor, ["change", "delete"]
             ),
             [
                 self.root_page,
@@ -426,7 +434,7 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
 
         self.assertResultSetEqual(
             self.policy.instances_user_has_any_permission_for(
-                self.report_editor, ["edit", "delete"]
+                self.report_editor, ["change", "delete"]
             ),
             [
                 self.reports_page,
@@ -439,45 +447,49 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
 
         self.assertResultSetEqual(
             self.policy.instances_user_has_any_permission_for(
-                self.report_adder, ["edit", "delete"]
+                self.report_adder, ["change", "delete"]
             ),
             [self.adder_report],
         )
 
         self.assertResultSetEqual(
             self.policy.instances_user_has_any_permission_for(
-                self.useless_user, ["edit", "delete"]
+                self.useless_user, ["change", "delete"]
             ),
             [],
         )
 
         self.assertResultSetEqual(
             self.policy.instances_user_has_any_permission_for(
-                self.anonymous_user, ["edit", "delete"]
+                self.anonymous_user, ["change", "delete"]
             ),
             [],
         )
 
     def test_users_with_permission_for_instance(self):
         self.assertResultSetEqual(
-            self.policy.users_with_permission_for_instance("edit", self.editor_page),
+            self.policy.users_with_permission_for_instance("change", self.editor_page),
             [self.superuser, self.root_editor],
         )
         self.assertResultSetEqual(
-            self.policy.users_with_permission_for_instance("edit", self.adder_report),
+            self.policy.users_with_permission_for_instance("change", self.adder_report),
             [self.superuser, self.root_editor, self.report_editor, self.report_adder],
         )
         self.assertResultSetEqual(
-            self.policy.users_with_permission_for_instance("edit", self.editor_report),
-            [self.superuser, self.root_editor, self.report_editor],
-        )
-        self.assertResultSetEqual(
-            self.policy.users_with_permission_for_instance("edit", self.useless_report),
+            self.policy.users_with_permission_for_instance(
+                "change", self.editor_report
+            ),
             [self.superuser, self.root_editor, self.report_editor],
         )
         self.assertResultSetEqual(
             self.policy.users_with_permission_for_instance(
-                "edit", self.anonymous_report
+                "change", self.useless_report
+            ),
+            [self.superuser, self.root_editor, self.report_editor],
+        )
+        self.assertResultSetEqual(
+            self.policy.users_with_permission_for_instance(
+                "change", self.anonymous_report
             ),
             [self.superuser, self.root_editor, self.report_editor],
         )
@@ -485,19 +497,19 @@ class TestPagePermissionPolicy(PermissionPolicyTestCase):
     def test_users_with_any_permission_for_instance(self):
         self.assertResultSetEqual(
             self.policy.users_with_any_permission_for_instance(
-                ["edit", "delete"], self.editor_page
+                ["change", "delete"], self.editor_page
             ),
             [self.superuser, self.root_editor],
         )
         self.assertResultSetEqual(
             self.policy.users_with_any_permission_for_instance(
-                ["edit", "delete"], self.adder_report
+                ["change", "delete"], self.adder_report
             ),
             [self.superuser, self.root_editor, self.report_editor, self.report_adder],
         )
         self.assertResultSetEqual(
             self.policy.users_with_any_permission_for_instance(
-                ["edit", "delete"], self.useless_report
+                ["change", "delete"], self.useless_report
             ),
             [self.superuser, self.root_editor, self.report_editor],
         )