id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
30284	Redundant is_active check in auth.backends.ModelBackend	Tobias Bengfort	nobody	"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 [https://github.com/django/django/pull/11037#pullrequestreview-217534553 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 [https://github.com/django/django/commit/c33447a50c1b0a96c6e2261f7c45d2522a3fe28d 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."	Cleanup/optimization	new	contrib.auth	dev	Normal				Unreviewed	1	0	0	0	0	0