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.

Sign up for GitHub

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

Jump to bottom

Fixed #32275 -- Added scrypt password hasher. #13799

Merged
merged 1 commit into from
Jul 22, 2021
Merged

Fixed #32275 -- Added scrypt password hasher. #13799

merged 1 commit into from
Jul 22, 2021

Conversation

ryowright
Copy link
Contributor

@ryowright ryowright commented Dec 21, 2020 •
edited by timgraham
Loading

jacobtylerwalls
jacobtylerwalls reviewed Dec 22, 2020
View reviewed changes
5.1.0 Outdated Show resolved Hide resolved
apollo13
apollo13 suggested changes Dec 27, 2020
View reviewed changes
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
django/utils/crypto.py Outdated Show resolved Hide resolved
docs/topics/auth/passwords.txt Outdated Show resolved Hide resolved
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
@timgraham
Copy link
Member

Anthony, it sounds like you've made changes some locally but haven't pushed them to your branch.

@ryowright
Copy link
Contributor Author

Just pushed recent changes @timgraham. I apologize I was distracted by some other stuff I was working on.

timgraham
timgraham reviewed Jan 3, 2021
View reviewed changes
Copy link
Member

@timgraham timgraham left a comment

Choose a reason for hiding this comment

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

Tests seem rather lacking, for example, there's no testing of password upgrades.

See also our patch review checklist about documentation requirements (release notes and versionadded annotation).

docs/topics/auth/passwords.txt Outdated Show resolved Hide resolved
docs/topics/auth/passwords.txt Outdated Show resolved Hide resolved
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
tests/auth_tests/test_hashers.py Show resolved Hide resolved
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
tests/auth_tests/test_hashers.py Show resolved Hide resolved
Base automatically changed from master to main March 9, 2021 06:21
felixxm
felixxm reviewed Jul 16, 2021
View reviewed changes
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
apollo13
apollo13 reviewed Jul 16, 2021
View reviewed changes
django/contrib/auth/hashers.py Show resolved Hide resolved
django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
docs/topics/auth/passwords.txt Outdated Show resolved Hide resolved
docs/topics/auth/passwords.txt Outdated Show resolved Hide resolved
@felixxm felixxm force-pushed the ticket_32275 branch 2 times, most recently from ae71eb8 to 8fec2aa Compare July 16, 2021 09:08
felixxm
felixxm reviewed Jul 16, 2021
View reviewed changes
Copy link
Member

@felixxm felixxm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@ryowright Thanks for all your efforts ⭐ Do you have time to keep working on this? If not, I can push this to the finish line.

I rebased, squashed commit, and pushed edits.

I think we should add a separate section Using Scrypt with Django to docs. Also, releases notes are missing and test_scrypt_upgrade() doesn't work.

EDIT: test_scrypt_upgrade() fixed.

django/contrib/auth/hashers.py Show resolved Hide resolved
@felixxm felixxm force-pushed the ticket_32275 branch 2 times, most recently from 0802b33 to 305a3d1 Compare July 16, 2021 10:22
@felixxm felixxm self-assigned this Jul 16, 2021
@felixxm felixxm changed the title Fixed #32275 -- Added scrypt password hasher Fixed #32275 -- Added Scrypt password hasher. Jul 16, 2021
@felixxm felixxm changed the title Fixed #32275 -- Added Scrypt password hasher. Fixed #32275 -- Added scrypt password hasher. Jul 19, 2021
@felixxm
Copy link
Member

felixxm commented Jul 19, 2021

@apollo13 I pushed final edits with docs changes and added release notes. Can you take a look?

apollo13
apollo13 reviewed Jul 19, 2021
View reviewed changes
docs/releases/4.0.txt Outdated Show resolved Hide resolved
smithdc1
smithdc1 reviewed Jul 19, 2021
View reviewed changes
docs/releases/4.0.txt Outdated Show resolved Hide resolved
docs/topics/auth/passwords.txt Outdated Show resolved Hide resolved
cjerdonek
cjerdonek reviewed Jul 19, 2021
View reviewed changes
Copy link
Contributor

@cjerdonek cjerdonek left a comment

Choose a reason for hiding this comment

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

Some comments.

django/contrib/auth/hashers.py Outdated Show resolved Hide resolved
tests/auth_tests/test_hashers.py Outdated Show resolved Hide resolved
tests/auth_tests/test_hashers.py Outdated Show resolved Hide resolved
@felixxm
Copy link
Member

felixxm commented Jul 20, 2021

Thanks for reviews. I pushed small edits.

@felixxm felixxm requested a review from apollo13 July 22, 2021 08:28
@ryowright @felixxm
Fixed #32275 -- Added scrypt password hasher.
1783b3c 
Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com>
@felixxm
Copy link
Member

felixxm commented Jul 22, 2021

Thanks y'all for reviews 🚀

@felixxm felixxm merged commit 1783b3c into django:main Jul 22, 2021
@bungoume bungoume mentioned this pull request Oct 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kerkeslager kerkeslager kerkeslager left review comments

@cjerdonek cjerdonek cjerdonek left review comments

@timgraham timgraham timgraham left review comments

@pauloxnet pauloxnet pauloxnet left review comments

@felixxm felixxm felixxm left review comments

@illia-v illia-v illia-v left review comments

@jacobtylerwalls jacobtylerwalls jacobtylerwalls left review comments

@smithdc1 smithdc1 smithdc1 left review comments

@apollo13 apollo13 apollo13 approved these changes

Assignees

@felixxm felixxm

Labels
None yet
Projects
None yet
Milestone
No milestone
10 participants
@ryowright @timgraham @felixxm @apollo13 @kerkeslager @cjerdonek @pauloxnet @illia-v @jacobtylerwalls @smithdc1