Improve filtering of audit logging based on the user's permissions

Until now, a user could see the audit log for all (!) custom models,
permissions haven't been checked yet. This may disclose sensitive
information to unauthorized admin users.
Now, only log entries of those content types are displayed, where
the user has at least one permission.

This change also fixes an issue with the log entries for pages:
If the user only had access to specific parts of the sitetree, the audit
log still contained all entries of the ancestor pages which the user
actually couldn't view/edit.
For this, parts of the UserPagePermissionsProxy's explorable_pages()
have been extracted into a new viewable_pages() method.

Fixes wagtail#9181

wagtail/admin/tests/test_reports_views.py

@@ -3,16 +3,18 @@
 
 from django.conf import settings
 from django.conf.locale import LANG_INFO
-from django.contrib.auth.models import Permission
+from django.contrib.auth.models import Group, Permission
+from django.contrib.contenttypes.models import ContentType
 from django.test import TestCase
 from django.test.utils import override_settings
 from django.urls import reverse
 from django.utils import timezone, translation
 from openpyxl import load_workbook
 
 from wagtail.admin.views.mixins import ExcelDateFormatter
-from wagtail.models import Page, PageLogEntry
+from wagtail.models import GroupPagePermission, ModelLogEntry, Page, PageLogEntry
 from wagtail.test.utils import WagtailTestUtils
+from wagtail.test.testapp.models import Advert
 
 
 class TestLockedPagesView(TestCase, WagtailTestUtils):
@@ -186,8 +188,25 @@ class TestFilteredLogEntriesView(TestCase, WagtailTestUtils):
     def setUp(self):
         self.user = self.login()
         self.home_page = Page.objects.get(url_path="/home/")
+        self.sub_page = Page.objects.get(url_path="/home/events/")
+        self.custom_model = Advert.objects.get(pk=1)
 
-        self.create_log = PageLogEntry.objects.log_action(
+        # Create an editor which only has permissions for the sub_page (and below).
+        self.editor = self.create_user(
+            username="the_editor", email="the_editor@example.com", password="password"
+        )
+        sub_editors = Group.objects.create(name="Sub editors")
+        sub_editors.permissions.add(
+            Permission.objects.get(
+                content_type__app_label="wagtailadmin", codename="access_admin"
+            )
+        )
+        self.editor.groups.add(sub_editors)
+        GroupPagePermission.objects.create(
+            group=sub_editors, page=self.sub_page, permission_type="edit"
+        )
+
+        self.create_log_1 = PageLogEntry.objects.log_action(
             self.home_page, "wagtail.create"
         )
         self.edit_log_1 = PageLogEntry.objects.log_action(
@@ -199,6 +218,12 @@ def setUp(self):
         self.edit_log_3 = PageLogEntry.objects.log_action(
             self.home_page, "wagtail.edit"
         )
+        self.create_sub_log = PageLogEntry.objects.log_action(
+            self.sub_page, "wagtail.create"
+        )
+        self.edit_sub_log = PageLogEntry.objects.log_action(
+            self.sub_page, "wagtail.edit"
+        )
 
         self.create_comment_log = PageLogEntry.objects.log_action(
             self.home_page,
@@ -231,6 +256,16 @@ def setUp(self):
             },
         )
 
+        self.create_custom_log = ModelLogEntry.objects.log_action(
+            self.custom_model,
+            "wagtail.create",
+        )
+
+        self.edit_custom_log = ModelLogEntry.objects.log_action(
+            self.custom_model,
+            "wagtail.edit",
+        )
+
     def get(self, params={}):
         return self.client.get(reverse("wagtailadmin_reports:site_history"), params)
 
@@ -244,13 +279,29 @@ def test_unfiltered(self):
         self.assert_log_entries(
             response,
             [
-                self.create_log,
+                self.create_log_1,
                 self.edit_log_1,
                 self.edit_log_2,
                 self.edit_log_3,
+                self.create_sub_log,
+                self.edit_sub_log,
                 self.create_comment_log,
                 self.edit_comment_log,
                 self.create_reply_log,
+                self.create_custom_log,
+                self.edit_custom_log,
+            ],
+        )
+
+        # The editor should only see logs for the sub_page.
+        self.login(user=self.editor)
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assert_log_entries(
+            response,
+            [
+                self.create_sub_log,
+                self.edit_sub_log,
             ],
         )
 
@@ -263,6 +314,18 @@ def test_filter_by_action(self):
                 self.edit_log_1,
                 self.edit_log_2,
                 self.edit_log_3,
+                self.edit_sub_log,
+                self.edit_custom_log,
+            ],
+        )
+
+        self.login(user=self.editor)
+        response = self.get(params={"action": "wagtail.edit"})
+        self.assertEqual(response.status_code, 200)
+        self.assert_log_entries(
+            response,
+            [
+                self.edit_sub_log,
             ],
         )
 
@@ -272,10 +335,25 @@ def test_hide_commenting_actions(self):
         self.assert_log_entries(
             response,
             [
-                self.create_log,
+                self.create_log_1,
                 self.edit_log_1,
                 self.edit_log_2,
                 self.edit_log_3,
+                self.create_sub_log,
+                self.edit_sub_log,
+                self.create_custom_log,
+                self.edit_custom_log,
+            ],
+        )
+
+        self.login(user=self.editor)
+        response = self.get(params={"hide_commenting_actions": "on"})
+        self.assertEqual(response.status_code, 200)
+        self.assert_log_entries(
+            response,
+            [
+                self.create_sub_log,
+                self.edit_sub_log,
             ],
         )
 

