Merge pull request #1017 from erikvw/master

evaluate history model permissions explicitly

Author: ddabble

CHANGES.rst

@@ -4,6 +4,9 @@ Changes
 Unreleased
 ----------
 
+- Added feature to evaluate ``history`` model permissions explicitly when
+  ``SIMPLE_HISTORY_ENFORCE_HISTORY_MODEL_PERMISSIONS`` is set to ``True``
+  in ``settings`` (gh-1017).
 
 3.3.0 (2023-03-08)
 ------------------

docs/admin.rst

@@ -72,12 +72,88 @@ admin class
 Disabling the option to revert an object
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-By default, an object can be reverted to its previous version. To disable this option update your settings with the following:
+By default, an object can be reverted to its previous version. To disable this option
+globally, update your settings with the following:
 
 .. code-block:: python
 
-    SIMPLE_HISTORY_REVERT_DISABLED=True
+    SIMPLE_HISTORY_REVERT_DISABLED = True
 
 When ``SIMPLE_HISTORY_REVERT_DISABLED`` is set to ``True``, the revert button is removed from the form.
 
 .. image:: screens/10_revert_disabled.png
+
+Enforcing history model permissions in Admin
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+To make the Django admin site evaluate history model permissions explicitly,
+update your settings with the following:
+
+.. code-block:: python
+
+    SIMPLE_HISTORY_ENFORCE_HISTORY_MODEL_PERMISSIONS = True
+
+By default, ``SIMPLE_HISTORY_ENFORCE_HISTORY_MODEL_PERMISSIONS`` is set to ``False``.
+When set to ``False``, permissions applied to the ``Poll`` model
+(from the examples above), also apply to the history model.
+That is, granting view and change permissions to the ``Poll`` model
+implicitly grants view and change permissions to the ``Poll`` history model.
+
+The user below has view and change permissions to the ``Poll`` model and the ``Poll``
+history model in admin.
+
+.. code-block:: python
+
+    user.user_permissions.clear()
+    user.user_permissions.add(
+        Permission.objects.get(codename="view_poll"),
+        Permission.objects.get(codename="change_poll"),
+    )
+
+The user below has view permission to the ``Poll`` model and the ``Poll`` history model
+in admin.
+
+.. code-block:: python
+
+    user.user_permissions.clear()
+    user.user_permissions.add(
+        Permission.objects.get(codename="view_poll"),
+    )
+
+When ``SIMPLE_HISTORY_ENFORCE_HISTORY_MODEL_PERMISSIONS`` is set to ``True``,
+permissions to history models are assigned and evaluated explicitly.
+
+The user below *does not have* view permission to the ``Poll`` history model in admin,
+even though they *have* view permission to the ``Poll`` model.
+
+.. code-block:: python
+
+    # SIMPLE_HISTORY_ENFORCE_HISTORY_MODEL_PERMISSIONS = True in settings
+    user.user_permissions.clear()
+    user.user_permissions.add(
+        Permission.objects.get(codename="view_poll"),
+    )
+
+The user below has view permission to the ``Poll`` model and the ``Poll``
+history model.
+
+.. code-block:: python
+
+    # SIMPLE_HISTORY_ENFORCE_HISTORY_MODEL_PERMISSIONS = True in settings
+    user.user_permissions.clear()
+    user.user_permissions.add(
+        Permission.objects.get(codename="view_poll"),
+        Permission.objects.get(codename="view_historicalpoll"),
+    )
+
+The user below has view permission to the ``Poll`` history model, but will need to
+access the page with a direct URL, since the ``Poll`` model will not be listed on
+the admin application index page, nor the ``Poll`` changelist.
+
+.. code-block:: python
+
+    # SIMPLE_HISTORY_ENFORCE_HISTORY_MODEL_PERMISSIONS = True in settings
+    user.user_permissions.clear()
+    user.user_permissions.add(
+        Permission.objects.get(codename="view_historicalpoll"),
+    )

