Opened 3 years ago

Last modified 2 years ago

#18096 new Bug

Overiding delete permissions in the Admin

Reported by: anonymous Owned by: nobody
Component: contrib.admin Version: 1.4
Severity: Normal Keywords:
Cc: msopacua Triage Stage: Accepted
Has patch: no Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The ModelAdmin delete permission is controlled by the get_delete_permission function which can be overridden by subclasses. However when actually deleting an object the permission checking is passed off to util.get_deleted_objects and then in format callback it does this.

            p = '%s.%s' % (opts.app_label,
                           opts.get_delete_permission())

Which goes back to checking the permissions on the models meta class. Which makes whatever behavior you specified in get_delete_permission irrelevant.

Change History (7)

comment:1 Changed 3 years ago by ptone

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to needsinfo
  • Status changed from new to closed

I think you may be confusing get_delete_permission - which is an undocumented accessor that can return the permission label from models, with ModelAdmin.has_delete_permission which does the permission checking for the admin, and is what you would override in a ModelAdmin subclass

https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.has_delete_permission

If this was a typo - and in fact you are saying has_delete_permission is not behaving as documented - please feel free to reopen.

comment:2 Changed 3 years ago by anonymous

Yes this was a typo, has_delete_permission is the method. The default implementation calls opts.get_delete_permission(). The point that regardless of what you say in has_delete_permission the check in opt.get_delete_permission must pass otherwise the delete will not be allowed is still valid.

comment:3 Changed 3 years ago by anonymous

  • Resolution needsinfo deleted
  • Status changed from closed to reopened

comment:4 Changed 3 years ago by koenb

You will only encounter this when you relax the delete permission in your custom has_delete_permission method.

I am not convinced that doing so is a good design. In time you will be very confused that a user can delete objects while not having the proper permission to do so.

I would propose not fixing this in code, but adding a note to the documentation clarifying that the has_delete_permission method is meant to further restrict the delete permission (e.g. on a per object basis).

comment:5 Changed 3 years ago by msopacua

  • Cc msopacua added
  • Needs documentation set
  • Triage Stage changed from Unreviewed to Design decision needed

(Documentation needs updating.)

However, the reporter has a point that there is no way to relax permissions for which there is a common use case, the "owner of object" state, which can only be determined at runtime. For example, an editor will have delete/change permissions by default, but a reporter will only be able to edit it's own articles and may be denied editing when he submitted the work for review. Lastly, the grammar police can only edit an article when the article is submitted for final review. It is equally confusing (not to mention more work) to create artificial permissions on the model and separate views and templates in the admin for the model that bypass the standard change/delete mechanisms.

On the other hand, the admin does a good job but is meant to be replaced for more complicated projects / applications. Core team therefore needs to decide where this "more complicated" area starts.

comment:6 Changed 2 years ago by aaugustin

  • Status changed from reopened to new

comment:7 Changed 2 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

Agreed that it should be possible to relax permissions, so marking accepted.

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