AdminSite should rely on self.has_permission for all permission checks

[Brandon Konkle]

At the end of the login method on django.contrib.admin.sites.AdminSite , if the user data is correct the method checks for user.is_active and user.is_staff and then logs the user in. The admin_view method calls the has_permission method, which also checks for user.is_active and user.is_staff by default. Putting this into a separate method provides an extension point, however, to customize the permissions checked.

The fact that the login method doesn't check has_permission but checks is_active and is_staff explicitly is redundant and breaks the customization if a developer wants to create a separate, limited admin site where user.is_staff isn't a requirement. A use case would be in multi-tenancy situations, where users with a special permission may be able to access a site-specific admin site where they can only access data for their site.

[Ramiro Morales]

This the one of the issues that had been reported in #8049, unfortunately the solution implemented when fixing it wasn't to centralize the logic but rather to duplicate the if checks.

[Julien Phalip]

This has been fixed as a side effect of [14769].

[danny.adair@…]

I don't believe this has been fully resolved.
Even when AdminSite.has_permission() is overridden, it won't lead to the expected effect:

https://code.djangoproject.com/browser/django/trunk/django/contrib/admin/forms.py#L40
and
https://code.djangoproject.com/browser/django/trunk/django/contrib/admin/templates/admin/base.html#L26

still do explicit "is_staff" checks.

See also
​http://stackoverflow.com/questions/4022551/how-to-make-django-admin-site-accessed-by-non-staff-user/5573686#5573686

[danny.adair@…]

I don't know enough about the workings of AdminSite to create a patch, but wanted to leave a comment on what I observed during my recent investigations:

AdminSite.login_form is an AdminAuthenticationForm but the latter doesn't yet have an attribute to go the other direction and ask "its" AdminSite (more than one could share the same login_form...) for has_permission()... That could be passed into the constructor but I don't know the consequences - getting instantiated with request (cookies) and all... The template has the same problem. On a case-by-case basis I can swap one hardcoded piece of code with another, a generic solution is harder...

That said I think "only" those two locations need to be adjusted.

[danny.adair@…]

Regarding access to has_permission() in the form, maybe give AdminAuthenticationForm a class attribute admin_site=None which gets defaulted to a contrib.admin.sites.AdminSite if it stays None? Regarding admin/base.html I have no idea how it could be told its AdminSite...

[tanner]

duplicate of https://code.djangoproject.com/ticket/22295