From 62f3acc70a43a3c4f4970839d490ac8ea6c79047 Mon Sep 17 00:00:00 2001 From: Myk Willis Date: Sun, 24 Jan 2016 08:13:23 -0500 Subject: [PATCH] Fixed incorrect permissions check for admin's "Save as new". This is a security fix. --- django/contrib/admin/options.py | 10 ++++++---- docs/releases/1.9.2.txt | 13 +++++++++++-- tests/admin_views/tests.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 1bf2709dc0..7c5346df42 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1399,6 +1399,10 @@ class ModelAdmin(BaseModelAdmin): model = self.model opts = model._meta + + if request.method == 'POST' and '_saveasnew' in request.POST: + object_id = None + add = object_id is None if add: @@ -1416,10 +1420,6 @@ class ModelAdmin(BaseModelAdmin): raise Http404(_('%(name)s object with primary key %(key)r does not exist.') % { 'name': force_text(opts.verbose_name), 'key': escape(object_id)}) - if request.method == 'POST' and "_saveasnew" in request.POST: - object_id = None - obj = None - ModelForm = self.get_form(request, obj) if request.method == 'POST': form = ModelForm(request.POST, request.FILES, instance=obj) @@ -1482,6 +1482,8 @@ class ModelAdmin(BaseModelAdmin): if request.method == 'POST' and not form_validated and "_saveasnew" in request.POST: context['show_save'] = False context['show_save_and_continue'] = False + # Use the change template instead of the add template. + add = False context.update(extra_context or {}) diff --git a/docs/releases/1.9.2.txt b/docs/releases/1.9.2.txt index cfa04b703f..770f9f7341 100644 --- a/docs/releases/1.9.2.txt +++ b/docs/releases/1.9.2.txt @@ -4,8 +4,17 @@ Django 1.9.2 release notes *Under development* -Django 1.9.2 fixes several bugs in 1.9.1 and makes a small backwards -incompatible change that hopefully doesn't affect any users. +Django 1.9.2 fixes a security regression in 1.9 and several bugs in 1.9.1. It +also makes a small backwards incompatible change that hopefully doesn't affect +any users. + +Security issue: User with "change" but not "add" permission can create objects for ``ModelAdmin``’s with ``save_as=True`` +========================================================================================================================= + +If a ``ModelAdmin`` uses ``save_as=True`` (not the default), the admin +provides an option when editing objects to "Save as new". A regression in +Django 1.9 prevented that form submission from raising a "Permission Denied" +error for users without the "add" permission. Backwards incompatible change: ``.py-tpl`` files rewritten in project/app templates =================================================================================== diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index ae4c68cbb0..22a5d87231 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -1673,6 +1673,35 @@ class AdminViewPermissionsTest(TestCase): self.assertContains(response, 'login-form') self.client.get(reverse('admin:logout')) + def test_change_view_save_as_new(self): + """ + 'Save as new' should raise PermissionDenied for users without the 'add' + permission. + """ + change_dict_save_as_new = { + '_saveasnew': 'Save as new', + 'title': 'Ikke fordømt', + 'content': '

edited article

', + 'date_0': '2008-03-18', 'date_1': '10:54:39', + 'section': self.s1.pk, + } + article_change_url = reverse('admin:admin_views_article_change', args=(self.a1.pk,)) + + # Add user can perform "Save as new". + article_count = Article.objects.count() + self.client.force_login(self.adduser) + post = self.client.post(article_change_url, change_dict_save_as_new) + self.assertRedirects(post, self.index_url) + self.assertEqual(Article.objects.count(), article_count + 1) + self.client.logout() + + # Change user cannot perform "Save as new" (no 'add' permission). + article_count = Article.objects.count() + self.client.force_login(self.changeuser) + post = self.client.post(article_change_url, change_dict_save_as_new) + self.assertEqual(post.status_code, 403) + self.assertEqual(Article.objects.count(), article_count) + def test_delete_view(self): """Delete view should restrict access and actually delete items.""" delete_dict = {'post': 'yes'}