Commit 3ea0c7d3 authored by Javier Mansilla's avatar Javier Mansilla Committed by Ramiro Morales
Browse files

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.
parent 51cc7029
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -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>
+40 −2
Original line number Diff line number Diff line
@@ -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):
+8 −0
Original line number Diff line number Diff line
@@ -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):
+39 −1
Original line number Diff line number Diff line
@@ -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