[1.3.x] Checked object permissions on admin history view.

carljm · aaugustin · commit d3a45e10c8ac · 2013-02-12T12:13:42.000+01:00
This is a security fix. Disclosure and advisory coming shortly.

Patch by Russell Keith-Magee.
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
@@ -1242,15 +1242,21 @@ def delete_view(self, request, object_id, extra_context=None):
     def history_view(self, request, object_id, extra_context=None):
         "The 'history' admin view for this model."
         from django.contrib.admin.models import LogEntry
+        # First check if the user can see this history.
         model = self.model
+        obj = get_object_or_404(model, pk=unquote(object_id))
+
+        if not self.has_change_permission(request, obj):
+            raise PermissionDenied
+
+        # Then get the history for this object.
         opts = model._meta
         app_label = opts.app_label
         action_list = LogEntry.objects.filter(
             object_id = object_id,
             content_type__id__exact = ContentType.objects.get_for_model(model).id
         ).select_related().order_by('action_time')
-        # If no history was found, see whether this object even exists.
-        obj = get_object_or_404(model, pk=unquote(object_id))
+
         context = {
             'title': _('Change history: %s') % force_unicode(obj),
             'action_list': action_list,
diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py
@@ -827,6 +827,46 @@ def testChangeView(self):
             self.assertContains(request, 'login-form')
             self.client.get('/test_admin/admin/logout/')
 
+    def testHistoryView(self):
+        """History view should restrict access."""
+
+        # add user shoud not be able to view the list of article or change any of them
+        self.client.get('/test_admin/admin/')
+        self.client.post('/test_admin/admin/', self.adduser_login)
+        response = self.client.get('/test_admin/admin/admin_views/article/1/history/')
+        self.assertEqual(response.status_code, 403)
+        self.client.get('/test_admin/admin/logout/')
+
+        # change user can view all items and edit them
+        self.client.get('/test_admin/admin/')
+        self.client.post('/test_admin/admin/', self.changeuser_login)
+        response = self.client.get('/test_admin/admin/admin_views/article/1/history/')
+        self.assertEqual(response.status_code, 200)
+
+        # Test redirection when using row-level change permissions. Refs #11513.
+        RowLevelChangePermissionModel.objects.create(id=1, name="odd id")
+        RowLevelChangePermissionModel.objects.create(id=2, name="even id")
+        for login_dict in [self.super_login, self.changeuser_login, self.adduser_login, self.deleteuser_login]:
+            self.client.post('/test_admin/admin/', login_dict)
+            response = self.client.get('/test_admin/admin/admin_views/rowlevelchangepermissionmodel/1/history/')
+            self.assertEqual(response.status_code, 403)
+
+            response = self.client.get('/test_admin/admin/admin_views/rowlevelchangepermissionmodel/2/history/')
+            self.assertEqual(response.status_code, 200)
+
+            self.client.get('/test_admin/admin/logout/')
+
+        for login_dict in [self.joepublic_login, self.no_username_login]:
+            self.client.post('/test_admin/admin/', login_dict)
+            response = self.client.get('/test_admin/admin/admin_views/rowlevelchangepermissionmodel/1/history/')
+            self.assertEqual(response.status_code, 200)
+            self.assertContains(response, 'login-form')
+            response = self.client.get('/test_admin/admin/admin_views/rowlevelchangepermissionmodel/2/history/')
+            self.assertEqual(response.status_code, 200)
+            self.assertContains(response, 'login-form')
+
+            self.client.get('/test_admin/admin/logout/')
+
     def testConditionallyShowAddSectionLink(self):
         """
         The foreign key widget should only show the "add related" button if the
