Opened 8 years ago

Closed 4 years ago

#26615 closed Cleanup/optimization (fixed)

Changing user's email could invalidate password reset tokens

Reported by: Alex Gaynor Owned by: Jacob Walls
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Sequence:

  • Have account with email address foo@…
  • Password reset request for that email (unused)
  • foo@… account changes their email address
  • Password reset email is used

The password reset email's token should be rejected at that point, but in fact it is allowed.

The fix is to add the user's email address into PasswordResetTokenGenerator._make_hash_value()

Nothing forces a user to even have an email as per AbstractBaseUser. Perhaps the token generation method could be factored out onto the model, ala get_session_auth_hash().

Change History (16)

comment:1 by Silas Barta, 8 years ago

Has patch: set

comment:2 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:3 by Silas Barta, 8 years ago

Patch needs improvement: unset

comment:5 by Berker Peksag, 8 years ago

Needs documentation: set
Patch needs improvement: set

comment:6 by Ross Curzon-Butler, 8 years ago

Owner: changed from nobody to Ross Curzon-Butler
Status: newassigned

comment:7 by Ross Curzon-Butler, 8 years ago

New pull request made at https://github.com/django/django/pull/6868 containing requested documentation

comment:8 by Tim Graham, 8 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:9 by Tim Graham, 8 years ago

Patch needs improvement: set

Comments for improvement are on the PR.

comment:10 by Mariusz Felisiak, 4 years ago

Owner: Ross Curzon-Butler removed
Status: assignednew

comment:11 by Jacob Walls, 4 years ago

Owner: set to Jacob Walls
Status: newassigned

New PR forthcoming, new patch plus original test written by Silas.

comment:12 by Jacob Walls, 4 years ago

Patch needs improvement: unset

PR

The linked patch may need an additional check if it's conceivable that EMAIL_FIELD might be nullable, but that's no hurdle. Waiting for input first.

comment:13 by Mariusz Felisiak, 4 years ago

Needs tests: set
Patch needs improvement: set

comment:14 by Jacob Walls, 4 years ago

Needs tests: unset
Patch needs improvement: unset

comment:15 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 0362b0e9:

Fixed #26615 -- Made password reset token invalidate when changing email.

Co-Authored-By: Silas Barta <sbarta@…>

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