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 on all of these methods:

• get_user_permissions()
• get_group_permissions()
• get_all_permissions()
• has_perm()
• has_module_perms()

I think the last three 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.

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.