Opened 11 months ago

Closed 10 months ago

Last modified 2 months ago

#30037 closed New feature (fixed)

RemoteUserBackend should pass request into configure_user()

Reported by: Joshua Cannon Owned by: Joshua Cannon
Component: contrib.auth Version: master
Severity: Normal Keywords: RemoteUserBackend
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Clients that wish to populate a user's information by subclassing RemoteUserBackend and overriding configure_user find themselves a bit restricted on what they can configure. If a client wants to fill out the user's first name and last name using HTTP headers, there is no easy solution for them.

Instead, the request should also be sent to configure_user (or something similar that wouldn't break compatibility).

Change History (11)

comment:1 Changed 11 months ago by Simon Charette

Triage Stage: UnreviewedAccepted
Version: 2.1master

Now that authenticate gets passed request I don't see a reason why this can't be done for configure_user.

I guess we could reuse configure_user(request, user) with a deprecation shim that warns about a configure_user(user) signature through inspect reflection just like when the request argument was introduced to authenticate in #25187.

Here's an example:

https://github.com/django/django/commit/4b9330ccc04575f9e5126529ec355a450d12e77c#diff-82ec6fd74a1cab41408fbf5bf0998f58R73

comment:2 Changed 11 months ago by Greg W

Is this as simple as adding request to configure_user(user) and updating the documentation regarding the change? If so, I can take it.

comment:3 Changed 11 months ago by Simon Charette

Hello Greg!

The idea is to follow the usual approach used when a function signature is changed in a backward incompatible way. That is:

  1. Before calling the function inspect it to determine whether or not it uses the new or old signature. This can be done using inspect.getcallargs(self.configure_user, request, user) in this case.
  2. If a TypeError is raised that means the function still uses the old signature. That means a RemovedInDjango31Warning warning should be emitted and the function should be called with the old signature, that is without passing request.
  3. In no exceptions are raised that means the function was either not overridden or switched to the new signature so it can be directly called with the new request argument.

Have a look at 4b9330ccc04575f9e5126529ec355a450d12e77c as an example.

This will also require documentation updates and tests as demonstrated in the above example as well.

comment:4 Changed 11 months ago by Joshua Cannon

Actually, if you don't mind, I'd love to take a stab at it. It seems easy enough as a first PR, and it's something I'm needing anyways :)

comment:5 Changed 11 months ago by Joshua Cannon

Owner: changed from nobody to Joshua Cannon
Status: newassigned

comment:6 Changed 11 months ago by Joshua Cannon

Has patch: set

comment:7 Changed 11 months ago by Greg W

Thanks for the explanation Simon!

Joshua - sure thing, have at it.

comment:8 Changed 11 months ago by Joshua Cannon

What's the protocol for "bump"-ing a review? Is there a reviewer or reviewers I should add? Or should I comment on the PR?

comment:9 Changed 11 months ago by Simon Charette

Hello Joshua, your patch is currently in the review list but it might take a few days before fellows get to it because of holidays.

comment:10 Changed 10 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In db1b10ef:

Fixed #30037 -- Added request arg to RemoteUserBackend.configure_user().

comment:11 Changed 2 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In d17be88a:

Refs #30037 -- Required the RemoteUserBackend.configure_user() to have request as the first positional argument.

Per deprecation timeline.

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