Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29487 closed Bug (fixed)

Admin "read only" doesn't check change permission for object specifically

Reported by: Matthew Frazier Owned by: Paulo
Component: contrib.admin Version: 2.1
Severity: Release blocker Keywords:
Cc: Carlton Gibson Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

When determining which fields are read-only, ModelAdmin._changeform_view calls ModelAdmin.has_change_permission(request) without the object parameter - so, even if the user has read-only permission, only fields returned by get_readonly_fields() are included. However, subsequent calls to has_change_permission to build the form do use the object parameter - so, all fields are left out of the form, with the assumption that they are included in get_readonly_fields(). This leads to a traceback when rendering the template, since none of the fields are present in the form. (The traceback is included below.)

Changing has_change_permission(request) to has_change_permission(request, obj) on line 1580 of django/contrib/admin/options.py resolves the issue.

Request Method: GET
Request URL: http://localhost:8000/admin/.../.../.../change/

Django Version: 2.1a1
Python Version: 3.6.5

Template error:
In template .../django/contrib/admin/templates/admin/includes/fieldset.html, error at line 7
   Key 'full_name' not found in 'UserForm'. Choices are: .
   1 : <fieldset class="module aligned {{ fieldset.classes }}">
   2 :     {% if fieldset.name %}<h2>{{ fieldset.name }}</h2>{% endif %}
   3 :     {% if fieldset.description %}
   4 :         <div class="description">{{ fieldset.description|safe }}</div>
   5 :     {% endif %}
   6 :     {% for line in fieldset %}
   7 :         <div class="form-row{% if line.fields|length_is:'1' and line.errors %} errors{% endif %}{% if not line.has_visible_field %} hidden{% endif %} {% for field in line %} {% if field.field.name %} field-{{ field.field.name }}{% endif %}{% endfor %}">
   8 :             {% if line.fields|length_is:'1' %}{{ line.errors }}{% endif %}
   9 :             {% for field in line %}
   10 :                 <div{% if not line.fields|length_is:'1' %} class="fieldBox{% if field.field.name %} field-{{ field.field.name }}{% endif %}{% if not field.is_readonly and field.errors %} errors{% endif %}{% if field.field.is_hidden %} hidden{% endif %}"{% elif field.is_checkbox %} class="checkbox-row"{% endif %}>
   11 :                     {% if not line.fields|length_is:'1' and not field.is_readonly %}{{ field.errors }}{% endif %}
   12 :                     {% if field.is_checkbox %}
   13 :                         {{ field.field }}{{ field.label_tag }}
   14 :                     {% else %}
   15 :                         {{ field.label_tag }}
   16 :                         {% if field.is_readonly %}
   17 :                             <div class="readonly">{{ field.contents }}</div>


Traceback:

File ".../django/forms/forms.py" in __getitem__
  163.             field = self.fields[name]

During handling of the above exception ('full_name'), another exception occurred:

...

File ".../django/template/base.py" in render_annotated
  904.             return self.render(context)

File ".../django/template/defaulttags.py" in render
  165.                 values = list(values)

File ".../django/contrib/admin/helpers.py" in __iter__
  118.                 yield AdminField(self.form, field, is_first=(i == 0))

File ".../django/contrib/admin/helpers.py" in __init__
  130.         self.field = form[field]  # A django.forms.BoundField instance

File ".../django/forms/forms.py" in __getitem__
  169.                     ', '.join(sorted(f for f in self.fields)),

Exception Type: KeyError
Exception Value: "Key 'full_name' not found in 'UserForm'. Choices are: ."

Change History (8)

comment:1 by Tim Graham, 6 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:2 by Paulo, 6 years ago

Owner: changed from nobody to Paulo
Status: newassigned

comment:3 by Carlton Gibson, 6 years ago

Cc: Carlton Gibson added

comment:4 by Tim Graham, 6 years ago

Paulo, are you able to complete this before Monday's beta release?

comment:5 by Paulo, 6 years ago

I can prepare it today or tomorrow latest. Would that work for you?
My reply keeps getting marked as spam :/

comment:7 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 553617e6:

Fixed #29487 -- Accounted for object level permissions when calculating change view's read-only fields.

Thanks Matthew Frazier for the report and fix.

comment:8 by Tim Graham <timograham@…>, 6 years ago

In 8cbfaf29:

[2.1.x] Fixed #29487 -- Accounted for object level permissions when calculating change view's read-only fields.

Thanks Matthew Frazier for the report and fix.

Backport of 553617e61324dd5d9b34c47ceb2b6f20888daf20 from master

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