#26986 closed Cleanup/optimization (fixed)
`force_login` requires `user.is_active` in 1.10
Reported by: | Guillermo O. Freschi | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In 1.9, users didn't need to be active in order to be authenticated by force_login
in tests.
In 1.10 it is necessary because it's done through login
, which requires it.
It's either a regression or should be mentioned in the release notes (as per bmispelon from IRC).
Change History (8)
comment:1 by , 8 years ago
Component: | Testing framework → Documentation |
---|---|
Has patch: | set |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
comment:2 by , 8 years ago
I don't understand the report. Can you give example code to show the behavior change? The release note is referring to the removal of the user.is_active check in login(). As far as I see, force_login()
isn't affected by that.
comment:3 by , 8 years ago
I have a custom user model that is valid when its validation_date
is set.
Simplified:
class MyUser(AbstractBaseUser, PermissionsMixin): email = models.CharField(max_length=255, unique=True) validation_date = models.DateTimeField(null=True) USERNAME_FIELD = 'email' @property def is_active(self): return self.validation_date is not None
I have a test on a view that requires login, like
def test_user_must_be_logged_in(self): response = self.client.get("some.url") self.assertRedirects(response, "/login/?next=some.url") self.client.force_login(self.user) response = self.client.get("some.url") self.assertEqual(200, response.status_code)
This test worked in 1.9 but is broken in 1.10; the user is not logged in unless I assign a value to validation_date
and save (i.e., unless the user's is_active
returns True
).
If you require a complete example, just ask :)
comment:4 by , 8 years ago
Easy pickings: | unset |
---|
Okay, that makes sense but the current documentation proposal isn't technically correct since force_login()
already did delegate the decision to the authentication backend. The issue is rather that the default authentication backend now rejects inactive users.
Maybe you can propose some other clarification or we can close this ticket as a misunderstanding?
comment:5 by , 8 years ago
I see, that makes sense; this
* The new :class:`~django.contrib.auth.backends.AllowAllUsersModelBackend` and :class:`~django.contrib.auth.backends.AllowAllUsersRemoteUserBackend` ignore the value of ``User.is_active``, while :class:`~django.contrib.auth.backends.ModelBackend` and :class:`~django.contrib.auth.backends.RemoteUserBackend` now reject inactive users.
would be the relevant item in the changelog.
My issue in particular is the disconnect between the symptom (force_login
"stopped working") and the cause (the backend's behavior changed).
Would an item mentioning something like "test cases using force_login()
uses may need to be updated to use an active user" make sense?
Mentioning
force_login
in the release note would be nice, if only for grepping.There's a pull request at https://github.com/django/django/pull/7003.