KeyError on empty_form.fields in InlineAdminFormSet when user don't have 'add' permission for the child model

[Clément Mangin]

The following use case works with Django 2.0 but breaks with Django 2.1:

• A Parent model and a Child model with a ForeignKey to the Parent model,
• A Parent admin form with an inline formset for adding and editing Children,
• A user with change permission for the Parent and Child models, but no add permission for the Child model, trying to edit a Parent object and its children.

The following error occurs:

KeyError at /admin/demo/parent/1/change/
'name'
Request Method:	GET
Request URL:	/admin/demo/parent/1/change/
Django Version:	2.1
Exception Type:	KeyError
Exception Value:	
'name'
Exception Location:	.../python3.7/site-packages/django/contrib/admin/helpers.py in fields, line 287
Python Version:	3.7.0

Error during template rendering
In template .../python3.7/site-packages/django/contrib/admin/templates/admin/edit_inline/tabular.html, error at line 13

name
3	     data-inline-type="tabular"
4	     data-inline-formset="{{ inline_admin_formset.inline_formset_data }}">
5	  <div class="tabular inline-related {% if forloop.last %}last-related{% endif %}">
6	{{ inline_admin_formset.formset.management_form }}
7	<fieldset class="module {{ inline_admin_formset.classes }}">
8	   <h2>{{ inline_admin_formset.opts.verbose_name_plural|capfirst }}</h2>
9	   {{ inline_admin_formset.formset.non_form_errors }}
10	   <table>
11	     <thead><tr>
12	       <th class="original"></th>
13	     {% for field in inline_admin_formset.fields %}
14	       {% if not field.widget.is_hidden %}
15	         <th{% if field.required %} class="required"{% endif %}>{{ field.label|capfirst }}
16	         {% if field.help_text %}&nbsp;<img src="{% static "admin/img/icon-unknown.svg" %}" class="help help-tooltip" width="10" height="10" alt="({{ field.help_text|striptags }})" title="{{ field.help_text|striptags }}">{% endif %}
17	         </th>
18	       {% endif %}
19	     {% endfor %}
20	     {% if inline_admin_formset.formset.can_delete %}<th>{% trans "Delete?" %}</th>{% endif %}
21	     </tr></thead>
22	
23	     <tbody>

Trace:

Internal Server Error: /admin/demo/parent/1/change/
Traceback (most recent call last):
  File ".../python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File ".../python3.7/site-packages/django/core/handlers/base.py", line 156, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File ".../python3.7/site-packages/django/core/handlers/base.py", line 154, in _get_response
    response = response.render()
  File ".../python3.7/site-packages/django/template/response.py", line 106, in render
    self.content = self.rendered_content
  File ".../python3.7/site-packages/django/template/response.py", line 83, in rendered_content
    content = template.render(context, self._request)
  File ".../python3.7/site-packages/django/template/backends/django.py", line 61, in render
    return self.template.render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 171, in render
    return self._render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 163, in _render
    return self.nodelist.render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 937, in render
    bit = node.render_annotated(context)
  File ".../python3.7/site-packages/django/template/base.py", line 904, in render_annotated
    return self.render(context)
  File ".../python3.7/site-packages/django/template/loader_tags.py", line 150, in render
    return compiled_parent._render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 163, in _render
    return self.nodelist.render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 937, in render
    bit = node.render_annotated(context)
  File ".../python3.7/site-packages/django/template/base.py", line 904, in render_annotated
    return self.render(context)
  File ".../python3.7/site-packages/django/template/loader_tags.py", line 150, in render
    return compiled_parent._render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 163, in _render
    return self.nodelist.render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 937, in render
    bit = node.render_annotated(context)
  File ".../python3.7/site-packages/django/template/base.py", line 904, in render_annotated
    return self.render(context)
  File ".../python3.7/site-packages/django/template/loader_tags.py", line 62, in render
    result = block.nodelist.render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 937, in render
    bit = node.render_annotated(context)
  File ".../python3.7/site-packages/django/template/base.py", line 904, in render_annotated
    return self.render(context)
  File ".../python3.7/site-packages/django/template/loader_tags.py", line 62, in render
    result = block.nodelist.render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 937, in render
    bit = node.render_annotated(context)
  File ".../python3.7/site-packages/django/template/base.py", line 904, in render_annotated
    return self.render(context)
  File ".../python3.7/site-packages/django/template/defaulttags.py", line 209, in render
    nodelist.append(node.render_annotated(context))
  File ".../python3.7/site-packages/django/template/base.py", line 904, in render_annotated
    return self.render(context)
  File ".../python3.7/site-packages/django/template/loader_tags.py", line 188, in render
    return template.render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 173, in render
    return self._render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 163, in _render
    return self.nodelist.render(context)
  File ".../python3.7/site-packages/django/template/base.py", line 937, in render
    bit = node.render_annotated(context)
  File ".../python3.7/site-packages/django/template/base.py", line 904, in render_annotated
    return self.render(context)
  File ".../python3.7/site-packages/django/template/defaulttags.py", line 165, in render
    values = list(values)
  File ".../python3.7/site-packages/django/contrib/admin/helpers.py", line 287, in fields
    form_field = empty_form.fields[field_name]
