#30037 closed New feature (fixed)
RemoteUserBackend should pass request into configure_user()
Reported by: | Joshua Cannon | Owned by: | Joshua Cannon |
---|---|---|---|
Component: | contrib.auth | Version: | dev |
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 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Version: | 2.1 → master |
comment:2 by , 6 years ago
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 by , 6 years ago
Hello Greg!
The idea is to follow the usual approach used when a function signature is changed in a backward incompatible way. That is:
- 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. - If a
TypeError
is raised that means the function still uses the old signature. That means aRemovedInDjango31Warning
warning should be emitted and the function should be called with the old signature, that is without passingrequest
. - 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 by , 6 years ago
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 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 6 years ago
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 by , 6 years ago
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.
Now that
authenticate
gets passedrequest
I don't see a reason why this can't be done forconfigure_user
.I guess we could reuse
configure_user(request, user)
with a deprecation shim that warns about aconfigure_user(user)
signature throughinspect
reflection just like when therequest
argument was introduced toauthenticate
in #25187.Here's an example:
https://github.com/django/django/commit/4b9330ccc04575f9e5126529ec355a450d12e77c#diff-82ec6fd74a1cab41408fbf5bf0998f58R73