From 3ea0c7d35ac461cb469170486683d10732eb534b Mon Sep 17 00:00:00 2001 From: Javier Mansilla Date: Sat, 23 Feb 2013 15:44:54 -0300 Subject: [PATCH] Fixed #19838 -- Admin: Don't leak a 500 HTTP status when trying to delete protected FKs. Thanks rafadev for the report and Javier Mansilla for the fix. --- AUTHORS | 1 + django/contrib/admin/options.py | 42 +++++++++++++++++++++++++++++++-- tests/admin_inlines/models.py | 8 +++++++ tests/admin_inlines/tests.py | 40 ++++++++++++++++++++++++++++++- 4 files changed, 88 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index c603833f59..d59404e771 100644 --- a/AUTHORS +++ b/AUTHORS @@ -363,6 +363,7 @@ answer newbie questions, and generally made Django that much better: Mike Malone Martin Maney Michael Manfre + Javier Mansilla masonsimon+django@gmail.com Manuzhai Petr Marhoun diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index ff7873b40c..8f78aaf8dd 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -3,12 +3,13 @@ from functools import update_wrapper, partial from django import forms from django.conf import settings -from django.forms.formsets import all_valid +from django.forms.formsets import all_valid, DELETION_FIELD_NAME from django.forms.models import (modelform_factory, modelformset_factory, inlineformset_factory, BaseInlineFormSet) from django.contrib.contenttypes.models import ContentType from django.contrib.admin import widgets, helpers -from django.contrib.admin.util import unquote, flatten_fieldsets, get_deleted_objects, model_format_dict +from django.contrib.admin.util import (unquote, flatten_fieldsets, get_deleted_objects, + model_format_dict, NestedObjects) from django.contrib.admin.templatetags.admin_static import static from django.contrib import messages from django.views.decorators.csrf import csrf_protect @@ -1452,7 +1453,44 @@ class InlineModelAdmin(BaseModelAdmin): "max_num": self.max_num, "can_delete": can_delete, } + defaults.update(kwargs) + base_model_form = defaults['form'] + + class DeleteProtectedModelForm(base_model_form): + def hand_clean_DELETE(self): + """ + We don't validate the 'DELETE' field itself because on + templates it's not rendered using the field information, but + just using a generic "deletion_field" of the InlineModelAdmin. + """ + if self.cleaned_data.get(DELETION_FIELD_NAME, False): + using = router.db_for_write(self._meta.model) + collector = NestedObjects(using=using) + collector.collect([self.instance]) + if collector.protected: + objs = [] + for p in collector.protected: + objs.append( + # Translators: Model verbose name and instance representation, suitable to be an item in a list + _('%(class_name)s %(instance)s') % { + 'class_name': p._meta.verbose_name, + 'instance': p} + ) + msg_dict = {'class_name': self._meta.model._meta.verbose_name, + 'instance': self.instance, + 'related_objects': get_text_list(objs, _('and'))} + msg = _("Deleting %(class_name)s %(instance)s would require " + "deleting the following protected related objects: " + "%(related_objects)s") % msg_dict + raise ValidationError(msg) + + def is_valid(self): + result = super(DeleteProtectedModelForm, self).is_valid() + self.hand_clean_DELETE() + return result + + defaults['form'] = DeleteProtectedModelForm return inlineformset_factory(self.parent_model, self.model, **defaults) def get_fieldsets(self, request, obj=None): diff --git a/tests/admin_inlines/models.py b/tests/admin_inlines/models.py index 31f5ecc46c..82c1c3f078 100644 --- a/tests/admin_inlines/models.py +++ b/tests/admin_inlines/models.py @@ -128,8 +128,16 @@ class Novel(models.Model): name = models.CharField(max_length=40) class Chapter(models.Model): + name = models.CharField(max_length=40) novel = models.ForeignKey(Novel) +class FootNote(models.Model): + """ + Model added for ticket 19838 + """ + chapter = models.ForeignKey(Chapter, on_delete=models.PROTECT) + note = models.CharField(max_length=40) + # Models for #16838 class CapoFamiglia(models.Model): diff --git a/tests/admin_inlines/tests.py b/tests/admin_inlines/tests.py index 91962bd821..714a2f1c61 100644 --- a/tests/admin_inlines/tests.py +++ b/tests/admin_inlines/tests.py @@ -12,7 +12,7 @@ from .admin import InnerInline, TitleInline, site from .models import (Holder, Inner, Holder2, Inner2, Holder3, Inner3, Person, OutfitItem, Fashionista, Teacher, Parent, Child, Author, Book, Profile, ProfileCollection, ParentModelWithCustomPk, ChildModel1, ChildModel2, - Sighting, Title) + Sighting, Title, Novel, Chapter, FootNote) @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) @@ -250,6 +250,44 @@ class TestInlineAdminForm(TestCase): parent_ct = ContentType.objects.get_for_model(Parent) self.assertEqual(iaf.original.content_type, parent_ct) + +@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) +class TestInlineProtectedOnDelete(TestCase): + urls = "admin_inlines.urls" + fixtures = ['admin-views-users.xml'] + + def setUp(self): + result = self.client.login(username='super', password='secret') + self.assertEqual(result, True) + + def tearDown(self): + self.client.logout() + + def test_deleting_inline_with_protected_delete_does_not_validate(self): + lotr = Novel.objects.create(name='Lord of the rings') + chapter = Chapter.objects.create(novel=lotr, name='Many Meetings') + foot_note = FootNote.objects.create(chapter=chapter, note='yadda yadda') + + change_url = '/admin/admin_inlines/novel/%i/' % lotr.id + response = self.client.get(change_url) + data = { + 'name': lotr.name, + 'chapter_set-TOTAL_FORMS': 1, + 'chapter_set-INITIAL_FORMS': 1, + 'chapter_set-MAX_NUM_FORMS': 1000, + '_save': 'Save', + 'chapter_set-0-id': chapter.id, + 'chapter_set-0-name': chapter.name, + 'chapter_set-0-novel': lotr.id, + 'chapter_set-0-DELETE': 'on' + } + response = self.client.post(change_url, data) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Deleting chapter %s would require deleting " + "the following protected related objects: foot note %s" + % (chapter, foot_note)) + + class TestInlinePermissions(TestCase): """ Make sure the admin respects permissions for objects that are edited