[Django] #36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings and code comments

[Django]

#36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings
and code comments
-------------------------------------+-------------------------------------
Reporter: Anders Einar Hilden | Type:
| Cleanup/optimization
Status: new | Component:
| contrib.auth
Version: dev | Severity: Normal
Keywords: | Triage Stage:
RemoteUserMiddleware, | Unreviewed
PersistentRemoteUserMiddleware |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
**File**
django/django/contrib/auth/middleware.py
(https://github.com/django/django/blob/main/django/contrib/auth/middleware.py)

**Middleware**
RemoteUserMiddleware and PersistentRemoteUserMiddleware

**Error**
The docstring and comments state that the middlewares by default uses a
**request header** named REMOTE_USER for authentication. This is wrong, as
it by default uses the REMOTE_USER **environment variable**.

Using git blame, all of these texts appear to be from when the respective
middlewares were added to Django, 16 and 9 years ago. While it might have
been correct 16/9 years ago, it is not correct for the current code base.

While the howto for RemoteUserMiddleware imho could use some work, it
appears to use the correct terms.
https://docs.djangoproject.com/en/5.1/howto/auth-remote-
user/#configuration

From RemoteUserMiddleware docstring (line 94)
{{{
If request.user is not authenticated, then this middleware attempts to
authenticate the username passed in the ``REMOTE_USER`` request
header.
If authentication is successful, the user is automatically logged in
to
persist the user in the session.

The header used is configurable and defaults to ``REMOTE_USER``.
Subclass
this class and change the ``header`` attribute if you need to use a
different header.
}}}


In the comment starting at line 119, it is referenced to as a header
again.
{{{
# Name of request header to grab username from. This will be the key
as
# used in the request.META dictionary, i.e. the normalization of
headers to
# all uppercase and the addition of "HTTP_" prefix apply.
header = "REMOTE_USER"
}}}


From PersistentRemoteUserMiddleware (line 258)

{{{
Like RemoteUserMiddleware but keeps the user authenticated even if
the header (``REMOTE_USER``) is not found in the request. Useful
for setups when the external authentication via ``REMOTE_USER``
is only expected to happen on some "logon" URL and the rest of
the application wants to use Django's authentication mechanism.
}}}

**For testing**

A view that dumps meta and the user:
{{{
from django.http import HttpResponse
import pprint

def dump_request(request):
rs = pprint.pformat(request.META, indent=2) + f"\nrequest.user:
{request.user}"
return HttpResponse(rs)
}}}

Start runserver with a REMOTE_USER environment variable
{{{
REMOTE_USER=set...@env.com ./manage.py runserver
}}}

Send a request the the request header REMOTE_USER set. Note that the
header is with a dash (-), not underline (_), as runserver and other WSGI
servers do not allow headers with underscores, as not to be confused with
environment variables)
{{{
curl -s -H 'REMOTE-USER: set...@header.com'
http://localhost:8000/dump_request | grep -E 'REMOTE_USER|request.user'
}}}

observe that the authenticated user is the email from the environment
variable.
{{{
'HTTP_REMOTE_USER': 'set...@header.com',
'REMOTE_USER': 'set...@env.com',
request.user: set...@env.com
}}}

Stop runserver and restart w/o environment variable
{{{
./manage.py runserver
}}}

observe that the user is not authenticated.
{{{
curl -s -H 'REMOTE-USER: set...@header.com'
http://localhost:8006/dump_request | grep -E 'REMOTE_USER|request.user'
}}}
{{{
'HTTP_REMOTE_USER': 'set...@header.com',
request.user: AnonymousUser
}}}

For the adventrus bugreader, subclass RemoteUserMiddleware and make it
actually respect a HTTP request header named Remote-User.
{{{
from django.contrib.auth.middleware import RemoteUserMiddleware

class CustomRemoteUserMiddleware(RemoteUserMiddleware):
header = "HTTP_REMOTE_USER"
}}}
{{{
curl -s -H 'REMOTE-USER: set...@header.com'
http://localhost:8000/dump_request | grep -E 'REMOTE_USER|request.user'
}}}
{{{
'HTTP_REMOTE_USER': 'set...@header.com',
'REMOTE_USER': 'set...@env.com',
request.user: set...@header.com
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36002>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings
and code comments
-------------------------------------+-------------------------------------

Reporter: Anders Einar Hilden | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage:
RemoteUserMiddleware, | Unreviewed
PersistentRemoteUserMiddleware |

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Anders Einar Hilden):

* has_patch: 0 => 1

Comment:

I have suggested changes in
https://github.com/Kagee/django/commit/ba199155ae9b82693b7d2b0f7dfd7e2fb3f036e2.
The contributor howtos feel unclear if i should have just pushed this as a
PR or waiting until i get some response here.
--
Ticket URL: <https://code.djangoproject.com/ticket/36002#comment:1>

[Django]

#36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings
and code comments
-------------------------------------+-------------------------------------
Reporter: Anders Einar Hilden | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
RemoteUserMiddleware, | Unreviewed
PersistentRemoteUserMiddleware |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Anders Einar Hilden):

Was informed a PR was the way to go, so made a PR:
https://github.com/django/django/pull/18920
--
Ticket URL: <https://code.djangoproject.com/ticket/36002#comment:2>

[Django]

#36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings
and code comments
-------------------------------------+-------------------------------------

Reporter: Anders Einar Hilden | Owner: Anders
Type: | Einar Hilden
Cleanup/optimization | Status: assigned

Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
RemoteUserMiddleware, | Unreviewed
PersistentRemoteUserMiddleware |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by JaeHyuckSa):

