Opened 2 years ago

Closed 2 years ago

Last modified 2 years 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 2 years ago.
Regression test for Django's test suite

Download all attachments as: .zip

Change History (11)

comment:1 Changed 2 years ago by Clément Mangin

Description: modified (diff)

comment:2 Changed 2 years 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 2 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Changed 2 years ago by Tim Graham

Attachment: 29637-test.diff added

Regression test for Django's test suite

comment:4 Changed 2 years 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 2 years 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 2 years 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 2 years ago by Clément Mangin

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

comment:8 Changed 2 years ago by Clément Mangin

Has patch: set

comment:9 Changed 2 years 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 2 years 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