Opened 13 years ago
Closed 11 years ago
#16919 closed New feature (fixed)
Pass user to set_password_form in GET requests
Reported by: | jaimeirurzun | Owned by: | nobody |
---|---|---|---|
Component: | contrib.auth | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | ethan.jucovy@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
SetPasswordForm is being passed None on GET requests even though there is always a user available at that point. This patch passes user, so you can use it in the form constructor for whatever - e.g. populate initial with values that depend on the user involved.
Attachments (2)
Change History (9)
by , 13 years ago
Attachment: | setpasswordform.patch added |
---|
comment:1 by , 13 years ago
comment:2 by , 13 years ago
I was also concerned about the security implications that this patch might have when I wrote it, but given this only applies to the case in which the token has already been validated, I can't think of any security hole.
Basically I have a custom SetPasswordForm in which I give the user the opportunity to update a few fields from his profile that will be used in the password reset logic, so I want to fill the initial values with his current data, for which I need the user object.
comment:3 by , 13 years ago
Needs tests: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 13 years ago
Cc: | added |
---|
I have another use case for this: rendering the user's name in the registration/password_reset_confirm.html
template.
Currently the password_reset_confirm
view does not provide "user" as a template context variable, nor even "uidb36" and "token". Since the form also doesn't have the user object stored on a GET request, this means that there's no way for the template to say "{% if validlink %} Hello, {{ user.username }} -- reset your password here {% endif %}" -- short of forking the view, or some pretty hacky middleware that re-parses the request URL and re-fetches the user from the given uid+token.
I see that the "needs_tests" flag is set on this ticket .. what sort of test would be required for this patch to be merged?
by , 13 years ago
Attachment: | setpasswordform_withtests.diff added |
---|
comment:5 by , 13 years ago
Needs tests: | unset |
---|
I've attached a new version of the patch including auth.views
tests that double as demonstration of a use case for this behavior.
comment:6 by , 11 years ago
Another use case for this:
I can add "security question/answer" that user picks when registering and extend SetPasswordForm with CharField labeled with question user picked.
comment:7 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Technically, the patch works.
However, I can't figure out a practical use case for prepopulating a password field that doesn't have security issues. I'd like to make sure this change doesn't encourage bad practices.
Could you explain what you're trying to achieve?