Fixed CVE-2019-19118 -- Required edit permissions on parent model for editable inlines in admin.

Thank you to Shen Ying for reporting this issue.

5 files changed, 116 insertions, 76 deletions

diff --git a/tests/admin_inlines/tests.py b/tests/admin_inlines/tests.py
index fe0d913b0d..65ac36fd74 100644
--- a/tests/admin_inlines/tests.py
+++ b/tests/admin_inlines/tests.py
@@ -1,3 +1,5 @@
+from selenium.common.exceptions import NoSuchElementException
+
 from django.contrib.admin import ModelAdmin, TabularInline
 from django.contrib.admin.helpers import InlineAdminForm
 from django.contrib.admin.tests import AdminSeleniumTestCase
@@ -863,6 +865,98 @@ class TestInlinePermissions(TestCase):
 
 
 @override_settings(ROOT_URLCONF='admin_inlines.urls')
+class TestReadOnlyChangeViewInlinePermissions(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = User.objects.create_user('testing', password='password', is_staff=True)
+        cls.user.user_permissions.add(
+            Permission.objects.get(codename='view_poll', content_type=ContentType.objects.get_for_model(Poll))
+        )
+        cls.user.user_permissions.add(
+            *Permission.objects.filter(
+                codename__endswith="question", content_type=ContentType.objects.get_for_model(Question)
+            ).values_list('pk', flat=True)
+        )
+
+        cls.poll = Poll.objects.create(name="Survey")
+        cls.add_url = reverse('admin:admin_inlines_poll_add')
+        cls.change_url = reverse('admin:admin_inlines_poll_change', args=(cls.poll.id,))
+
+    def setUp(self):
+        self.client.force_login(self.user)
+
+    def test_add_url_not_allowed(self):
+        response = self.client.get(self.add_url)
+        self.assertEqual(response.status_code, 403)
+
+        response = self.client.post(self.add_url, {})
+        self.assertEqual(response.status_code, 403)
+
+    def test_post_to_change_url_not_allowed(self):
+        response = self.client.post(self.change_url, {})
+        self.assertEqual(response.status_code, 403)
+
+    def test_get_to_change_url_is_allowed(self):
+        response = self.client.get(self.change_url)
+        self.assertEqual(response.status_code, 200)
+
+    def test_main_model_is_rendered_as_read_only(self):
+        response = self.client.get(self.change_url)
+        self.assertContains(
+            response,
+            '<div class="readonly">%s</div>' % self.poll.name,
+            html=True
+        )
+        input = '<input type="text" name="name" value="%s" class="vTextField" maxlength="40" required id="id_name">'
+        self.assertNotContains(
+            response,
+            input % self.poll.name,
+            html=True
+        )
+
+    def test_inlines_are_rendered_as_read_only(self):
+        question = Question.objects.create(text="How will this be rendered?", poll=self.poll)
+        response = self.client.get(self.change_url)
+        self.assertContains(
+            response,
+            '<td class="field-text"><p>%s</p></td>' % question.text,
+            html=True
+        )
+        self.assertNotContains(response, 'id="id_question_set-0-text"')
+        self.assertNotContains(response, 'id="id_related_objs-0-DELETE"')
+
+    def test_submit_line_shows_only_close_button(self):
+        response = self.client.get(self.change_url)
+        self.assertContains(
+            response,
+            '<a href="/admin/admin_inlines/poll/" class="closelink">Close</a>',
+            html=True
+        )
+        delete_link = '<p class="deletelink-box"><a href="/admin/admin_inlines/poll/%s/delete/" class="deletelink">Delete</a></p>'  # noqa
+        self.assertNotContains(
+            response,
+            delete_link % self.poll.id,
+            html=True
+        )
+        self.assertNotContains(response, '<input type="submit" value="Save and add another" name="_addanother">')
+        self.assertNotContains(response, '<input type="submit" value="Save and continue editing" name="_continue">')
+
+    def test_inline_delete_buttons_are_not_shown(self):
+        Question.objects.create(text="How will this be rendered?", poll=self.poll)
+        response = self.client.get(self.change_url)
+        self.assertNotContains(
+            response,
+            '<input type="checkbox" name="question_set-0-DELETE" id="id_question_set-0-DELETE">',
+            html=True
+        )
+
+    def test_extra_inlines_are_not_shown(self):
+        response = self.client.get(self.change_url)
+        self.assertNotContains(response, 'id="id_question_set-0-text"')
+
+
+@override_settings(ROOT_URLCONF='admin_inlines.urls')
 class SeleniumTests(AdminSeleniumTestCase):
 
     available_apps = ['admin_inlines'] + AdminSeleniumTestCase.available_apps
@@ -965,6 +1059,24 @@ class SeleniumTests(AdminSeleniumTestCase):
         self.assertEqual(ProfileCollection.objects.all().count(), 1)
         self.assertEqual(Profile.objects.all().count(), 3)
 
+    def test_add_inline_link_absent_for_view_only_parent_model(self):
+        user = User.objects.create_user('testing', password='password', is_staff=True)
+        user.user_permissions.add(
+            Permission.objects.get(codename='view_poll', content_type=ContentType.objects.get_for_model(Poll))
+        )
+        user.user_permissions.add(
+            *Permission.objects.filter(
+                codename__endswith="question", content_type=ContentType.objects.get_for_model(Question)
+            ).values_list('pk', flat=True)
+        )
+        self.admin_login(username='testing', password='password')
+        poll = Poll.objects.create(name="Survey")
+        change_url = reverse('admin:admin_inlines_poll_change', args=(poll.id,))
+        self.selenium.get(self.live_server_url + change_url)
+        with self.disable_implicit_wait():
+            with self.assertRaises(NoSuchElementException):
+                self.selenium.find_element_by_link_text('Add another Question')
+
     def test_delete_inlines(self):
         self.admin_login(username='super', password='secret')
         self.selenium.get(self.live_server_url + reverse('admin:admin_inlines_profilecollection_add'))
diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py
index 4f39381783..ca326aab75 100644
--- a/tests/admin_views/admin.py
+++ b/tests/admin_views/admin.py
@@ -1168,12 +1168,3 @@ class ArticleAdmin9(admin.ModelAdmin):
 
 site9 = admin.AdminSite(name='admin9')
 site9.register(Article, ArticleAdmin9)
-
-
-class ArticleAdmin10(admin.ModelAdmin):
-    def has_change_permission(self, request, obj=None):
-        return False
-
-
-site10 = admin.AdminSite(name='admin10')
-site10.register(Article, ArticleAdmin10)
diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py
index 9709bcaf88..b046f37171 100644
--- a/tests/admin_views/tests.py
+++ b/tests/admin_views/tests.py
@@ -1794,8 +1794,7 @@ class AdminViewPermissionsTest(TestCase):
         self.assertEqual(post.status_code, 403)
         self.client.get(reverse('admin:logout'))
 
-        # view user should be able to view the article but not change any of them
-        # (the POST can be sent, but no modification occurs)
+        # view user can view articles but not make changes.
         self.client.force_login(self.viewuser)
         response = self.client.get(article_changelist_url)
         self.assertEqual(response.status_code, 200)
@@ -1806,7 +1805,7 @@ class AdminViewPermissionsTest(TestCase):
         self.assertContains(response, '<label>Extra form field:</label>')
         self.assertContains(response, '<a href="/test_admin/admin/admin_views/article/" class="closelink">Close</a>')
         post = self.client.post(article_change_url, change_dict)
-        self.assertEqual(post.status_code, 302)
+        self.assertEqual(post.status_code, 403)
         self.assertEqual(Article.objects.get(pk=self.a1.pk).content, '<p>Middle content</p>')
         self.client.get(reverse('admin:logout'))
 
@@ -1864,7 +1863,7 @@ class AdminViewPermissionsTest(TestCase):
                 response = self.client.get(change_url_3)
                 self.assertEqual(response.status_code, 200)
                 response = self.client.post(change_url_3, {'name': 'changed'})
-                self.assertRedirects(response, self.index_url)
+                self.assertEqual(response.status_code, 403)
                 self.assertEqual(RowLevelChangePermissionModel.objects.get(id=3).name, 'odd id mult 3')
                 response = self.client.get(change_url_6)
                 self.assertEqual(response.status_code, 200)
@@ -1901,21 +1900,6 @@ class AdminViewPermissionsTest(TestCase):
         self.assertEqual(response.context['title'], 'View article')
         self.assertContains(response, '<a href="/test_admin/admin9/admin_views/article/" class="closelink">Close</a>')
 
-    def test_change_view_post_without_object_change_permission(self):
-        """A POST redirects to changelist without modifications."""
-        change_dict = {
-            'title': 'Ikke fordømt',
-            'content': '<p>edited article</p>',
-            'date_0': '2008-03-18', 'date_1': '10:54:39',
-            'section': self.s1.pk,
-        }
-        change_url = reverse('admin10:admin_views_article_change', args=(self.a1.pk,))
-        changelist_url = reverse('admin10:admin_views_article_changelist')
-        self.client.force_login(self.viewuser)
-        response = self.client.post(change_url, change_dict)
-        self.assertRedirects(response, changelist_url)
-        self.assertEqual(Article.objects.get(pk=self.a1.pk).content, '<p>Middle content</p>')
-
     def test_change_view_save_as_new(self):
         """
         'Save as new' should raise PermissionDenied for users without the 'add'
@@ -4072,52 +4056,6 @@ class AdminInlineTests(TestCase):
         self.assertEqual(Widget.objects.count(), 1)
         self.assertEqual(Widget.objects.all()[0].name, "Widget 1 Updated")
 
-    def test_simple_inline_permissions(self):
-        """
-        Changes aren't allowed without change permissions for the inline object.
-        """
-        # User who can view Articles
-        permissionuser = User.objects.create_user(
-            username='permissionuser', password='secret',
-            email='vuser@example.com', is_staff=True,
-        )
-        permissionuser.user_permissions.add(get_perm(Collector, get_permission_codename('view', Collector._meta)))
-        permissionuser.user_permissions.add(get_perm(Widget, get_permission_codename('view', Widget._meta)))
-        self.client.force_login(permissionuser)
-        # Without add permission, a new inline can't be added.
-        self.post_data['widget_set-0-name'] = 'Widget 1'
-        collector_url = reverse('admin:admin_views_collector_change', args=(self.collector.pk,))
-        response = self.client.post(collector_url, self.post_data)
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(Widget.objects.count(), 0)
-        # But after adding the permission it can.
-        permissionuser.user_permissions.add(get_perm(Widget, get_permission_codename('add', Widget._meta)))
-        self.post_data['widget_set-0-name'] = "Widget 1"
-        collector_url = reverse('admin:admin_views_collector_change', args=(self.collector.pk,))
-        response = self.client.post(collector_url, self.post_data)
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(Widget.objects.count(), 1)
-        self.assertEqual(Widget.objects.first().name, 'Widget 1')
-        widget_id = Widget.objects.first().id
-        # Without the change permission, a POST doesn't change the object.
-        self.post_data['widget_set-INITIAL_FORMS'] = '1'
-        self.post_data['widget_set-0-id'] = str(widget_id)
-        self.post_data['widget_set-0-name'] = 'Widget 1 Updated'
-        response = self.client.post(collector_url, self.post_data)
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(Widget.objects.count(), 1)
-        self.assertEqual(Widget.objects.first().name, 'Widget 1')
-        # Now adding the change permission and editing works.
-        permissionuser.user_permissions.remove(get_perm(Widget, get_permission_codename('add', Widget._meta)))
-        permissionuser.user_permissions.add(get_perm(Widget, get_permission_codename('change', Widget._meta)))
-        self.post_data['widget_set-INITIAL_FORMS'] = '1'
-        self.post_data['widget_set-0-id'] = str(widget_id)
-        self.post_data['widget_set-0-name'] = 'Widget 1 Updated'
-        response = self.client.post(collector_url, self.post_data)
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(Widget.objects.count(), 1)
-        self.assertEqual(Widget.objects.first().name, 'Widget 1 Updated')
-
     def test_explicit_autofield_inline(self):
         "A model with an explicit autofield primary key can be saved as inlines. Regression for #8093"
         # First add a new inline
diff --git a/tests/admin_views/urls.py b/tests/admin_views/urls.py
index fdb61d759d..ca684b2f2e 100644
--- a/tests/admin_views/urls.py
+++ b/tests/admin_views/urls.py
@@ -17,7 +17,6 @@ urlpatterns = [
     # All admin views accept `extra_context` to allow adding it like this:
     path('test_admin/admin8/', (admin.site.get_urls(), 'admin', 'admin-extra-context'), {'extra_context': {}}),
     path('test_admin/admin9/', admin.site9.urls),
-    path('test_admin/admin10/', admin.site10.urls),
     path('test_admin/has_permission_admin/', custom_has_permission_admin.site.urls),
     path('test_admin/autocomplete_admin/', autocomplete_site.urls),
 ]
diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py
index 42acafd26d..ac39a79689 100644
--- a/tests/auth_tests/test_views.py
+++ b/tests/auth_tests/test_views.py
@@ -1262,7 +1262,7 @@ class ChangelistTests(AuthViewsTestCase):
         data['password'] = 'shouldnotchange'
         change_url = reverse('auth_test_admin:auth_user_change', args=(u.pk,))
         response = self.client.post(change_url, data)
-        self.assertRedirects(response, reverse('auth_test_admin:auth_user_changelist'))
+        self.assertEqual(response.status_code, 403)
         u.refresh_from_db()
         self.assertEqual(u.password, original_password)