-
Notifications
You must be signed in to change notification settings - Fork 788
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
Prevent password reset brute force #778
base: master
Are you sure you want to change the base?
Conversation
update master
Added messages to support changes in PasswordResetModel::verifyPasswordReset
Added a method named expirePasswordReset to be used with verifyPasswordReset to mark existing password reset requests as expired when it is detected that fake attempts are being used. modified verifyPasswordReset in a few ways. 1. Made the query search for just username instead of the username/verification code combo 2. Added feedback for if user does not exist. 3. Added feedback for if verification code does not exist for selected user 4. Added feedback for if verification code does not match the selected users current code and calls the expirePasswordReset method to current code invalid These changes assume that one bad guess at a verification code is an attempted attack. A better solution might include keeping a tally that way if users click on old link by mistake they don't have to start the process over, but that probably requires using altering the database structure slightly and I don't know if that would be desired, but can be easily added into here if its. Steps need incorporate a tally system: 1. Add field in database users table for number of attempts 2. Add value in config file for max number of attempts 3. Make setPasswordResetDatabaseToken set number of attempts to 0 4. make expirePasswordReset read the current number of attempts. 5.a. make expirePasswordReset update number of attempts 5.b make expirePasswordReset update the timestamp to make it expired
hi thanks, can you please add the database fields to the SQL files, so it works 100% without manual editing ? THanks a lot, cool feature! |
What should the default max number of attempts be? |
Added field in database user table for counting number of attempts made to reset the their password with the current code.
update database install file
Hi, sorry maybe I've missed something but for me this is totally broken, the field "user_password_reset_attempts" is not used in your code. A big thanks for committing cool features, but it's get complicated when features are incomplete / unuseable, so please let's finish this feature first before going on here! |
I haven't finished applying the changes with using the database like you asked. |
Removing DB from this branch so that it is in a completed state.
I have removed the DB changes that way this branch is atleast complete to solving the issue. |
These are changes to address a potential attack method revealed in discussion within #776
These changes make it so that if an attack were to try at guessing a user's reset password verification hash they would only have one chance to do so.
These changes assume that one bad guess at a verification code is an attempted attack. A better solution might include keeping a tally that way if users click on old link by mistake they don't have to start the process over, but that probably requires using altering the database structure slightly and I don't know if that would be desired, but can be easily added into here if its.
Steps need incorporate a tally system:
5.a. make expirePasswordReset update number of attempts
5.b make expirePasswordReset update the timestamp to make it expired