Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented Oct 17, 2020

ticket-26615

Thanks to @SilasX for authoring the test.

Previous discussion in pull requests #6868, #6621, although this patch is more concise.

UPDATE: If it's conceivable that EMAIL_FIELD could be nullable, this may warrant more handling. Done.

@jacobtylerwalls jacobtylerwalls changed the title Fixed #26615 -- Made password token generator incorporate email address. Fixed #26615 -- Made password reset token generator incorporate email address. Oct 17, 2020
@smithdc1
Copy link
Member

Hi Jacob. I think you may need to look at over riding _now()? The test passes without your patch if the two tests are either side of a second boundary.

@jacobtylerwalls
Copy link
Member Author

Thanks @dcsmith1, in skimming the unmerged PRs I didn't see much discussion around the test, and so it escaped my notice that it wasn't targeting the reported behavior. This should fail without my patch.

Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacobtylerwalls Thanks 👍

tests/auth_tests/test_tokens.py Outdated Show resolved Hide resolved
tests/auth_tests/test_tokens.py Outdated Show resolved Hide resolved
tests/auth_tests/test_tokens.py Show resolved Hide resolved
django/contrib/auth/tokens.py Outdated Show resolved Hide resolved
django/contrib/auth/tokens.py Show resolved Hide resolved
docs/releases/3.2.txt Outdated Show resolved Hide resolved
@felixxm felixxm changed the title Fixed #26615 -- Made password reset token generator incorporate email address. Fixed #26615 -- Made password reset token invalidate when changing email. Oct 20, 2020
@jacobtylerwalls
Copy link
Member Author

Thanks for improvements @felixxm, including correcting the nullable field test! :/

Copy link
Member

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super. Just a couple of teeny-weeny suggestions.

django/contrib/auth/tokens.py Outdated Show resolved Hide resolved
docs/releases/3.2.txt Outdated Show resolved Hide resolved
…ail.

Co-Authored-By: Silas Barta <sbarta@gmail.com>
@felixxm felixxm merged commit 0362b0e into django:master Oct 21, 2020
@jacobtylerwalls jacobtylerwalls deleted the password_reset_hash branch October 21, 2020 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants