Relationships

Activities

Before the patch was applied, I tried to enter multiple wrong passwords to an account, fast after each other. I got no error message but that the PW were wrong and had no issue to log in with the correct PW, afterwards.

After the patch was applied, I tried the same and got the message: "You hit the login rate limit of 1 login per 5 seconds." if I did this too fast. I could log in with the correct PW after that delay.
-> ok

Further observations:
- I tried this with two accounts from the same computer at the same time, the limit did only count for one account at the same time.

- [correct or wrong] PWs entered during the 5 seconds did not re-set the delay.

Maybe there is room for improvement, but this patch seems to cover the issue of fast automated PW-attacks, while not hindering humans to enter real PWs (as those have to be typed in, anyway, to begin with)
=> OK

I have a somewhat strange feeling to handle email addresses for all accounts (even deleted or blocked ones), but it should be OK. Have those restrictions been left out intentionally?

Apart from this two more minor issues:

* In the project it seems to more common to use NOW() instead of
CURRENT_TIMESTAMP. And even as the latter actually is in the
ANSI standard I know more DBMS that support NOW() instead of
CURRENT_TIMESTAMP.
* Wouldn't it be more precise to call the new column "lastFailedLogin"?

Both aren't critical issues.

Even without the changes the patch is fine with me.

Regards,
Ted
---
BenBE: CARS for the translation.

Regarding the two points mentioned in the review by Ted:

1. The choice of CURRENT_TIMESTAMP in this case was done by the patch author (felixd). No particular preference exists for either one.

2. When the original DB migration script was written the final semantics had yet to be discussed in the team. No update of the script (or patch) based on that was done in order to rename the column afterwards to avoid name mixups.

I try to login with out password => message "Falsche E-Mail-Adresse und/oder falsches Kennwort." => ok
If I just retry id in a short timespan => message "You hit the login rate limit of 1 login per 5 seconds." =>ok

If I wait some time and I try to login in again with wrong credentials I get again message "You hit the login rate limit of 1 login per 5 seconds." => false

After fix has been applied:
Login with false credentials => message "Login failed due to incorrect email address, wrong passphrase or account rate limit of one login per 5 seconds was hit." => ok
Directly new attempt => same message => ok
Waiting for some time and new attempt => same message => ok

The privacy issue mentioned by INOPIAE in ^5359 refers to different behaviour if an account exists or not. The mitigation now uses the same wording regardless of hitting the rate limit or using wrong credentials. This might be somewhat confusing (long error message with several reasons), but it avoids the information leak of primary email addresses.

Review for modified patch -> OK
Ted has been notified of this change for review/confirmation.