KeyError: 'name'

GitHub project to see the difference in behaviour between Django 2.0 and Django 2.1 on this use case, using tox: ​https://github.com/clementmangin/django-21-error-demo

EDIT: potential origin of issue: ​https://github.com/django/django/commit/825f0beda804e48e9197fcf3b0d909f9f548aa47#diff-3c42de3e53aba87b32c494f995a728dfR2023

[Clément Mangin]

After running some tests, the exception occurs when using admin.TabularInline, but does not occur when using admin.StackedInline.

The Django tests suit shows the same error I get with tests test_inline_change_fk_change_del_perm and test_inline_change_fk_change_perm in admin_inlines.tests.TestInlinePermissions, after editing admin.InnerInline2 to inherit admin.TabularInline instead of admin.StackedInline.

[Tim Graham]

Regression test for Django's test suite

[Tim Graham]

Your "potential origin of issue" analysis seems correct. The "empty_form" in the traceback looks like an "add" form, so its fields are removed. A different approach to protecting against unauthorized edits may be needed.

[Clément Mangin]

Replying to Tim Graham:

Your "potential origin of issue" analysis seems correct. The "empty_form" in the traceback looks like an "add" form, so its fields are removed. A different approach to protecting against unauthorized edits may be needed.

Would self._meta.exclude = self._meta.fields[:] work, instead of self.fields = {}?
Because this fixes the crash and seems to achieve the same purpose, but I am not familiar enough with the codebase to trust this is a correct fix.

[Tim Graham]

django/contrib/admin/options.py

@@ -2058 +2058 @@
             def __init__(self, *args, **kwargs):
                 super(DeleteProtectedModelForm, self).__init__(*args, **kwargs)
                 if not can_change and not self.instance._state.adding:
-                    self.fields = {}
+                    self._meta.exclude = self._meta.fields[:]
                 if not can_add and self.instance._state.adding:
-                    self.fields = {}
+                    self._meta.exclude = self._meta.fields[:]
 
             def hand_clean_DELETE(self):
                 """

doesn't pass admin_views.tests.AdminInlineTests.test_simple_inline_permissions. I didn't investigate this approach in detail.

[Clément Mangin]

​PR

[Tim Graham <timograham@…>]

In 64e1a271:

Fixed #29637 -- Fixed admin change form crash if the user doesn’t have the add permission to a TabularInline.

Regression in 825f0beda804e48e9197fcf3b0d909f9f548aa47.

[Tim Graham <timograham@…>]

In d761567:

[2.1.x] Fixed #29637 -- Fixed admin change form crash if the user doesn’t have the add permission to a TabularInline.

Regression in 825f0beda804e48e9197fcf3b0d909f9f548aa47.

Backport of 64e1a271f50d921a54388539b6ff7102a31c3d29 from master