[Django] #34066: Accessing UserAdmin via to_field leads to link to PasswordResetForm being broken (404)
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon | Owner: nobody
Kern |
Type: Bug | Status: new
Component: | Version: dev
contrib.auth | Keywords: auth, password,
Severity: Normal | reset, passwordreset
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
Accessing the {{{UserAdmin}}} via another model's Admin that has a
reference to {{{User}}} (with to_field set, e.g., {{{to_field="uuid"}}})
leads to the {{{UserAdmin}}} being accessed via an url that looks similar
to this one:
{{{.../user/22222222-3333-4444-5555-666677778888/change/?_to_field=uuid}}}
However the underlying form looks like this:
{{{
#!div style="font-size: 80%"
Code highlighting:
{{{#!python
class UserChangeForm(forms.ModelForm):
password = ReadOnlyPasswordHashField(
label=_("Password"),
help_text=_(
"Raw passwords are not stored, so there is no way to see this
"
"user’s password, but you can change the password using "
'<a href="{}">this form</a>.'
),
)
...
...
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
password = self.fields.get("password")
if password:
password.help_text = password.help_text.format("../password/")
...
...
}}}
}}}
This results in the link to the {{{PasswordResetForm}}} being wrong and
thus ending up in a 404. If we drop the assumption that UserAdmin is
always accessed via its pk, then we're good to go. It's as simple as
replacing {{{password.help_text =
password.help_text.format("../password/")}}} with {{{password.help_text =
password.help_text.format(f"../../{self.instance.pk}/password/")}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34066>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
broken (404)
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage:
reset, passwordreset | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "patch.diff" added.
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage:
reset, passwordreset | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
I've opened a pull request on GitHub for this Ticket, please see:
https://github.com/django/django/pull/16139
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:1>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage:
reset, passwordreset | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Thanks for the report. Could you add a regression test for this to your
patch Simon?
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:2>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage:
reset, passwordreset | Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:3>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage:
reset, passwordreset | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Kern):
Hi Carlton, I'd love to, but I don't find anything that looks similar in
the auth_tests. So I am wondering what would be the best approach for this
scenario. Could you point me in the right direction?
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:4>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage:
reset, passwordreset | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Hi Simon — so it's setting up some models with the relationships at
question and then using the test client to go through the flow, hopefully
showing the error.
There are various setups in auth_tests/models. You can add new models if
needed to demonstrate the issue. Once we have a reproduce we can look at
whether it's possible to simplify it, but all changes need regression
tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:5>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
I'd add a test in `UserChangeFormTest` (`auth_tests/test_forms.py`) that
verifies the password field's help_text. I don't think the test client is
needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:6>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Kern):
Thanks Carlton and Tim, I've added a test in {{{UserChangeFormTest
(auth_tests/test_forms.py)}}}.
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:7>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "test.diff" added.
diff for the test
--
Ticket URL: <https://code.djangoproject.com/ticket/34066>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Kern):
* Attachment "patch.diff" removed.
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Kern):
* Attachment "patch.diff" added.
Patch including tests
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
[https://github.com/django/django/pull/16139 PR]
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:8>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
Comment:
[https://github.com/django/django/pull/16139 PR]
By the way, there's no need to attach your patch to the ticket, but you
should link to the pull request. I'll do that and uncheck "Needs tests" to
put this patch in the review queue. In the future, check "According to the
ticket's flags" on this page for the steps to move the ticket forward.
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:9>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Kern):
Thank you Tim, I was not aware that putting the PR in the initial post is
not enough (it was in there since after my first edit). For the future:
It's ok for me to uncheck todo flags like "Needs documentation" and "Needs
tests" the underlying todo is done?
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:10>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Yes
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:11>
Django
broken (404)
-------------------------------------+-------------------------------------
| Kern
Type: Bug | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Simon Kern
* status: new => assigned
* component: contrib.auth => contrib.admin
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:12>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: Simon
| Kern
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by David Sanders):
* component: contrib.admin => contrib.auth
Comment:
Actually sorry it is auth as the form is the problem
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:13>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: Simon
| Kern
Type: Bug | Status: assigned
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:14>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: Simon
| Kern
Type: Bug | Status: assigned
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
Keywords: auth, password, | Triage Stage: Accepted
reset, passwordreset |
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:15>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: Simon
| Kern
Type: Bug | Status: assigned
Component: contrib.auth | Version: dev
Severity: Normal | Resolution:
reset, passwordreset | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
Comment:
@felixx Simon's made the requested updates, should be good for you to re-
review ☺️
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:16>
Django
broken (404)
-------------------------------------+-------------------------------------
Reporter: Simon Kern | Owner: Simon
| Kern
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: fixed
Keywords: auth, password, | Triage Stage: Ready for
reset, passwordreset | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"de2c2127b66e77a034c01c81753c5c08e651a5b4" de2c2127]:
{{{
#!CommitTicketReference repository=""
revision="de2c2127b66e77a034c01c81753c5c08e651a5b4"
Fixed #34066 -- Fixed link to password reset view in
UserChangeForm.password's help text when using to_field.
Co-Authored-By: David Sanders <shang.xia...@gmail.com>
Co-Authored-By: Mariusz Felisiak <felisiak...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34066#comment:17>