Fixed #37160 -- Made admin views raise PermissionDenied consistently.

The admin view_on_site and history views now return an HTTP 403 response
when a staff user lacks view or change permission for the target model,
consistent with the changeform and autocomplete views.

Thanks Bence Nagy for the report.

Author: nessita

django/contrib/admin/options.py

@@ -2563,14 +2563,14 @@ def history_view(self, request, object_id, extra_context=None):
         # First check if the user can see this history.
         model = self.model
         obj = self.get_object(request, unquote(object_id))
+        if not self.has_view_or_change_permission(request, obj):
+            raise PermissionDenied
+
         if obj is None:
             return self._get_obj_does_not_exist_redirect(
                 request, model._meta, object_id
             )
 
-        if not self.has_view_or_change_permission(request, obj):
-            raise PermissionDenied
-
         # Then get the history for this object.
         app_label = self.opts.app_label
         action_list = (

django/contrib/admin/sites.py

@@ -9,7 +9,7 @@
 from django.contrib.admin.views.autocomplete import AutocompleteJsonView
 from django.contrib.auth import REDIRECT_FIELD_NAME
 from django.contrib.auth.decorators import login_not_required
-from django.core.exceptions import ImproperlyConfigured
+from django.core.exceptions import ImproperlyConfigured, PermissionDenied
 from django.db.models.base import ModelBase
 from django.http import Http404, HttpResponsePermanentRedirect, HttpResponseRedirect
 from django.template.response import TemplateResponse
@@ -256,10 +256,6 @@ def inner(request, *args, **kwargs):
         return update_wrapper(inner, view)
 
     def get_urls(self):
-        # Since this module gets imported in the application's root package,
-        # it cannot import models from other applications at the module level,
-        # and django.contrib.contenttypes.views imports ContentType.
-        from django.contrib.contenttypes import views as contenttype_views
         from django.urls import include, path, re_path
 
         def wrap(view, cacheable=False):
@@ -290,7 +286,7 @@ def wrapper(*args, **kwargs):
             path("jsi18n/", wrap(self.i18n_javascript, cacheable=True), name="jsi18n"),
             path(
                 "r/<path:content_type_id>/<path:object_id>/",
-                wrap(contenttype_views.shortcut),
+                wrap(self.shortcut_view),
                 name="view_on_site",
             ),
         ]
@@ -451,6 +447,24 @@ def login(self, request, extra_context=None):
     def autocomplete_view(self, request):
         return AutocompleteJsonView.as_view(admin_site=self)(request)
 
+    def shortcut_view(self, request, content_type_id, object_id):
+        from django.contrib.contenttypes import views as contenttype_views
+        from django.contrib.contenttypes.models import ContentType
+
+        try:
+            content_type = ContentType.objects.get_for_id(int(content_type_id))
+        except (ContentType.DoesNotExist, ValueError):
+            pass
+        else:
+            model_class = content_type.model_class()
+            if (
+                model_class is not None
+                and self.is_registered(model_class)
+                and not self.get_model_admin(model_class).has_view_permission(request)
+            ):
+                raise PermissionDenied
+        return contenttype_views.shortcut(request, content_type_id, object_id)
+
     @no_append_slash
     def catch_all_view(self, request, url):
         if settings.APPEND_SLASH and not url.endswith("/"):

docs/releases/6.2.txt

@@ -266,6 +266,16 @@ backends.
 
 * ...
 
+:mod:`django.contrib.admin`
+---------------------------
+
+* The admin ``view_on_site`` URL now consistently returns an HTTP 403 response
+  when a staff user lacks view or change permission for the target model.
+
+* The admin history view now checks permissions before object existence,
+  consistently returning an HTTP 403 response for staff users without the view
+  or change permission regardless of whether the object exists.
+
 Miscellaneous
 -------------
 

tests/admin_views/tests.py

@@ -3380,6 +3380,24 @@ def test_history_view_bad_url(self):
             ["article with ID “foo” doesn’t exist. Perhaps it was deleted?"],
         )
 
