Skip to content

Commit

Permalink
Improve filtering of audit logging based on the user's permissions
Browse files Browse the repository at this point in the history
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 wagtail#9181.
  • Loading branch information
th3hamm0r committed Oct 9, 2023
1 parent a2c9e9a commit bfe9e62
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 10 deletions.
102 changes: 102 additions & 0 deletions wagtail/admin/tests/test_reports_views.py
Expand Up @@ -13,6 +13,7 @@

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


Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -267,13 +283,30 @@ 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)

def assert_log_entries(self, response, expected):
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)
Expand All @@ -287,10 +320,64 @@ def test_unfiltered(self):
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(
Expand All @@ -303,6 +390,21 @@ def test_filter_by_action(self):
)

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(
Expand Down
12 changes: 11 additions & 1 deletion wagtail/admin/views/generic/history.py
Expand Up @@ -24,10 +24,16 @@
)


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"),
Expand All @@ -42,6 +48,10 @@ class Meta:
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"]
Expand Down
14 changes: 13 additions & 1 deletion wagtail/admin/views/pages/history.py
Expand Up @@ -13,10 +13,18 @@
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"),
Expand All @@ -41,6 +49,10 @@ class Meta:
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
Expand Down
33 changes: 26 additions & 7 deletions wagtail/admin/views/reports/audit_logging.py
Expand Up @@ -22,27 +22,40 @@
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"),
Expand All @@ -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):
Expand All @@ -83,6 +96,12 @@ class Meta:
"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"
Expand Down
39 changes: 38 additions & 1 deletion wagtail/models/audit_log.py
Expand Up @@ -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.core.serializers.json import DjangoJSONEncoder
Expand All @@ -21,6 +22,12 @@


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
Expand Down Expand Up @@ -130,7 +137,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.
Expand Down

0 comments on commit bfe9e62

Please sign in to comment.