Background.
I know this is an 'old' issue, but all solutions I've seen are simply to recover the admin's password versus a regular user. Previous threads are as follows:
http://drupal.org/node/729064
http://drupal.org/node/1023440
Here's the situation. A user tries to login 5 times. They get blocked. They see a warning to simply try again later or request new password. They request said password and get a one time URL. They successfully login and change password. They logout and then log back in. They are still blocked by flood control.
Desired result.
Upon successful login via remote URL, remove a users record from flood control checking.
Steps to recreate:
- Download and install vanilla version of Drupal 7.10
- Create test user (non-admin).
- Try logging in 5 times with random passwords
- Go until you get the following error message: "Sorry, there have been more than 5 failed login attempts for this account. It is temporarily blocked. Try again later or request a new password."
- Request a new password and receive via email.
- Use one time URL, login, enter new password, save.
- Logout
- Try logging in again and see the error message again: "Sorry, there have been more than 5 failed login attempts for this account. It is temporarily blocked. Try again later or request a new password."
Motivation
For a normal user, this is surprising behavior. They did all the necessary steps and are still not able to access their account. Any attempts to access after a reset are merely adding more failed attempts to the flood control table and making it worse.
If there is a known solution, please send along. Thanks!
Comments
Comment #1
rickmanelius CreditAttribution: rickmanelius commentedActually I think a better outcome would be to remove any failed attempts upon successful reset of a password... because it's possible to use a one time login and not change that, leaving flood control as a viable option (particularly if the person registering the password reset wasn't actually in control of said email/account).
Comment #2
rickmanelius CreditAttribution: rickmanelius commentedin function user_login_final_validate($form, &$form_state) {
We see the following to clear a user's flood control
// Clear past failures for this user so as not to block a user who might
// log in and out more than once in an hour.
flood_clear_event('failed_login_attempt_user', $form_state['flood_control_user_identifier']);
However in the actual user edit form
user_account_form
I don't see where the password submission is occurring. Upon successfully updating one's profile, would it be prudent to run another flood_clear_event so that someone updating their information through a one-time login can prevent lock out?
Comment #3
lukusSame problems here.
Comment #4
rickmanelius CreditAttribution: rickmanelius commentedI don't mind writing a patch here, but if any core contributor can at least chime in and say what would or would not be acceptable... that would be helpful so I don't goof and submit something that wouldn't pass a security analysis, etc.
My proposal is every submission of the user edit form would result in a clearing of the flood tables for that user, but I'd like to know if there are any use cases that this would create a problem.
Comment #5
diedlikeahero CreditAttribution: diedlikeahero commentedHere's a small patch to resolve this. This will clear out the flood control when someone changes their password.
Comment #7
rickmanelius CreditAttribution: rickmanelius commentedWhen I first tried to patch user.module, I received the following:
So I just manually dropped it in for now. I tried a reset password link and now when I go to the user/%uid/edit page after one time login, I see:
The values field of $form_state is not set yet, but thankfully the $account->name value is.
Therefore I tested this and it seems to work now:
Comment #8
rickmanelius CreditAttribution: rickmanelius commentedConverting #7 into a patch for review
Comment #10
rickmanelius CreditAttribution: rickmanelius commentedI don't fully understand why the patch failed because I used git diff to create the patch. Oddly enough, the previous patch had the same issue when trying to apply it.
Comment #11
rickmanelius CreditAttribution: rickmanelius commentedTrying one more time... and stripping out superfluous docroot directory.
Comment #12
rickmanelius CreditAttribution: rickmanelius commentedIgnore #11. I realized we're trying to load the $account object again even though it's already there... so I simply switched it to a conditional if statement to check the status. Use this one instead.
Comment #13
rickmanelius CreditAttribution: rickmanelius commentedHmm... I think I may have introduced a security hole in #12. I just attempted to reset my password on a test user and it somehow logged me in as user #1. I'm not sure if I simply was mistaken or by using the cached form, something went amiss. It may have been smarter in the end to use the method outlined in the earlier patch.
Putting to needs work until I get more time to test.
PS. My production server is load balanced w/memcache enabled, so I'm not sure if it's related to that and therefore not recreatable on my local machine, etc.
Comment #14
rickmanelius CreditAttribution: rickmanelius commentedReviewing the code again, it shouldn't be an issue because of the $user->uid == $account->uid. And I think I must have accidentially logged in as admin instead of used the reset password function. I've tried to recreate the glitch a few times and I don't see it, so I think it's fine.
Any other eyeballs for #12 would be appreciated.
Comment #15
rickmanelius CreditAttribution: rickmanelius commentedFor what it's worth, I now have it working on my setup now with #12.
Comment #16
jlgreen2012 CreditAttribution: jlgreen2012 commentedIf it helps, here's diedlikeahero's patch reformatted for the bot testing.
Comment #18
jlgreen2012 CreditAttribution: jlgreen2012 commentedComment #20
Matt V. CreditAttribution: Matt V. commentedThis appears to be a duplicate of #992540: Nothing clears the "5 failed login attempts" security message when a user resets their own password, which has a slightly longer history, tests, and a bit more momentum. Plus, the patch there is now written against the most recent development version, which should help it get applied (and backported) more quickly.
Comment #21
Andrei.Sapeshko CreditAttribution: Andrei.Sapeshko commentedflood_clear_event() after new password was saved
Comment #22
Andrei.Sapeshko CreditAttribution: Andrei.Sapeshko commentedComment #23
Andrei.Sapeshko CreditAttribution: Andrei.Sapeshko commentedHere's a small patch to resolve this. This will clear out the flood control when someone changes their password.
Comment #25
poker10 CreditAttribution: poker10 at ActivIT s.r.o. commentedThis was fixed by #2880910: [D7] Nothing clears the "5 failed login attempts" security message when a user resets their own password (as per #20).
Comment #26
poker10 CreditAttribution: poker10 at ActivIT s.r.o. commented