wagtail/admin/views/reports/audit_logging.py

@@ -22,19 +22,21 @@
 from .base import ReportView
 
 
-def get_users_for_filter():
+def get_users_for_filter(user):
     user_ids = set()
     for log_model in log_action_registry.get_log_entry_models():
-        user_ids.update(log_model.objects.all().get_user_ids())
+        user_ids.update(log_model.objects.viewable_by_user(user).get_user_ids())
 
     User = get_user_model()
     return User.objects.filter(pk__in=user_ids).order_by(User.USERNAME_FIELD)
 
 
-def get_content_types_for_filter():
+def get_content_types_for_filter(user):
     content_type_ids = set()
     for log_model in log_action_registry.get_log_entry_models():
-        content_type_ids.update(log_model.objects.all().get_content_type_ids())
+        content_type_ids.update(
+            log_model.objects.viewable_by_user(user).get_content_type_ids()
+        )
 
     return ContentType.objects.filter(pk__in=content_type_ids).order_by("model")
 
@@ -56,12 +58,12 @@ class SiteHistoryReportFilterSet(WagtailFilterSet):
     user = django_filters.ModelChoiceFilter(
         label=_("User"),
         field_name="user",
-        queryset=lambda request: get_users_for_filter(),
+        queryset=lambda request: get_users_for_filter(request.user),
     )
     object_type = ContentTypeFilter(
         label=_("Type"),
         method="filter_object_type",
-        queryset=lambda request: get_content_types_for_filter(),
+        queryset=lambda request: get_content_types_for_filter(request.user),
     )
 
     def filter_hide_commenting_actions(self, queryset, name, value):

wagtail/models/__init__.py

@@ -2955,18 +2955,16 @@ def for_page(self, page):
         permission to perform specific tasks on the given page"""
         return PagePermissionTester(self, page)
 
-    def explorable_pages(self):
-        """Return a queryset of pages that the user has access to view in the
-        explorer (e.g. add/edit/publish permission). Includes all pages with
-        specific group permissions and also the ancestors of those pages (in
-        order to enable navigation in the explorer)"""
+    def viewable_pages(self):
+        """Return a queryset of pages that the user has access to view (e.g. add/edit/publish permission).
+        Includes all pages with specific group permissions."""
         # Deal with the trivial cases first...
         if not self.user.is_active:
             return Page.objects.none()
         if self.user.is_superuser:
             return Page.objects.all()
 
-        explorable_pages = Page.objects.none()
+        viewable_pages = Page.objects.none()
 
         # Creates a union queryset of all objects the user has access to add,
         # edit and publish
@@ -2976,7 +2974,22 @@ def explorable_pages(self):
             | Q(permission_type="publish")
             | Q(permission_type="lock")
         ):
-            explorable_pages |= Page.objects.descendant_of(perm.page, inclusive=True)
+            viewable_pages |= Page.objects.descendant_of(perm.page, inclusive=True)
+
+        return viewable_pages
+
+    def explorable_pages(self):
+        """Return a queryset of pages that the user has access to view in the
+        explorer (e.g. add/edit/publish permission). Includes all pages with
+        specific group permissions and also the ancestors of those pages (in
+        order to enable navigation in the explorer)"""
+        # Deal with the trivial cases first...
+        if not self.user.is_active:
+            return Page.objects.none()
+        if self.user.is_superuser:
+            return Page.objects.all()
+
+        explorable_pages = self.viewable_pages()
 
         # For all pages with specific permissions, add their ancestors as
         # explorable. This will allow deeply nested pages to be accessed in the
@@ -4407,7 +4420,7 @@ def log_action(self, instance, action, **kwargs):
     def viewable_by_user(self, user):
         q = Q(
             page__in=UserPagePermissionsProxy(user)
-            .explorable_pages()
+            .viewable_pages()
             .values_list("pk", flat=True)
         )
 

wagtail/models/audit_log.py

@@ -8,6 +8,7 @@
 
 from django.conf import settings
 from django.contrib.auth import get_user_model
+from django.contrib.auth.models import Permission
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ValidationError
 from django.db import models
@@ -117,7 +118,37 @@ def log_action(self, instance, action, **kwargs):
         )
 
     def viewable_by_user(self, user):
-        return self.all()
+        if user.is_superuser:
+            return self.all()
+
+        # This will be called multiple times per request, so we cache those ids once.
+        if not hasattr(user, "_allowed_content_type_ids"):
+            # 1) Only query those permissions, where log entries exist for their content
+            # types.
+            used_content_type_ids = self.values_list(
+                "content_type_id", flat=True
+            ).distinct()
+            permissions = Permission.objects.filter(
+                content_type_id__in=used_content_type_ids
+            )
+            # 2) If the user has at least one permission for a content type, we add its
+            # id to the allowed-set.
+            allowed_content_type_ids = set()
+            for permission in permissions:
+                if permission.content_type_id in allowed_content_type_ids:
+                    continue
+
+                content_type = ContentType.objects.get_for_id(
+                    permission.content_type_id
+                )
+                if user.has_perm(
+                    "%s.%s" % (content_type.app_label, permission.codename)
+                ):
+                    allowed_content_type_ids.add(permission.content_type_id)
+
+            user._allowed_content_type_ids = allowed_content_type_ids
+
+        return self.filter(content_type_id__in=user._allowed_content_type_ids)
 
     def get_for_model(self, model):
         # Return empty queryset if the given object is not valid.