simple_history/admin.py

@@ -4,6 +4,7 @@
 from django.contrib import admin
 from django.contrib.admin import helpers
 from django.contrib.admin.utils import unquote
+from django.contrib.auth import get_permission_codename
 from django.core.exceptions import PermissionDenied
 from django.shortcuts import get_object_or_404, render
 from django.urls import re_path, reverse
@@ -12,7 +13,7 @@
 from django.utils.text import capfirst
 from django.utils.translation import gettext as _
 
-from . import utils
+from .utils import get_history_manager_for_model, get_history_model_for_model
 
 USER_NATURAL_KEY = tuple(key.lower() for key in settings.AUTH_USER_MODEL.split(".", 1))
 
@@ -61,7 +62,7 @@ def history_view(self, request, object_id, extra_context=None):
             except action_list.model.DoesNotExist:
                 raise http.Http404
 
-        if not self.has_change_permission(request, obj):
+        if not self.has_view_history_or_change_history_permission(request, obj):
             raise PermissionDenied
 
         # Set attribute on each action_list entry from admin methods
@@ -79,7 +80,7 @@ def history_view(self, request, object_id, extra_context=None):
             content_type.model,
         )
         context = {
-            "title": self.history_view_title(obj),
+            "title": self.history_view_title(request, obj),
             "action_list": action_list,
             "module_name": capfirst(force_str(opts.verbose_name_plural)),
             "object": obj,
@@ -88,7 +89,7 @@ def history_view(self, request, object_id, extra_context=None):
             "opts": opts,
             "admin_user_view": admin_user_view,
             "history_list_display": history_list_display,
-            "revert_disabled": self.revert_disabled,
+            "revert_disabled": self.revert_disabled(request, obj),
         }
         context.update(self.admin_site.each_context(request))
         context.update(extra_context or {})
@@ -97,8 +98,8 @@ def history_view(self, request, object_id, extra_context=None):
             request, self.object_history_template, context, **extra_kwargs
         )
 
-    def history_view_title(self, obj):
-        if self.revert_disabled and not SIMPLE_HISTORY_EDIT:
+    def history_view_title(self, request, obj):
+        if self.revert_disabled(request, obj) and not SIMPLE_HISTORY_EDIT:
             return _("View history: %s") % force_str(obj)
         else:
             return _("Change history: %s") % force_str(obj)
@@ -131,7 +132,7 @@ def history_form_view(self, request, object_id, version_id, extra_context=None):
         ).instance
         obj._state.adding = False
 
-        if not self.has_change_permission(request, obj):
+        if not self.has_view_history_or_change_history_permission(request, obj):
             raise PermissionDenied
 
         if SIMPLE_HISTORY_EDIT:
@@ -140,7 +141,7 @@ def history_form_view(self, request, object_id, version_id, extra_context=None):
             change_history = False
 
         if "_change_history" in request.POST and SIMPLE_HISTORY_EDIT:
-            history = utils.get_history_manager_for_model(obj)
+            history = get_history_manager_for_model(obj)
             obj = history.get(pk=version_id).instance
 
         formsets = []
@@ -173,7 +174,7 @@ def history_form_view(self, request, object_id, version_id, extra_context=None):
         model_name = original_opts.model_name
         url_triplet = self.admin_site.name, original_opts.app_label, model_name
         context = {
-            "title": self.history_form_view_title(obj),
+            "title": self.history_form_view_title(request, obj),
             "adminform": admin_form,
             "object_id": object_id,
             "original": obj,
@@ -186,12 +187,13 @@ def history_form_view(self, request, object_id, version_id, extra_context=None):
             "change_url": reverse("%s:%s_%s_change" % url_triplet, args=(obj.pk,)),
             "history_url": reverse("%s:%s_%s_history" % url_triplet, args=(obj.pk,)),
             "change_history": change_history,
-            "revert_disabled": self.revert_disabled,
+            "revert_disabled": self.revert_disabled(request, obj),
             # Context variables copied from render_change_form
             "add": False,
             "change": True,
             "has_add_permission": self.has_add_permission(request),
-            "has_change_permission": self.has_change_permission(request, obj),
+            "has_view_permission": self.has_view_history_permission(request, obj),
+            "has_change_permission": self.has_change_history_permission(request, obj),
             "has_delete_permission": self.has_delete_permission(request, obj),
             "has_file_field": True,
             "has_absolute_url": False,
@@ -211,8 +213,8 @@ def history_form_view(self, request, object_id, version_id, extra_context=None):
             request, self.object_history_form_template, context, **extra_kwargs
         )
 
