Opened 20 months ago

Closed 20 months ago

Last modified 20 months ago

#29637 closed Bug (fixed)

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

Reported by: Clément Mangin Owned by: Clément Mangin
Component: contrib.admin Version: 2.1
Severity: Release blocker Keywords: admin, permission, inline
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by 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

Attachments (1)

29637-test.diff (5.7 KB) - added by Tim Graham 20 months ago.
Regression test for Django's test suite

Download all attachments as: .zip

Change History (11)

comment:1 Changed 20 months ago by Clément Mangin

Description: modified (diff)

comment:2 Changed 20 months ago by 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.

comment:3 Changed 20 months ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Changed 20 months ago by Tim Graham

Attachment: 29637-test.diff added

Regression test for Django's test suite

comment:4 Changed 20 months ago by 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.

comment:5 in reply to:  4 Changed 20 months ago by 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.

comment:6 Changed 20 months ago by Tim Graham

  • django/contrib/admin/options.py

    diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
    index 0f4dd93..9ff9842 100644
    a b class InlineModelAdmin(BaseModelAdmin): 
    20582058            def __init__(self, *args, **kwargs):
    20592059                super(DeleteProtectedModelForm, self).__init__(*args, **kwargs)
    20602060                if not can_change and not self.instance._state.adding:
    2061                     self.fields = {}
     2061                    self._meta.exclude = self._meta.fields[:]
    20622062                if not can_add and self.instance._state.adding:
    2063                     self.fields = {}
     2063                    self._meta.exclude = self._meta.fields[:]
    20642064
    20652065            def hand_clean_DELETE(self):
    20662066                """

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

comment:7 Changed 20 months ago by Clément Mangin

Owner: changed from nobody to Clément Mangin
Status: newassigned

comment:8 Changed 20 months ago by Clément Mangin

Has patch: set

comment:9 Changed 20 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 64e1a271:

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

Regression in 825f0beda804e48e9197fcf3b0d909f9f548aa47.

comment:10 Changed 20 months ago by 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

Note: See TracTickets for help on using tickets.
Back to Top