Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #19838 -- Admin: Don't leak a 500 HTTP status when trying to de…

…lete protected FKs.

Thanks rafadev for the report and Javier Mansilla for the fix.
  • Loading branch information...
commit 3ea0c7d35ac461cb469170486683d10732eb534b 1 parent 51cc702
@jmansilla jmansilla authored ramiro committed
View
1  AUTHORS
@@ -363,6 +363,7 @@ answer newbie questions, and generally made Django that much better:
Mike Malone <mjmalone@gmail.com>
Martin Maney <http://www.chipy.org/Martin_Maney>
Michael Manfre <mmanfre@gmail.com>
+ Javier Mansilla <javimansilla@gmail.com>
masonsimon+django@gmail.com
Manuzhai
Petr Marhoun <petr.marhoun@gmail.com>
View
42 django/contrib/admin/options.py
@@ -3,12 +3,13 @@
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 @@ def get_formset(self, request, obj=None, **kwargs):
"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):
View
8 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):
View
40 tests/admin_inlines/tests.py
@@ -12,7 +12,7 @@
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 @@ def test_immutable_content_type(self):
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
Please sign in to comment.
Something went wrong with that request. Please try again.