-    def history_form_view_title(self, obj):
-        if self.revert_disabled:
+    def history_form_view_title(self, request, obj):
+        if self.revert_disabled(request, obj):
             return _("View %s") % force_str(obj)
         else:
             return _("Revert %s") % force_str(obj)
@@ -231,6 +233,54 @@ def content_type_model_cls(self):
         """Returns the ContentType model class."""
         return django_apps.get_model("contenttypes.contenttype")
 
+    def revert_disabled(self, request, obj=None):
+        """If `True`, hides the "Revert" button in the `submit_line.html` template."""
+        if getattr(settings, "SIMPLE_HISTORY_REVERT_DISABLED", False):
+            return True
+        elif self.has_view_history_permission(
+            request, obj
+        ) and not self.has_change_history_permission(request, obj):
+            return True
+        return False
+
+    def has_view_permission(self, request, obj=None):
+        return super().has_view_permission(request, obj)
+
+    def has_change_permission(self, request, obj=None):
+        return super().has_change_permission(request, obj)
+
+    def has_view_or_change_permission(self, request, obj=None):
+        return self.has_view_permission(request, obj) or self.has_change_permission(
+            request, obj
+        )
+
+    def has_view_history_or_change_history_permission(self, request, obj=None):
+        if self.enforce_history_permissions:
+            return self.has_view_history_permission(
+                request, obj
+            ) or self.has_change_history_permission(request, obj)
+        return self.has_view_or_change_permission(request, obj)
+
+    def has_view_history_permission(self, request, obj=None):
+        if self.enforce_history_permissions:
+            opts_history = get_history_model_for_model(self.model)._meta
+            codename_view_history = get_permission_codename("view", opts_history)
+            return request.user.has_perm(
+                f"{opts_history.app_label}.{codename_view_history}"
+            )
+        return self.has_view_permission(request, obj)
+
+    def has_change_history_permission(self, request, obj=None):
+        if self.enforce_history_permissions:
+            opts_history = get_history_model_for_model(self.model)._meta
+            codename_change_history = get_permission_codename("change", opts_history)
+            return request.user.has_perm(
+                f"{opts_history.app_label}.{codename_change_history}"
+            )
+        return self.has_change_permission(request, obj)
+
     @property
-    def revert_disabled(self):
-        return getattr(settings, "SIMPLE_HISTORY_REVERT_DISABLED", False)
+    def enforce_history_permissions(self):
+        return getattr(
+            settings, "SIMPLE_HISTORY_ENFORCE_HISTORY_MODEL_PERMISSIONS", False
+        )

simple_history/tests/admin.py

@@ -21,6 +21,9 @@ class PersonAdmin(SimpleHistoryAdmin):
     def has_change_permission(self, request, obj=None):
         return False
 
+    def has_view_permission(self, request, obj=None):
+        return False
+
 
 class ChoiceAdmin(SimpleHistoryAdmin):
     history_list_display = ["votes"]

simple_history/tests/models.py

@@ -549,6 +549,7 @@ def __str__(self):
 
     class Meta:
         verbose_name = "Planet"
+        verbose_name_plural = "Planets"
 
 
 class Contact(models.Model):