Improve filtering of audit logging based on the user's permissions (#11020)

Until now, a user could see the audit log for all custom models,
permissions haven't been checked yet.
Now, only log entries of those content types are displayed, where
the user has at least one permission.

The change also adds filtering to the action-dropdown.
This avoids showing unnecessary action types, e.g. actions which aren't
used at all.
All 3 uses of action-dropdowns (site history, page history, generic
history) only display those actions, which are used at least once.

This partially fixes #9181.

CHANGELOG.txt

@@ -53,6 +53,7 @@ Changelog
  * Update all `FieldPanel('title')` examples to use the recommended `TitleFieldPanel('title')` panel (Chinedu Ihedioha)
  * The `purge_revisions` management command now respects revisions that have a `on_delete=PROTECT` foreign key relation and won't delete them (Neeraj P Yetheendran, Meghana Reddy, Sage Abdullah, Storm Heg)
  * Add support for Shift + Click behaviour in form submissions and simple tranlations submissions (LB (Ben) Johnston)
+ * Improve filtering of audit logging based on the user's permissions (Stefan Hammer)
  * Fix: Ensure that StreamField's `FieldBlock`s correctly set the `required` and `aria-describedby` attributes (Storm Heg)
  * Fix: Avoid an error when the moderation panel (admin dashboard) contains both snippets and private pages (Matt Westcott)
  * Fix: When deleting collections, ensure the collection name is correctly shown in the success message (LB (Ben) Johnston)

docs/releases/5.2.md

@@ -83,6 +83,7 @@ Thank you to core contributor (LB (Ben) Johnston) for writing this documentation
  * Implement breadcrumbs design refinements (Thibaud Colas)
  * The `purge_revisions` management command now respects revisions that have a `on_delete=PROTECT` foreign key relation and won't delete them (Neeraj P Yetheendran, Meghana Reddy, Sage Abdullah, Storm Heg)
  * Add support for Shift + Click behaviour in form submissions and simple tranlations submissions (LB (Ben) Johnston)
+ * Improve filtering of audit logging based on the user's permissions (Stefan Hammer)
 
 ### Bug fixes
 

wagtail/admin/tests/test_reports_views.py

@@ -13,6 +13,7 @@ from openpyxl import load_workbook
 
 from wagtail.admin.views.mixins import ExcelDateFormatter
 from wagtail.models import GroupPagePermission, ModelLogEntry, Page, PageLogEntry
+from wagtail.test.testapp.models import Advert
 from wagtail.test.utils import WagtailTestUtils
 
 
@@ -222,6 +223,21 @@ class TestFilteredLogEntriesView(WagtailTestUtils, TestCase):
     def setUp(self):
         self.user = self.login()
         self.home_page = Page.objects.get(url_path="/home/")
+        self.custom_model = Advert.objects.get(pk=1)
+
+        self.editor = self.create_user(
+            username="the_editor", email="the_editor@example.com", password="password"
+        )
+        editors = Group.objects.get(name="Editors")
+        editors.permissions.add(
+            Permission.objects.get(
+                content_type__app_label="wagtailadmin", codename="access_admin"
+            )
+        )
+        GroupPagePermission.objects.create(
+            group=editors, page=self.home_page, permission_type="change"
+        )
+        editors.user_set.add(self.editor)
 
         self.create_log = PageLogEntry.objects.log_action(
             self.home_page, "wagtail.create"
@@ -267,6 +283,16 @@ class TestFilteredLogEntriesView(WagtailTestUtils, TestCase):
             },
         )
 
+        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)
 
@@ -274,6 +300,13 @@ class TestFilteredLogEntriesView(WagtailTestUtils, TestCase):
         actual = set(response.context["object_list"])
         self.assertSetEqual(actual, set(expected))
 
+    def assert_filter_actions(self, response, expected):
+        actual = {
+            choice[0]
+            for choice in response.context["filters"].filters["action"].extra["choices"]
+        }
+        self.assertSetEqual(actual, set(expected))
+
     def test_unfiltered(self):
         response = self.get()
         self.assertEqual(response.status_code, 200)
@@ -287,10 +320,64 @@ class TestFilteredLogEntriesView(WagtailTestUtils, TestCase):
                 self.create_comment_log,
                 self.edit_comment_log,
                 self.create_reply_log,
+                self.create_custom_log,
+                self.edit_custom_log,
+            ],
+        )
+
+        self.assert_filter_actions(
+            response,
+            [
+                "wagtail.create",
+                "wagtail.edit",
+                "wagtail.comments.create",
+                "wagtail.comments.edit",
+                "wagtail.comments.create_reply",
+            ],
+        )
+
+        # The editor should not see the Advert's log entries.
+        self.login(user=self.editor)
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assert_log_entries(
+            response,
+            [
+                self.create_log,
+                self.edit_log_1,
+                self.edit_log_2,
+                self.edit_log_3,
+                self.create_comment_log,
+                self.edit_comment_log,
+                self.create_reply_log,
+            ],
+        )
+
+        self.assert_filter_actions(
+            response,
+            [
+                "wagtail.create",
+                "wagtail.edit",
+                "wagtail.comments.create",
+                "wagtail.comments.edit",
+                "wagtail.comments.create_reply",
             ],
         )
 
     def test_filter_by_action(self):
