AdminSite login has_permission inconsistency

[nathan]

This is mainly a suggestion for refactoring but it may have security implications. Within the new AdminSite class you have a has_permission method and a login method.

has_permission checks for:
request.user.is_authenticated() and request.user.is_staff

while after authenticating login checks for:
if user.is_active and user.is_staff:

This results in a situation where a user may no longer be able to login but they still have permission to use the site, for example if a user were deactivated while logged in. Since the has_perm method of the user object checks if the user is active this doesn't seem to be too big of a problem under the current configuration, any attempt to access a specific object fails when the user.has_perm method is called. However this seems to be relying on a side effect, as the code evolves or for people subclassing the AdminSite real problems may result. Things could get ugly if someone were to override the has_permission method in a subclass and not the login method or vice versa.

I perhaps may not have dug deeply enough and there is a good reason for having two sets of tests that can be different but in general this seems to violate the DRY principle and open the door for future problems. It appears the code would be more robust if there were a single set of tests that both login and has_permission checked during verification. At the very least it would seem to make sense to explicitly check for user.is_active within the has_permission method rather than waiting for the admin model object to implicitly check it during an add/change/delete has_perm test.

Thank you,

[mredar]

This problem also limits the utility of custom AdminSites. I wanted to override the has_permission to allow non-staff users to access a limited customized AdminSite, while retaining the default admin site with full access for staff users. By using the has_permission test in both places, it will allow more custom usage of the admin interface.
Could also create a new function, perhaps 'has_access_permission', as separate test to see if the user can access the AdminSite at all? Subclasses will then have an easy way to control access to custom AdminSites.

[Ivan Sagalaev]

Patch

[Ivan Sagalaev]

Added a patch fixing all mentioned cases and a couple more in obscure places (special header generation).

[Adrian Holovaty]

(In [12159]) Fixed #8049 -- Fixed inconsistency in admin site is_active checks. Thanks for patch and tests, isagalaev