Opened 5 weeks ago

Closed 5 weeks ago

Last modified 5 weeks ago

#35971 closed New feature (wontfix)

RemoteUserMiddleware needs a get_username method

Reported by: Adrien Kunysz Owned by:
Component: contrib.auth Version: 5.0
Severity: Normal Keywords:
Cc: Adrien Kunysz Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Adrien Kunysz)

As currently implemented, the only way to customise how RemoteUserMiddleware gets the username is through the "header" variable. This is then used in call and acall methods like this:

            username = request.META[self.header]

It would be convenient to move that logic into a separate method that could be overridden. For example:

get_username(self, request, header_name):
    return request.META[header_name]    

Specific use case: the proxy I have in front of Django always sets two specific headers (say "X-Username" and "X-Authenticated"). The value of "X-Username" is only valid if "X-Authenticated" is "true", otherwise it should be ignored (typically it ends up being a single space character). I use PersistentRemoteMiddleware to use X-Username but the only way I found to ignore it when X-Authenticated is not true is to override call / acall , which seems rather fragile while a small change to RemoteUserMiddleware would make for a much more robust, flexible and maintainable solution.

With the proposed change, in my child class I could just say

def get_username(self, request, header_name):
    if request.META["X-Authenticated"].lower() != "true":
        raise KeyError
    return request.META[header_name]

I am happy to propose a patch if we can agree this change is desirable.

The analysis above is for the latest version on github. I have marked this feature request as 5.0 because that's the version I currently use and backporting the proposed change seems easy enough.

Change History (6)

comment:1 by Adrien Kunysz, 5 weeks ago

Type: UncategorizedNew feature

comment:2 by Adrien Kunysz, 5 weeks ago

Version: dev5.0

comment:3 by Adrien Kunysz, 5 weeks ago

Description: modified (diff)

comment:4 by Adrien Kunysz, 5 weeks ago

Description: modified (diff)

comment:5 by Sarah Boyce, 5 weeks ago

Resolution: wontfix
Status: newclosed

I think given that it's probably not too difficult to achieve this by overriding the call method, I'm not sure it's worth adding a hook
(below is untested)

class MyRemoteUserMiddleware(RemoteUserMiddleware):
    def __call__(self, request):
        if request.META.get(self.header) and request.META["X-Authenticated"].lower() != "true":
            if self.force_logout_if_no_header and request.user.is_authenticated:
                self._remove_invalid_user(request)
            return self.get_response(request)
        return super().__call__(request)

If you want it to be added, you can propose and discuss the idea on the Django Forum, where you'll reach a broader audience and receive additional feedback.

I'll close the ticket for now, but if the community agrees with the proposal, please return to this ticket and reference the forum discussion so we can re-open it. For more information, please refer to the documented guidelines for requesting features.

comment:6 by Adrien Kunysz, 5 weeks ago

Thank you. I don't think your proposal resolves my problem but it gave me another idea. I can just delete the entry from request.META before calling the parent call. In case anyone else runs into this issue, this is what I am doing (5.0 has process_request instead of call):

def process_request(self, request):
    authenticated = request.META.get("X-Authenticated", "false").lower() == "true"
    if not authenticated and self.header in request.META:
        del request.META[self.header]
    return super().process_request(request)
Note: See TracTickets for help on using tickets.
Back to Top