[Django] #30037: RemoteUserBackend should pass request into configure_user()
Django
-------------------------------------+-------------------------------------
Reporter: Joshua | Owner: nobody
Cannon |
Type: New | Status: new
feature |
Component: | Version: 2.1
contrib.auth |
Severity: Normal | Keywords: RemoteUserBackend
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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).
--
Ticket URL: <https://code.djangoproject.com/ticket/30037>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Joshua Cannon | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* version: 2.1 => master
* stage: Unreviewed => Accepted
Comment:
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
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:1>
Django
Reporter: Joshua Cannon | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:2>
Django
Reporter: Joshua Cannon | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:3>
Django
Reporter: Joshua Cannon | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (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 :)
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:4>
Django
-------------------------------------+-------------------------------------
| Cannon
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Joshua Cannon
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:5>
Django
-------------------------------------+-------------------------------------
| Cannon
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/10751 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:6>
Django
-------------------------------------+-------------------------------------
| Cannon
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Greg W):
Thanks for the explanation Simon!
Joshua - sure thing, have at it.
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:7>
Django
-------------------------------------+-------------------------------------
| Cannon
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (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?
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:8>
Django
-------------------------------------+-------------------------------------
| Cannon
Type: New feature | Status: assigned
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Hello Joshua, your
[https://code.djangoproject.com/query?status=!closed&needs_better_patch=0&needs_tests=0&needs_docs=0&has_patch=1&stage=Accepted&desc=1&order=changetime
patch is currently in the review list] but it might take a few days before
fellows get to it because of holidays.
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:9>
Django
-------------------------------------+-------------------------------------
| Cannon
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"db1b10ef0dcab2b8bacbea4adc681a57bd70b462" db1b10ef]:
{{{
#!CommitTicketReference repository=""
revision="db1b10ef0dcab2b8bacbea4adc681a57bd70b462"
Fixed #30037 -- Added request arg to RemoteUserBackend.configure_user().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:10>
Django
-------------------------------------+-------------------------------------
| Cannon
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: RemoteUserBackend | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"d17be88afd5f5e1059491e10408ba239e2e99fe2" d17be88a]:
{{{
#!CommitTicketReference repository=""
revision="d17be88afd5f5e1059491e10408ba239e2e99fe2"
Refs #30037 -- Required the RemoteUserBackend.configure_user() to have
request as the first positional argument.
Per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30037#comment:11>