Redundant is_active check in auth.backends.ModelBackend

[Tobias Bengfort]

See ​https://github.com/django/django/pull/11037

User.is_active is checked in ModelBackend.get_all_permissions(), ModelBackend.has_perm() and ModelBackend.has_module_perms(). I think the last two are redundant and should be removed.

timgraham had ​concerns though:

I'm unsure about removing the "redundant" is_active checks. It might be that some ModelBackend sublcasses rely on them. For example, if you subclass get_all_permissions() and omitting the is_active check (which wasn't there prior to Django 1.8 ​c33447a)... then your application would still have is_active checks in the other methods. This needs careful consideration and perhaps a discussion on the mailing list as to whether the benefits are worth the possible security issues. At least a release note is required. I'm not sure if this is required as part of the "BaseBackend" change, but I think it merits its own ticket.

My opinion is exactly the opposite: If a ModelBackend subclass does not check is_active in get_all_permissions() that is a bug and potentially even a security issue. The redundand checks hide these issues and therefore make it harder to find them.

I also think that the different methods should be consitent: If a permission is returned by get_all_permissions(), then checking that permission with has_perm() should return True. The only reason to do anything special in has_perm() is for performance optimizations.