#35555: Add additional middleware checks for django.contrib.auth
-------------------------------------+-------------------------------------
Reporter: Jaap Roes | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: auth session | Triage Stage:
middleware checks | 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 Mariusz for highlighting that contribution! I wasn't aware of
`admin.E410`. It's interesting, the current implementation of
[https://github.com/django/django/blob/e56a32b89bb7fadffdfaa2cdf12b4863ccd5af9b/django/contrib/admin/checks.py#L158-L173
admin.E410] has a hint that almost mirrors the message of the runtime
check in
[https://github.com/django/django/blob/f1705c8780c0a7587654fc736542d55fe4a7f29b/django/contrib/auth/middleware.py#L27-L38
AuthenticationMiddleware].
The checks for `AuthenticationMiddleware`
([https://github.com/django/django/blob/e56a32b89bb7fadffdfaa2cdf12b4863ccd5af9b/django/contrib/admin/checks.py#L138-L147
admin.E408] and
[https://github.com/django/django/blob/e56a32b89bb7fadffdfaa2cdf12b4863ccd5af9b/django/contrib/auth/checks.py#L240-L262
auth.E013]) have the same goal as the runtime check in
[https://github.com/django/django/blob/f1705c8780c0a7587654fc736542d55fe4a7f29b/django/contrib/auth/middleware.py#L91-L120
RemoteUserMiddleware]. They all want `request.user` to exist, i.e. for
`AuthenticationMiddleware` to be enabled.
I do agree that there is a fundamental difference between a duck-type
check and a subclass check. Currently Django inconsistently applies both
to achieve the same goal.
In the cases of
[https://github.com/django/django/blob/f1705c8780c0a7587654fc736542d55fe4a7f29b/django/contrib/auth/middleware.py#L91-L120
RemoteUserMiddleware] and
[https://github.com/django/django/blob/f1705c8780c0a7587654fc736542d55fe4a7f29b/django/contrib/auth/middleware.py#L27-L38
AuthenticationMiddleware], both error message point to the exact
middleware class that is expected to be enabled. So while alternative
implementations may be able to pass the current implementation, it isn't
in really the true spirit of these checks.
As Tim identified, the major downside to adding these checks is that some
user might have to add these checks to `SILENCED_SYSTEM_CHECKS`. The
upside is that Django is more consistent (and helpful) when configuring
`contrib.auth`. Currently `contrib.auth` takes some of that
responsibility, but not every project uses that.
I've prepared a pull request on GitHub, to make it easier to see how
similar all these cases are. Sadly this ticket was just closed as WONTFIX
while doing this. I hope someone will still take the time to take a look
at it.
--
Ticket URL: <https://code.djangoproject.com/ticket/35555#comment:11>
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/0107019056c7b6b4-22fdf34d-ce3c-4012-93fe-9ec0311c1d5e-000000%40eu-central-1.amazonses.com.
