diff --git a/CHANGELOG.txt b/CHANGELOG.txt index ae82dca274..37519fa48d 100644 --- a/CHANGELOG.txt +++ b/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) diff --git a/docs/releases/5.2.md b/docs/releases/5.2.md index 2f8e487b9d..09a4bb6c4c 100644 --- a/docs/releases/5.2.md +++ b/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 diff --git a/wagtail/admin/tests/test_reports_views.py b/wagtail/admin/tests/test_reports_views.py index 3fbe239281..c8a2195de8 100644 --- a/wagtail/admin/tests/test_reports_views.py +++ b/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( diff --git a/wagtail/admin/views/generic/history.py b/wagtail/admin/views/generic/history.py index e344ad1b1b..c79d9c0e18 100644 --- a/wagtail/admin/views/generic/history.py +++ b/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"] diff --git a/wagtail/admin/views/pages/history.py b/wagtail/admin/views/pages/history.py index 5236d9ed0c..e9c26b91f6 100644 --- a/wagtail/admin/views/pages/history.py +++ b/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 diff --git a/wagtail/admin/views/reports/audit_logging.py b/wagtail/admin/views/reports/audit_logging.py index 64e4d79bf8..f3dc2e27c2 100644 --- a/wagtail/admin/views/reports/audit_logging.py +++ b/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" diff --git a/wagtail/models/audit_log.py b/wagtail/models/audit_log.py index 4e19e3bfca..c5d068e0b7 100644 --- a/wagtail/models/audit_log.py +++ b/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.