[Django] #28718: Resetting your password shouldn't depend on your current password being hashed with a supported hasher
Django
hashed with a supported hasher
-----------------------------------------+------------------------
Reporter: mr-bo-jangles | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
Currently the built in django password reset system requires that you have
an active account and that your password can be compared to by an enabled
hasher.
I think that this is in error, as you are about to reset the password to
something new (hence resetting it) and the standard process of password
resetting requires an email confirmation. I can see no way in which this
is able to be abused by a malicious 3rd party. If I'm mistaken here then
feel free to correct me.
I propose that the system is changed to just require that the user is
active and that their password is not marked disabled as per the
UNUSABLE_PASSWORD_PREFIX.
--
Ticket URL: <https://code.djangoproject.com/ticket/28718>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
hashed with a supported hasher
Reporter: Josh Harwood | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Bug => Cleanup/optimization
Comment:
Where's that restriction implemented?
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:1>
Django
hashed with a supported hasher
Reporter: Josh Harwood | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Josh Harwood):
Replying to [comment:1 Tim Graham]:
> Where's that restriction implemented?
This line with "has_usable_password":
https://github.com/django/django/blob/master/django/contrib/auth/forms.py#L264
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:2>
Django
supported hasher
------------------------------+------------------------------------
Reporter: Josh Harwood | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
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 Tim Graham):
* stage: Unreviewed => Accepted
* type: Cleanup/optimization => Bug
Comment:
It looks like this is a regression in
aeb1389442d0f9669edf6660b747fd10693b63a7. The change in
`django/contrib/auth/forms.py` line 234 added the undesired check that the
password uses a valid hasher.
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:3>
Django
supported hasher
------------------------------+--------------------------------------
Reporter: Josh Harwood | Owner: Levi Payne
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Levi Payne):
* owner: nobody => Levi Payne
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:4>
Django
supported hasher
------------------------------+--------------------------------------
Reporter: Josh Harwood | Owner: Levi Payne
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Levi Payne):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/9276 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:5>
Django
supported hasher
------------------------------+--------------------------------------
Reporter: Josh Harwood | Owner: Levi Payne
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Levi Payne):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
Not going to be able to finish this. Just needs unit tests. See pull
request for more details.
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:6>
Django
supported hasher
------------------------------+--------------------------------------
Reporter: Josh Harwood | Owner: Tim Graham
Type: Bug | Status: assigned
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Changes (by Tim Graham):
* owner: Levi Payne => Tim Graham
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:7>
Django
supported hasher
------------------------------+--------------------------------------
Reporter: Josh Harwood | Owner: Tim Graham
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
Comment:
I think the change to `has_usable_password()` in
703c266682be39f7153498ad0d8031231f12ee79 was unintentional and that adding
a `User.has_disabled_password()` method as originally proposed here will
only add more confusion. I've proposed an
[https://github.com/django/django/pull/9802 alternate PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:8>
Django
supported hasher
------------------------------+--------------------------------------
Reporter: Josh Harwood | Owner: Tim Graham
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"43348e4aa16834d0456e7c9f593e6c5e5447bcd3" 43348e4a]:
{{{
#!CommitTicketReference repository=""
revision="43348e4aa16834d0456e7c9f593e6c5e5447bcd3"
Reverted "[2.0.x] Expanded docs for
AbstractBaseUser.has_usable_password()."
This reverts commit c2962d8147380b00bb965c0d42b7e05ca4596868. After
discussion
in refs #28718, the decision is to treat the reverted statements as bugs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:9>
Django
supported hasher
------------------------------+--------------------------------------
Reporter: Josh Harwood | Owner: Tim Graham
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"a4f0e9aec76ee40c938d3bf450ff63211b7ba1f1" a4f0e9ae]:
{{{
#!CommitTicketReference repository=""
revision="a4f0e9aec76ee40c938d3bf450ff63211b7ba1f1"
Fixed #28718 -- Allowed user to request a password reset if their password
doesn't use an enabled hasher.
Regression in aeb1389442d0f9669edf6660b747fd10693b63a7.
Reverted changes to is_password_usable() from
703c266682be39f7153498ad0d8031231f12ee79 and documentation changes from
92f48680dbd2e02f2b33f6ad0e35b7d337889fb2.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28718#comment:10>