django.test.client.Client.login() checking for is_active attribute on User objects

[mrmagooey]

As per #13125 there has been a conscious decision to leave the is_active attribute as a hook for custom behaviour, and not used for built-in code. However, the login() method on django.test.client.Client objects checks for is_active. It would seem that this probably shouldn't be the case if the same logic used in the linked ticket for the login_required decorator is to be applied framework-wide.

[Nick Sandford]

Seems reasonable to me, would be good for consistency.

[Nick Sandford]

Added a pull request here: ​https://github.com/django/django/pull/718.

[Claude Paroz]

On one side, I understand the issue, and the is_active check might be inappropriate in the test client when using a custom auth backend.

On the other side, if the test client login method is meant to be equivalent to the contrib.auth.login view, we have a problem with this patch. The latter will refuse the login because it uses the AuthenticationForm which checks the is_active flag, while the former will let the user in (no more is_active check).

This might be solved by documenting the discrepancy between Client.login() and contrib.auth.login() (​https://docs.djangoproject.com/en/dev/topics/testing/overview/#django.test.client.Client.login). A release note would also be mandatory, as this would be slightly backwards-incompatible.

[Nick Sandford]

Hmm, on second thought, I guess the inconsistency lies with the @login_required decorator. If we expect the test client to work like the contrib.auth.login view, the is_active check should remain. Even with custom auth backends the Client.login() method works in much the same way as contrib.auth.login.

Though it is a bit confusing that Jacob has said that is_active is purely for custom auth sources and that the built-in stuff doesn't use it. Is that the expected behaviour and should the AuthenticationForm is_active check also be removed?

[Claude Paroz]

No, the is_active check cannot be removed in AuthenticationForm. I'm convinced that numerous sites are counting on this behaviour.

The login_required decorator is basically checking request.user.is_authenticated(), and a user with is_active=False will not be authenticated with standard contrib.auth.login.

I'd tend to won't fix this ticket, unless we get a convincing use case of removing this check.

[DjangoPanngo]

I also agree that removing the is_active check wouldn't be a good thing. As no convincing use cases has been raised, I'm setting this to wontfix.