+        response = self.get(params={"action": "wagtail.edit"})
+        self.assertEqual(response.status_code, 200)
+        self.assert_log_entries(
+            response,
+            [
+                self.edit_log_1,
+                self.edit_log_2,
+                self.edit_log_3,
+                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(
@@ -303,6 +390,21 @@ class TestFilteredLogEntriesView(WagtailTestUtils, TestCase):
         )
 
     def test_hide_commenting_actions(self):
+        response = self.get(params={"hide_commenting_actions": "on"})
+        self.assertEqual(response.status_code, 200)
+        self.assert_log_entries(
+            response,
+            [
+                self.create_log,
+                self.edit_log_1,
+                self.edit_log_2,
+                self.edit_log_3,
+                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(

wagtail/admin/views/generic/history.py

@@ -24,10 +24,16 @@ from wagtail.models import (
 )
 
 
+def get_actions_for_filter():
+    # Only return those actions used by model log entries.
+    actions = set(ModelLogEntry.objects.all().get_actions())
+    return [action for action in log_registry.get_choices() if action[0] in actions]
+
+
 class HistoryReportFilterSet(WagtailFilterSet):
     action = django_filters.ChoiceFilter(
         label=gettext_lazy("Action"),
-        choices=log_registry.get_choices,
+        # choices are set dynamically in __init__()
     )
     user = django_filters.ModelChoiceFilter(
         label=gettext_lazy("User"),
@@ -42,6 +48,10 @@ class HistoryReportFilterSet(WagtailFilterSet):
         model = ModelLogEntry
         fields = ["action", "user", "timestamp"]
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.filters["action"].extra["choices"] = get_actions_for_filter()
+
 
 class HistoryView(IndexView):
     any_permission_required = ["add", "change", "delete"]

wagtail/admin/views/pages/history.py

@@ -13,10 +13,18 @@ from wagtail.log_actions import registry as log_action_registry
 from wagtail.models import Page, PageLogEntry
 
 
+def get_actions_for_filter():
+    # Only return those actions used by page log entries.
+    actions = set(PageLogEntry.objects.all().get_actions())
+    return [
+        action for action in log_action_registry.get_choices() if action[0] in actions
+    ]
+
+
 class PageHistoryReportFilterSet(WagtailFilterSet):
     action = django_filters.ChoiceFilter(
         label=_("Action"),
-        choices=log_action_registry.get_choices,
+        # choices are set dynamically in __init__()
     )
     hide_commenting_actions = django_filters.BooleanFilter(
         label=_("Hide commenting actions"),
@@ -41,6 +49,10 @@ class PageHistoryReportFilterSet(WagtailFilterSet):
         model = PageLogEntry
         fields = ["action", "user", "timestamp", "hide_commenting_actions"]
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.filters["action"].extra["choices"] = get_actions_for_filter()
+
 
 class PageWorkflowHistoryViewMixin:
     model = Page

wagtail/admin/views/reports/audit_logging.py

@@ -22,27 +22,40 @@ from wagtail.models import PageLogEntry
 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")
 
 
+def get_actions_for_filter(user):
+    # Only return those actions used by log entries visible to the user.
+    actions = set()
+    for log_model in log_action_registry.get_log_entry_models():
+        actions.update(log_model.objects.viewable_by_user(user).get_actions())
+
+    return [
+        action for action in log_action_registry.get_choices() if action[0] in actions
+    ]
+
+
 class SiteHistoryReportFilterSet(WagtailFilterSet):
     action = django_filters.ChoiceFilter(
         label=_("Action"),
-        choices=log_action_registry.get_choices,
+        # choices are set dynamically in __init__()
     )
     hide_commenting_actions = django_filters.BooleanFilter(
         label=_("Hide commenting actions"),
@@ -56,12 +69,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):
@@ -83,6 +96,12 @@ class SiteHistoryReportFilterSet(WagtailFilterSet):
             "hide_commenting_actions",
         ]
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.filters["action"].extra["choices"] = get_actions_for_filter(
+            self.request.user
+        )
+
 
 class LogEntriesView(ReportView):
     template_name = "wagtailadmin/reports/site_history.html"

wagtail/models/audit_log.py

@@ -8,6 +8,7 @@ from collections import defaultdict
 
 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.core.serializers.json import DjangoJSONEncoder
@@ -21,6 +22,12 @@ from wagtail.users.utils import get_deleted_user_display_name
 
 
 class LogEntryQuerySet(models.QuerySet):
+    def get_actions(self):
+        """
+        Returns a set of actions used by at least one log entry in this QuerySet
+        """
+        return set(self.order_by().values_list("action", flat=True).distinct())
+
     def get_user_ids(self):
         """
         Returns a set of user IDs of users who have created at least one log entry in this QuerySet
@@ -130,7 +137,37 @@ class BaseLogEntryManager(models.Manager):
         )
 
     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.