[Django] #29419: Missing change in permissions for admin actions from release notes
Django
-----------------------------------------+------------------------
Reporter: Paulo | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
In the pr that introduced view only admin, there was also a change to
require user to have the "change" permission in order to render admin
actions.
This is a backwards incompatible change, as such, it would be good to have
it in https://docs.djangoproject.com/en/dev/releases/2.1/
--
Ticket URL: <https://code.djangoproject.com/ticket/29419>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
---------------------------------+------------------------------------
Reporter: Paulo | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Uncategorized => Bug
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
On second thought, I wonder if this is the best design decision. For
example, you would expect the default "deleted selected" bulk action to
require the delete permission rather than the change permission, right?
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:1>
Django
---------------------------------+------------------------------------
Type: Bug | Status: new
Component: Documentation | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hiroki Kiyohara):
I understood. You meant the behavior of ``ModelAdmin.get_actions`` has
changed, and it should be described as backward incompatible change.
https://github.com/django/django/blob/2.1a1/django/contrib/admin/options.py#L863-L865
I agree.
We need...
* Writing about it in the release note
* Adding "version changed" note here
https://docs.djangoproject.com/en/dev/ref/contrib/admin/actions/#django.contrib.admin.ModelAdmin.get_actions
Also I agree that the current design is not perfect.
* Actions like "CSV Download" requires only "view" permission, so
disabling all actions by "change" permission does not make sense for me
* Django should notice which permission is required by each actions
https://github.com/django/django/pull/5297#discussion_r42751084
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:2>
Django
Reporter: Paulo | Owner: Hiroki
| Kiyohara
Type: Bug | Status: assigned
Component: Documentation | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hiroki Kiyohara):
* owner: nobody => Hiroki Kiyohara
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Hiroki
| Kiyohara
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hiroki Kiyohara):
* has_patch: 0 => 1
Comment:
I opened the Pull Request https://github.com/django/django/pull/9970
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Hiroki
| Kiyohara
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Paulo):
I agree with giving this change another look.
It would be more flexible to leave the permissions handling to the actions
themselves.
Apps like django-tablib would no longer allow users to export without
permission to change objects.
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Hiroki
| Kiyohara
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
The previous behaviour here is to show all actions regardless of
permissions. If then you (for example) try to delete objects without the
delete permission you'll hit a permission denied error.
The ''idea'' in the PR is that actions won't be available to read-only
users, and that ''change'' will imply e.g. ''delete''. Whilst nice in
theory, neither of those seem to be true.
There's no current mechanism for actions to specify their permissions.
That might be a good addition as a separate feature. We could then filter
displayed actions on that basis.
For now, the consistent thing would seem to be to continue to display all
actions regardless of permission levels, allowing that users may run into
permission denied errors, as they will have up to now.
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Hiroki
| Kiyohara
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:7>
Django
right.
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Hiroki
| Kiyohara
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Carlton Gibson:
Old description:
> In the pr that introduced view only admin, there was also a change to
> require user to have the "change" permission in order to render admin
> actions.
>
> This is a backwards incompatible change, as such, it would be good to
> have it in https://docs.djangoproject.com/en/dev/releases/2.1/
New description:
In the pr that introduced view only admin, there was also a change to
require user to have the "change" permission in order to render admin
actions.
This is a backwards incompatible change, as such, it would be good to have
it in https://docs.djangoproject.com/en/dev/releases/2.1/
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:8>
Django
right.
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Hiroki
| Kiyohara
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* cc: Carlton Gibson (added)
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:9>
Django
right.
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Hiroki
| Kiyohara
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
[https://github.com/django/django/pull/10024 New PR] partially reverting
825f0beda804e48e9197fcf3b0d909f9f548aa47 to restore visibility of actions
to all staff users.
* Individual actions should check permissions.
* Additionally, users may follow the steps in
[https://docs.djangoproject.com/en/dev/ref/contrib/admin/actions
/#conditionally-enabling-or-disabling-actions Conditionally enabling or
disabling actions].
_Maybe_ we could add an `allowed_permissions` tuple to the action
functions — so `delete_selected` would have `(delete,)` — that could be
examined in the default implementation of `get_actions()`. (but that seems
like a New Feature beyond the scope of this one...)
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:10>
Django
right.
Reporter: Paulo | Owner: (none)
Type: Bug | Status: new
Component: Documentation | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hiroki Kiyohara):
* status: assigned => new
* owner: Hiroki Kiyohara => (none)
Comment:
I think Carlton Gibson should be assigned this issue, so I deassign
myself.
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:11>
Django
right.
Reporter: Paulo | Owner: Carlton
| Gibson
Type: Bug | Status: assigned
Component: Documentation | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => assigned
* owner: (none) => Carlton Gibson
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:12>
Django
right.
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Carlton
| Gibson
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* component: Documentation => contrib.admin
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:13>
Django
right.
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Carlton
| Gibson
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"b30f9b131c9489b9d9f21c311ecb46d0aea91381" b30f9b13]:
{{{
#!CommitTicketReference repository=""
revision="b30f9b131c9489b9d9f21c311ecb46d0aea91381"
Refs #29419, #8936 -- Removed change permission requirement for admin
actions.
Partially reverted 825f0beda804e48e9197fcf3b0d909f9f548aa47.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:14>
Django
right.
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Carlton
| Gibson
Type: Bug | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"aea0e2b9ca8376c1491f98262e20ed6069c70e5e" aea0e2b9]:
{{{
#!CommitTicketReference repository=""
revision="aea0e2b9ca8376c1491f98262e20ed6069c70e5e"
[2.1.x] Refs #29419, #8936 -- Removed change permission requirement for
admin actions.
Partially reverted 825f0beda804e48e9197fcf3b0d909f9f548aa47.
Backport of b30f9b131c9489b9d9f21c311ecb46d0aea91381 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Carlton
| Gibson
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* type: Bug => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Carlton
| Gibson
Type: New feature | Status: assigned
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 1 => 0
Comment:
OK, bar another review I think the PR is more or less there.
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Carlton
| Gibson
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"958c7b301ead79974db8edd5b9c6588a10a28ae7" 958c7b30]:
{{{
#!CommitTicketReference repository=""
revision="958c7b301ead79974db8edd5b9c6588a10a28ae7"
Fixed #29419 -- Allowed permissioning of admin actions.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: Paulo | Owner: Carlton
| Gibson
Type: New feature | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"306f1f8ea3e2b54e194a59ac0ecb686460f180e8" 306f1f8]:
{{{
#!CommitTicketReference repository=""
revision="306f1f8ea3e2b54e194a59ac0ecb686460f180e8"
[2.1.x] Fixed #29419 -- Allowed permissioning of admin actions.
Backport of 958c7b301ead79974db8edd5b9c6588a10a28ae7 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29419#comment:19>