+    def test_history_view_without_permission_returns_403(self):
+        self.client.force_login(self.adduser)
+        for label, pk in [("existing", self.a1.pk), ("missing", 999999)]:
+            with self.subTest(pk=label):
+                response = self.client.get(
+                    reverse("admin:admin_views_article_history", args=(pk,))
+                )
+                self.assertEqual(response.status_code, 403)
+
+    def test_history_view_with_view_or_change_permission_success(self):
+        for permission, user in [("view", self.viewuser), ("change", self.changeuser)]:
+            with self.subTest(permission=permission):
+                self.client.force_login(user)
+                response = self.client.get(
+                    reverse("admin:admin_views_article_history", args=(self.a1.pk,))
+                )
+                self.assertEqual(response.status_code, 200)
+
     def test_conditionally_show_add_section_link(self):
         """
         The foreign key widget should only show the "add related" button if the
@@ -3527,6 +3545,56 @@ def test_shortcut_view_only_available_to_staff(self):
         # Domain may depend on contrib.sites tests also run
         self.assertRegex(response.url, "http://(testserver|example.com)/dummy/foo/")
 
+    def test_shortcut_view_without_permission_returns_403(self):
+        obj = ModelWithStringPrimaryKey.objects.create(string_pk="bar")
+        model_ctype = ContentType.objects.get_for_model(ModelWithStringPrimaryKey)
+        shortcut_url = reverse("admin:view_on_site", args=(model_ctype.pk, obj.pk))
+        # deleteuser has no view permission on ModelWithStringPrimaryKey.
+        self.client.force_login(self.deleteuser)
+        response = self.client.get(shortcut_url)
+        self.assertEqual(response.status_code, 403)
+
+    def test_shortcut_view_with_view_or_change_permission_success(self):
+        obj = ModelWithStringPrimaryKey.objects.create(string_pk="bar")
+        model_ctype = ContentType.objects.get_for_model(ModelWithStringPrimaryKey)
+        shortcut_url = reverse("admin:view_on_site", args=(model_ctype.pk, obj.pk))
+        opts = ModelWithStringPrimaryKey._meta
+        for permission in ["view", "change"]:
+            codename = get_permission_codename(permission, opts)
+            perm = get_perm(ModelWithStringPrimaryKey, codename)
+            with self.subTest(permission=permission):
+                self.viewuser.user_permissions.set([perm])
+                self.client.force_login(self.viewuser)
+                response = self.client.get(shortcut_url)
+                self.assertEqual(response.status_code, 302)
+
+    def test_shortcut_view_for_invalid_content_type_returns_404(self):
+        # An unknown or non-int content type id skips the permission check and
+        # falls back to the contenttypes shortcut view, which raises Http404.
+        self.client.force_login(self.deleteuser)
+        for content_type_id in [9999, "not-an-int", None]:
+            with self.subTest(content_type_id=content_type_id):
+                response = self.client.get(
+                    reverse("admin:view_on_site", args=(content_type_id, 1))
+                )
+                self.assertEqual(response.status_code, 404)
+
+    def test_shortcut_view_does_not_repeat_content_type_query(self):
+        obj = ModelWithStringPrimaryKey.objects.create(string_pk="bar")
+        model_ctype = ContentType.objects.get_for_model(ModelWithStringPrimaryKey)
+        shortcut_url = reverse("admin:view_on_site", args=(model_ctype.pk, obj.pk))
+        self.client.force_login(self.superuser)
+        # A warmup request populates the ContentType and Site caches, so only
+        # relevant queries are measured. The 4 expected queries are:
+        # 1. Load the session.
+        # 2. Load the user.
+        # 3. Look up the content type.
+        # 4. Fetch the target instance.
+        self.client.get(shortcut_url)
+        with self.assertNumQueries(4):
+            response = self.client.get(shortcut_url)
+        self.assertEqual(response.status_code, 302)
+
     def test_has_module_permission(self):
         """
         has_module_permission() returns True for all users who