[Django] #36300: request.META["HTTP_" + self.header] in RemoteUserMiddleware __acall__ does not sound correct

8 views
Skip to first unread message

Django

unread,
Apr 4, 2025, 11:46:07 AM4/4/25
to django-...@googlegroups.com
#36300: request.META["HTTP_" + self.header] in RemoteUserMiddleware __acall__ does
not sound correct
-------------------------------+-----------------------------------------
Reporter: Jan Pazdziora | Type: Uncategorized
Status: new | Component: contrib.auth
Version: 5.2 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
I've been investigating why https://github.com/adelton/django-identity-
external no longer works with Django 5.2. The
https://docs.djangoproject.com/en/5.2/releases/5.2/#django-contrib-auth
talks about new async auth functions. I have no idea if the async
functions are part of the problem I try to solve but it made me look at
the code changes.

The PR https://github.com/django/django/pull/18036 for
https://code.djangoproject.com/ticket/35303 added `__acall__` with code
{{{
+ try:
+ username = request.META["HTTP_" + self.header]
+ except KeyError:
+ # If specified header doesn't exist then remove any existing
+ # authenticated remote-user, or return (leaving request.user
set to
+ # AnonymousUser by the AuthenticationMiddleware).
}}}
among others.

However, the code in `__call__` (previously `process_request`) has code
{{{
try:
username = request.META[self.header]
except KeyError:
# If specified header doesn't exist then remove any existing
# authenticated remote-user, or return (leaving request.user
set to
# AnonymousUser by the AuthenticationMiddleware).
if self.force_logout_if_no_header and
request.user.is_authenticated:
}}}

Since they implement the same logic, the discrepancy is worrying. I
believe the `"HTTP_"` prefix is wrong -- if the user (admin) wants to
consume some HTTP header, let them configure the value with the `HTTP_`
prefix already.

This also shows that there don't seem tests covering the
`RemoteUserMiddleware`, or the problem would have been caught.
--
Ticket URL: <https://code.djangoproject.com/ticket/36300>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 4, 2025, 12:28:12 PM4/4/25
to django-...@googlegroups.com
#36300: request.META["HTTP_" + self.header] in RemoteUserMiddleware __acall__ does
not sound correct
-------------------------------+--------------------------------------
Reporter: Jan Pazdziora | Owner: (none)
Type: Uncategorized | Status: closed
Component: contrib.auth | Version: 5.2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by Sarah Boyce):

* cc: Jon Janzen, Carlton Gibson (added)
* resolution: => needsinfo
* status: new => closed

Comment:

There are tests such as
`tests.auth_tests.test_remote_user.RemoteUserTest.test_known_user_async`
If the HTTP prefix is removed, the tests fail.

You will need to demonstrate an issue that we can replicate so that we can
better understand the request here.

I think really this is a discussion around that WSGI adds a `HTTP_` prefix
but ASGI does not (see `HttpHeaders.to_wsgi_name` vs
`HttpHeaders.to_asgi_name`)
I will cc some other folks to the ticket in case they have thoughts to add
--
Ticket URL: <https://code.djangoproject.com/ticket/36300#comment:1>

Django

unread,
Apr 4, 2025, 1:19:05 PM4/4/25
to django-...@googlegroups.com
#36300: request.META["HTTP_" + self.header] in RemoteUserMiddleware __acall__ does
not sound correct
-------------------------------+--------------------------------------
Reporter: Jan Pazdziora | Owner: (none)
Type: Uncategorized | Status: closed
Component: contrib.auth | Version: 5.2
Severity: Normal | Resolution: needsinfo
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 Carlton Gibson):

I think the difference here is that WSGI servers generally set
`REMOTE_USER` in the environment, where the ASGI scope is parsed as
headers (so get the `HTTP_` prefix).

`needsinfo`: Yes. A clearer demonstration of why Django is at fault would
be needed here.

Unless `django-identity-external ` is using the new `__acall__` methods,
it's not clear why it should be affected. (I didn't see a tracking issue
on its repo.) Perhaps you're using it with an async app, and you're
hitting the new pathway? Maybe the
[https://docs.djangoproject.com/en/5.1/howto/auth-remote-user/#id1
`CustomHeaderRemoteUserMiddleware` example] in the docs would be what you
need, as a workaround?
--
Ticket URL: <https://code.djangoproject.com/ticket/36300#comment:2>
Reply all
Reply to author
Forward
0 new messages