[Django] #30028: Uneditable object still editable through change_list if list_editable not empty
Django
empty
-----------------------------------------+----------------------------
Reporter: ksl | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 2.1
Severity: Normal | Keywords: changelist
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+----------------------------
= Abstract
This bug allows an object that should be uneditable (its
`has_change_permission` method always returns `False`) to be edited
through an editable changelist.
= Steps to reproduce
- Use the following admin:
{{{
class ArticleAdmin(models.ModelAdmin):
list_display = ("title", "author", "abstract")
list_editable = ("title", "author")
def has_change_permission(self, request, obj=None):
return False
}}}
- Navigate to the article changelist.
- Change any title/author field and save.
= Result
The modified article objects are indeed modified and saved to database.
= Expected result
The changelist view should (as does change form) display read-only fields
(ie: `span`s, not `input`s), and disallow any modification to be saved to
database.
= Technical information
Tested on Django 2.1.4.
--
Ticket URL: <https://code.djangoproject.com/ticket/30028>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => worksforme
Comment:
I can't reproduce this.
* For a superuser `list_editable` is working as expected.
* For a user with view-only permissions on the admin, `list_editable`
fields are **not** presented as form widgets. (As expected.)
* Any POST data submitted is not processed.
* Same adding `has_change_permission()` to always return `False`
* For superuser and view-only user, fields are not presented as
editable.
I'm going to close as-is. If you can provide an example project
reproducing this (perhaps with a frozen requirements files so we can see
the exact Django version) I'm happy to look again.
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:1>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by ksl):
Sorry, my bad.
The situation is actually more complex but boils down to the fact that
`has_change_permission` is called with `obj=None`.
This does not allow individual objects (rows) in the changelist to be
editable while others are not: either the whole changelist is editable or
it's not. Or am I missing something here?
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:2>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Can you put this into a project or a test case, so we can see it in
action?
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:3>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Happy to look at a project if you can provide one but just glancing at the
code, it looks like a programming error: you’re going to need to look at
the `request.user` to see what you should return. Otherwise you’ve
overridden the default implementation, which protects against this sort of
thing.
You should probably be calling `super()` before your own logic, and only
continuing if that returns `True`.
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:4>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
That looks like a duplicate of #15759 to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:5>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ksl):
* Attachment "django_test.tar.gz" added.
Test project
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ksl):
* Attachment "django_test.sql" added.
Test project postreSQL database dump
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by ksl):
Replying to [comment:4 Carlton Gibson]:
> Happy to look at a project if you can provide one but just glancing at
the code, it looks like a programming error: you’re going to need to look
at the `request.user` to see what you should return. Otherwise you’ve
overridden the default implementation, which protects against this sort of
thing, and created the issue.
>
> You should probably be calling `super()` before your own logic, and only
continuing if that returns `True`.
Please find enclosed a test project reflecting our situation. In this
project, the Question object with ID 1 should be the only one editable.
As you understand, our logic here is not based on per-user permission
(hence we do not use `request.user` nor do we call `super()`) but on
**per-object** permission.
Test project credentials:
* User `admin` with password `adminadmin`
* User `notadmin` with password `adminadmin`
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:6>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: worksforme
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by ksl):
Replying to [comment:5 Simon Charette]:
> That looks like a duplicate of #15759 to me.
Might be a duplicate indeed, except I'm not sure I understand the ''"if an
auth backend supports per-object permissions."'' correctly.
In our case, it's a matter of "if an object's `has_permission` returns
`False`".
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:7>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: duplicate
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* resolution: worksforme => duplicate
Comment:
Hi `ksl` — Thanks for the follow-up.
Looks like Simon's right about it being a Duplicate of #15759. With the
superuser all rows are shown as editable.
The view-only user behaviour looks correct though: No rows are shown as
editable if the user can only `view` the admin.
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:8>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: duplicate
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by ksl):
View-only user is actually the only one working as expected. Once you
empower a user to change objects of the model, the `has_change_permission`
logic is somewhat bypassed (or at least does not allow a per-object
logic).
Thank you, for taking precious time to answer.
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:9>
Django
empty
Reporter: ksl | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: duplicate
Keywords: changelist | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
No problem. Thank you for your report, and for the effort of making sure I
followed properly. 🙂
--
Ticket URL: <https://code.djangoproject.com/ticket/30028#comment:10>