Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#23793 closed Cleanup/optimization (fixed)

Password Reset is confusing

Reported by: Collin Anderson Owned by: Yiğit Güler
Component: contrib.auth Version: 1.6
Severity: Normal Keywords:
Cc: cmawebsite@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Ever since #19758 (Avoided leaking email existence through the password reset), I think the password reset is confusing.

  • If it's not too late, could we add #19758 to the release notes for 1.6?
  • Seems to me, like the comments on the original ticket said, we should still document a code example of how to validate the email address (with a caution about information leakage). (Could just copy the removed code.)
  • Maybe we could reword the message "Password reset successful", because we're saying it's successful even if it isn't. Maybe something like, "If we find matching email address, we'll send you an email".
  • Better yet, if there's no match, maybe we could send an email saying "We're sorry, we couldn't find an account with your email address."

Change History (14)

comment:1 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

This sounds fine. Ideally the documentation changes would be a separate commit or pull request so it can be more easily backported.

comment:2 by Sasha Romijn, 9 years ago

Easy pickings: set
  • I agree completely that the existing situation is confusing to users and developers.
  • We should indeed document how to recreate the old situation where explicit errors are raised if no e-mail was sent.
  • We should also explicitly document this silent error behaviour on the pages where we describe the templates to create for password reset.
  • Note that an account not existing is not the only case now where no e-mail is sent (without error): this is also the case if the account is inactive or has an unusable password.
  • I am completely opposed to sending an e-mail if there is no match. That would turn any Django site into a trivial spam machine, and I think there may also be security implications, but not entirely sure on that.

comment:3 by Yiğit Güler, 9 years ago

Owner: changed from nobody to Yiğit Güler
Status: newassigned

I started working on this.

comment:4 by Yiğit Güler, 9 years ago

Has patch: set

comment:5 by Erik Romijn <eromijn@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In 9dde0a211e0be8829e03b3c0fb236c408f888d44:

Fixed #23793 -- Clarified password reset messages.

comment:6 by Sasha Romijn, 9 years ago

Resolution: fixed
Status: closednew

Oops, didn't mean to close the ticket, this was just the first PR.

comment:7 by Erik Romijn <eromijn@…>, 9 years ago

In c1584e1df45a28cc374f634982065472dd23cc11:

Refs #23793 -- Fixed test failure after password reset messages clarification

comment:8 by Erik Romijn <eromijn@…>, 9 years ago

Resolution: fixed
Status: newclosed

In c5132382f081bd1b5a3618bbf23fa0cf720af14b:

Fixed #23793 -- Clarified password reset behavior in auth docs

comment:9 by Erik Romijn <eromijn@…>, 9 years ago

In d49c42e20e24df2f248f15ac902121c1cc554656:

[1.7.x] Fixed #23793 -- Clarified password reset behavior in auth docs

Backport of c5132382f081bd1b5a3618bbf23fa0cf720af14b from master.

comment:10 by Erik Romijn <eromijn@…>, 9 years ago

In 7323e15d872058ff4519f2c294507b16c9df7c9b:

[1.6.x] Fixed #23793 -- Clarified password reset behavior in auth docs

Backport of c5132382f081bd1b5a3618bbf23fa0cf720af14b from master.

comment:11 by Sasha Romijn, 9 years ago

Upon closer review, we decided not to add additional documentation on how to restore the behaviour where error messages are enabled. The proper solution turns out to already be documented in the docs: inherit from the password reset form, and pass your new form as a view parameter. Any deeper suggestion or code sample would quickly be very specific and full of assumptions. What if someone wants to return an error for accounts that do not exist, but not for accounts that are disabled?

Considering the security sensitive nature of errors in this area, this is not a place where a copy-pastable standard solution is appropriate, and the general approach is already documented.

comment:12 by Collin Anderson, 9 years ago

Sounds good. Thanks.

comment:13 by Thomas Capricelli, 9 years ago

I'm not sure to understand everything that's going on, but my main concern is still not fixed in django 1.7, as of today (did just update) :

the title for password_reset_done is still 'Password reset successful' instead of 'Password reset sent'. The patch in 9dde0a211e0be8829e03b3c0fb236c408f888d44 is what would fix my issue.

This ticket is close. Should i open another ticket or do you really mean "we don't think this needs fixing".. ?
I'm confused by the last comment.

comment:14 by Collin Anderson, 9 years ago

Hi, It was fixed in 1.8, not 1.7. (Though some of the doc changes were backported.)

Note: See TracTickets for help on using tickets.
Back to Top