Code

Opened 3 years ago

Closed 9 months 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)

setpasswordform.patch (479 bytes) - added by Jaime Irurzun <jaime.irurzun@…> 3 years ago.
setpasswordform_withtests.diff (2.4 KB) - added by ejucovy 2 years ago.

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by Jaime Irurzun <jaime.irurzun@…>

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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?

comment:2 Changed 3 years ago by jaimeirurzun

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 Changed 3 years ago by aaugustin

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 3 years ago by ejucovy

  • Cc ethan.jucovy@… 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.

If this patch were accepted, that would be possible, like so: "{% if validlink %} Hello, {{ form.user.username }} -- reset your password here {% endif %}"

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?

Last edited 3 years ago by ejucovy (previous) (diff)

Changed 2 years ago by ejucovy

comment:5 Changed 2 years ago by ejucovy

  • 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 Changed 10 months ago by anonymous

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 Changed 9 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 1285ca67eba96045b4f6fe6f5c7fd6570571f1bd:

Fixed #16919 -- Passed user to set_password_form in GET requests.

Thanks Jaime Irurzun for the report and initial patch and
ejucovy for the test.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.