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

[Jan Pazdziora]

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.

[Sarah Boyce]

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

[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 ​`CustomHeaderRemoteUserMiddleware` example in the docs would be what you need, as a workaround?

[Jacob Walls]

Replying to Jan Pazdziora:

This also shows that there don't seem tests covering the RemoteUserMiddleware, or the problem would have been caught.

As developed in ticket:37025#comment:19, this is essentially right -- we had tests, but they were written incorrectly.

It's a regression in 50f89ae850f6b4e35819fe725a08c7e579bfd099 with respect to custom headers. (The effort to support the default header under ASGI changed the semantic for custom headers.)

I repurposed #37025 to clean up the tests, which should show the problem more clearly. I'll put up a fix.

[Jacob Walls]

​PR -- this branch contains some other related fixes, so we might hold this branch until the other work is reviewed.

[Jacob Walls <jacobtylerwalls@…>]

In 1085e5e:

Fixed #36300 -- Restored the semantic where RemoteUserMiddleware.header corresponds to request.META under ASGI.

Because these tests always passed both WSGI environ values and HTTP
headers via **extra, this masked a behavior difference between WSGI
and ASGI.

What should happen: everything should be passed via headers but for
the default REMOTE_USER case on WSGI, which should be passed via
**extra.

Since that was not done, a regression made it into Django 5.2
(50f89ae850f6b4e35819fe725a08c7e579bfd099) where .header no longer
corresponded to the request.META key under ASGI. To cope, an ASGI user
would have started(*) sending HTTP headers that match the .header
attribute, which may or may not have been edited to remove the HTTP_
prefix. (Note: the default REMOTE_USER case did not work under ASGI,
so the change in Django 5.2 had the effect of fixing the default case
but changing the semantic of the custom case.)

(*): Unless they were getting the sync execution path, which didn't have
this bug. See the fix in 0f4fff79d33b7cc84822e66bd1fc16caf8222e3a.

Thanks Mykhailo Havelia and Sarah Boyce for reviews.