[Django] #23793: Password Reset is confusing
Django
--------------------------------+--------------------
Reporter: collinanderson | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.auth | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------
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."
--
Ticket URL: <https://code.djangoproject.com/ticket/23793>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: collinanderson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
This sounds fine. Ideally the documentation changes would be a separate
commit or pull request so it can be more easily backported.
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:1>
Django
Reporter: collinanderson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by erikr):
* easy: 0 => 1
Comment:
* 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:2>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Cleanup/optimization | Status: assigned
Component: contrib.auth | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by yigitguler):
* status: new => assigned
* owner: nobody => yigitguler
Comment:
I started working on this.
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:3>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by yigitguler):
* has_patch: 0 => 1
Comment:
The patch is here? https://github.com/django/django/pull/3534
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:4>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Component: contrib.auth | Version: 1.6
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
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:"9dde0a211e0be8829e03b3c0fb236c408f888d44"]:
{{{
#!CommitTicketReference repository=""
revision="9dde0a211e0be8829e03b3c0fb236c408f888d44"
Fixed #23793 -- Clarified password reset messages.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:5>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Component: contrib.auth | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
Comment:
Oops, didn't mean to close the ticket, this was just the first PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:6>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Component: contrib.auth | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Erik Romijn <eromijn@…>):
In [changeset:"c1584e1df45a28cc374f634982065472dd23cc11"]:
{{{
#!CommitTicketReference repository=""
revision="c1584e1df45a28cc374f634982065472dd23cc11"
Refs #23793 -- Fixed test failure after password reset messages
clarification
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:7>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Component: contrib.auth | Version: 1.6
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"c5132382f081bd1b5a3618bbf23fa0cf720af14b"]:
{{{
#!CommitTicketReference repository=""
revision="c5132382f081bd1b5a3618bbf23fa0cf720af14b"
Fixed #23793 -- Clarified password reset behavior in auth docs
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:8>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Erik Romijn <eromijn@…>):
In [changeset:"d49c42e20e24df2f248f15ac902121c1cc554656"]:
{{{
#!CommitTicketReference repository=""
revision="d49c42e20e24df2f248f15ac902121c1cc554656"
[1.7.x] Fixed #23793 -- Clarified password reset behavior in auth docs
Backport of c5132382f081bd1b5a3618bbf23fa0cf720af14b from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:9>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Erik Romijn <eromijn@…>):
In [changeset:"7323e15d872058ff4519f2c294507b16c9df7c9b"]:
{{{
#!CommitTicketReference repository=""
revision="7323e15d872058ff4519f2c294507b16c9df7c9b"
[1.6.x] Fixed #23793 -- Clarified password reset behavior in auth docs
Backport of c5132382f081bd1b5a3618bbf23fa0cf720af14b from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:10>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by erikr):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:11>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by collinanderson):
Sounds good. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:12>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by orzel):
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
[changeset:"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.
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:13>
Django
Reporter: collinanderson | Owner:
Type: | yigitguler
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by collinanderson):
Hi, It was fixed in 1.8, not 1.7. (Though some of the doc changes were
backported.)
--
Ticket URL: <https://code.djangoproject.com/ticket/23793#comment:14>