How exactly do lockouts work, and should they work that way?

[TuringTux]

I am currently debugging issues with unexpected lockouts. To do so, I am trying to figure out how the lockout process works exactly. My naive intuition was:

• A login attempt for a user with an invalid password increases a counter for this particular user by 1
• A successful login resets the counter to 0
• For any login attempt, it is checked whether the counter is greater than a set amount. If so, the attempt is blocked (even if the password is valid).

After thinking about this for a while, I noticed that this cannot be the whole truth, because then the counter would never be reset. So we also need a timestamp of e.g. the last failed login, that causes the counter to be reset as soon as the last failed login is somewhat in the past.

My question now is: How do the lockouts work exactly? Maybe this would help me figure out if there is a bug in the code causing the unexpected lockouts, or if I just need to alter my expectations.

[figureone]

So the lockout code is split into two portions:

1. We hook into wp_login_failed to update the lockout metadata (either for a specific user, or a global counter for all nonexistent users) after a failed login attempt. This is where the Simple History messages get logged: https://github.com/uhm-coe/authorizer/blob/master/src/authorizer/class-login-form.php#L316-L391
2. During authentication, we prevent a login attempt if a lockout is indicated (either the short lockout or the long lockout): https://github.com/uhm-coe/authorizer/blob/master/src/authorizer/class-authentication.php#L83-L134

It's possible there's a logic error in there so if you want to review it let us know if you find any! Since (1) fires the Simple History message and (2) shows the "too many invalid login attempts" error message when trying to log in, there might be some edge cases where one fires but not the other.

[TuringTux]

Thanks for the links! I'd actually quite like to review it, I'll see that I get a testing instance prepared and will report back as soon as I have found something reproducible (might take a few weeks though, I got a lot on my plate right now).

[TuringTux]

One observation:

When a user provides the correct password, and the last failed login is long enough in the past, the failed login counter is not considered. This is, of course, correct.

The failed login counter, however, is not reset to 0 in this case. This seemed like a bug to me at first, but is actually totally correct (this is also hinted at in the code).

The failed login counter is reset as soon as there is another failed login. If the last failed login is long enough in the past, on a failed login, the counter will be reset to 0 and then immediately increased by 1.

This has two consequences that might not be intuitive at first (at least, they weren't to me):

1. A high number of failed logins for a person might not mean that the person is currently locked out, or even that those logins were recently. The person might have been logging in successfully for weeks, because as long as there isn't at least one invalid password attempt, the counter stays as is.
2. If a person successfully logs in, then logs out, has one failed login shortly after, then a successful one after that, a failed one after that etc. and this all happens faster than the reset duration, the account will still be locked after e.g. 10 invalid attempts, even if they were all closely followed by 10 valid attempts.

Both are not bugs, but something at least I will keep in mind while reviewing.

[figureone]

Fixed in https://github.com/uhm-coe/authorizer/issues/141, closing.