get_deleted_objects does not check permissions on proxy model objects without ModelAdmin

[anonymous]

When deleting an object through django admin interface, only related objects with Admin pages are checked for delete permissions. The following objects are NOT checked for delete permissions:

• proxy models with no ModelAdmin (even if the concrete model as an admin page)
• models with InlineAdmin

[Simon Charette]

The proxy model issue is somehow related to #11154 -- if proxy permissions were created we could check them just like any other model.

I'm not sure ​we should checks for delete permission on objects which model is not registered to the current admin site. If we don't why aren't we relying on ModelAdmin.has_delete_permission and do the same with inlines?

The only drawback is that you must register your model in order to get deletion permission checks. This should be documented at least.

IMO checks should be made this way:

1. If a ModelAdmin has been registered for this model or an InlineAdmin matches the relationship collected it should be used.
2. Else if the model is a proxy attempt 1. with the the model it's proxying (allow multiple levels of proxying).
3. Else fallback on user has_perm.

[Jerlo F. De Leon]

So, I have reproduced this on the current main branch and submitted a ​Pull Request

Following the excellent insight from Simon Charette (Comment 1)

This patch implements the suggested permission hierarchy:

1. Checks the concrete model's admin first for has_delete_permission.

2. If not found (or not registered), it recursively checks proxy levels.

3. It finally falls back to a global user permission check for the specific proxy.

The fix ensures get_deleted_objects correctly checks permissions for proxy models, even if they aren't registered in the admin.
I also added regression tests in tests/admin_utils/tests.py." which verifies that perms_needed correctly identifies missing proxy permissions to block unauthorized deletions.

[James Osgood]

As I remarked on the PR (​https://github.com/django/django/pull/21058#pullrequestreview-4134066520), following from the resolution of #11154, proxy model permissions use their own ContentType, rather than their parents' ContentType. So the concrete model check suggested by Simon Charette is no longer applicable.

The fallback to user.has_perm is still applicable, as it implements the missing part of "models that the user doesn’t have permission to delete" for perms_needed as mentioned in the ​docs for ModelAdmin.get_deleted_objects. However, this is a change in behaviour, rather than a strict bug-fix, as admin users may no longer be able to delete objects they previously could; so it must be documented as such.

Alternatively, if it is decided that the existing behaviour should be maintained, then the documentation for ModelAdmin.get_deleted_objects should be updated to specify that the permission check applies only to models registered in the admin. E.g.
"perms_needed is a set of verbose_names of the models registered in the admin that the user doesn’t have permission to delete."

Personally, I think the user.has_perm check is appropriate, as it matches the expectations set forth by the documentation. Not respecting the permissions for models being deleted seems like a security oversight.

[Jerlo F. De Leon]

I have opened a new PR (​https://github.com/django/django/pull/21127) to address the your feedback, as the original PR (​https://github.com/django/django/pull/21058) was closed by the GitHub Actions bot.

As you suggested, I moved the tests to tests.admin_views.test_actions.AdminActionsTest. The new test case now verifies that deleting a Persona (registered) is blocked when the user lacks delete permissions for the related FooAccount (not registered), confirming the expected HTML output and ensuring the deletion does not occur on POST.

I have also added the .. versionchanged:: documentation to ref/contrib/admin/index.txt to note the behavior change regarding permission checks for non-registered models.

[James Osgood]

Thank you Jerlo for making the new PR. The structure of your solution is much better, especially the documentation and test. I reviewed (​https://github.com/django/django/pull/21127#pullrequestreview-4135634159) - it needs just a bit more work.