#35530: `django.contrib.auth.login` inconsistently guards `request.user`
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jaap Roes):
Thanks. The reason I filed this issue is because I tasked myself with
describing how Django's authentication and login flow works. This bit
stuck out as particularly confusing and basically unexplainable. I haven't
been able to come up with a rational use case where passing in `None` for
the `user` argument will make `login` behave in a way that I would expect.
I removed the confusing bit:
{{{
if user is None:
user = request.user
}}}
then ran the tests again.
The only test that breaks is
[https://github.com/django/django/blob/20c2d625d3d5062e43918d1d7b6f623202491dd4/tests/async/test_async_auth.py#L36-L43
async def test_alogin_without_user(self):], which is a test for the async
wrapper of this function.
Based on this test I have created another testcase that shows the issue
when `request.user` is `AnonymousUser` (which is common when
`AuthenticationMiddleware` is used).
{{{
async def test_alogin_without_user_anonymous_request(self):
request = HttpRequest()
request.user = AnonymousUser()
request.session = await self.client.asession()
await alogin(request, None)
user = await aget_user(request)
self.assertIsInstance(user, User)
self.assertEqual(user.username, self.test_user.username)
}}}
This will fail with an `AttributeError: 'AnonymousUser' object has no
attribute '_meta'`.
Another way this function will fail is when `request.user` is absent (i.e.
`AuthenticationMiddleware` is not in use):
{{{
async def test_alogin_without_user_or_request_user(self):
request = HttpRequest()
request.session = await self.client.asession()
await alogin(request, None)
user = await aget_user(request)
self.assertIsInstance(user, User)
self.assertEqual(user.username, self.test_user.username)
}}}
This will fail with an `AttributeError: 'HttpRequest' object has no
attribute 'user'`.
Setting `request.user = None` and passing in `user=None` will do the same
thing as just removing the `if user is None` test and fail with
`AttributeError: 'NoneType' object has no attribute '_meta'`.
There seems no real reason for this behaviour to exists. The only thing
touching this code in the Django code base is a recently added test for
the async wrapper. The code branch only works in very specific
circumstances, and does not fail gracefully if these circumstances are not
met.
Not sure if this is enough background to make you open this ticket again?
--
Ticket URL: <https://code.djangoproject.com/ticket/35530#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/01070190376a0373-0b8fee97-9a95-4efe-aefc-89d978d71544-000000%40eu-central-1.amazonses.com.
