Opened 7 hours ago
Last modified 5 hours ago
#36002 new Cleanup/optimization
Errors in RemoteUserMiddleware/PersistentRemoteUserMiddleware docstrings and code comments
Reported by: | Anders Einar Hilden | Owned by: | |
---|---|---|---|
Component: | contrib.auth | Version: | dev |
Severity: | Normal | Keywords: | RemoteUserMiddleware, PersistentRemoteUserMiddleware |
Cc: | Anders Einar Hilden | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
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-in@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-in@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-in@header.com', 'REMOTE_USER': 'set-in@env.com', request.user: set-in@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-in@header.com' http://localhost:8006/dump_request | grep -E 'REMOTE_USER|request.user'
'HTTP_REMOTE_USER': 'set-in@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-in@header.com' http://localhost:8000/dump_request | grep -E 'REMOTE_USER|request.user'
'HTTP_REMOTE_USER': 'set-in@header.com', 'REMOTE_USER': 'set-in@env.com', request.user: set-in@header.com
Change History (2)
comment:1 by , 6 hours ago
Has patch: | set |
---|
comment:2 by , 5 hours ago
Was informed a PR was the way to go, so made a PR: https://github.com/django/django/pull/18920
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.