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 Anders Einar Hilden, 6 hours ago

Has patch: set

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.

comment:2 by Anders Einar Hilden, 5 hours ago

Was informed a PR was the way to go, so made a PR: https://github.com/django/django/pull/18920

Note: See TracTickets for help on using tickets.
Back to Top