* owner: (none) => Anders Einar Hilden
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36002#comment:3>

[Django]

#36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings
and code comments
-------------------------------------+-------------------------------------
Reporter: Anders Einar Hilden | Owner: Anders
Type: | Einar Hilden

Cleanup/optimization | Status: closed
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: invalid

Keywords: | Triage Stage:
RemoteUserMiddleware, | Unreviewed
PersistentRemoteUserMiddleware |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* resolution: => invalid
* status: assigned => closed

Comment:

[https://docs.djangoproject.com/en/5.1/ref/request-
response/#django.http.HttpRequest.META HttpRequest.META] is a dictionary
of headers. On WSGI, these are initiated from
[https://github.com/django/django/blob/6f38697f90a14f1450a71c1e40aea0f5df7dca86/django/core/handlers/wsgi.py#L68
environment variables].
I think both the docs and the doc strings are correct. The docs refer to
how the header is likely to be set (from the environment variable) and the
docstring is on what it actually is looking at (the header).
--
Ticket URL: <https://code.djangoproject.com/ticket/36002#comment:4>

[Django]

#36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings
and code comments
-------------------------------------+-------------------------------------
Reporter: Anders Einar Hilden | Owner: Anders
Type: | Einar Hilden
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
RemoteUserMiddleware, | Unreviewed
PersistentRemoteUserMiddleware |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Anders Einar Hilden):

I do not concurre with that evaluation. The docstring & comments
spesifically say "the ``REMOTE_USER`` request header" - But a request
header can never be named "REMOTE_USER", if you send a header named
"REMOTE-USER", it will be named HTTP_REMOTE_USER in the dict, not
REMOTE_USER. Thus the default can only be passed as a direct envirionment
variable, it can never come from a header.
--
Ticket URL: <https://code.djangoproject.com/ticket/36002#comment:5>

[Django]

#36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings
and code comments
-------------------------------------+-------------------------------------
Reporter: Anders Einar Hilden | Owner: Anders
Type: | Einar Hilden
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
RemoteUserMiddleware, | Unreviewed
PersistentRemoteUserMiddleware |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Anders Einar Hilden):

For more context, this issue was raised after a lengthy support session
after a user assumed passing a header named REMOTE-USER would work,
because that was spesifically what the docstring for the middleware said
would work.
--
Ticket URL: <https://code.djangoproject.com/ticket/36002#comment:6>

[Django]

#36002: Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings
and code comments
-------------------------------------+-------------------------------------
Reporter: Anders Einar Hilden | Owner: Anders
Type: | Einar Hilden
Cleanup/optimization | Status: closed
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
RemoteUserMiddleware, | Unreviewed
PersistentRemoteUserMiddleware |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce):

There is a configurable `header` attribute and in the docs there is an
[https://docs.djangoproject.com/en/5.1/howto/auth-remote-
user/#configuration example configuring it]
{{{
from django.contrib.auth.middleware import RemoteUserMiddleware


class CustomHeaderMiddleware(RemoteUserMiddleware):
header = "HTTP_AUTHUSER"
}}}

By default, the value is `"REMOTE_USER"`. This header can only be set via
an environment variable.

I think the comment above the `header` also hints at this

{{{
# Name of request header to grab username from. This will be the key
as
# used in the request.META dictionary, i.e. the normalization of
headers to
# all uppercase and the addition of "HTTP_" prefix apply.
header = "REMOTE_USER"
}}}

As there are different ways to set headers, I don't think it's wrong to
refer to this as a header 🤔

----
We might be able to make the docstring clearer, but I'm not convinced that
it's clearly wrong
--
Ticket URL: <https://code.djangoproject.com/ticket/36002#comment:7>