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
Implements a rate limit for password resets #13934
Conversation
I would maybe change the limit to 2 per hour? There are valid reasons to try it a second time. |
Are there? If sending the email fails on Matomo side the request get's removed again. So a new request can be done until the email was sent. |
Mail might get not deliviered on recieving end for some spam or missconfiguring reasons (e.g. greylisting, missing forwarding, wrong trusted networks, etc). |
Then it would make sense to allow resending the last request once instead of allowing a complete new one... |
I currently don't see a difference, but that seems fine for me. |
Requesting a new reset means filling out the form again and generating a new token for the reset. That means the email content is slightly another as the link differs |
Ok, it's fine for me to send the exact same mail again. |
I would have also allowed 2 or 3 per hour as it can be useful eg if mailbox is full etc but if it otherwise works within an hour again should be fine. Could have been also useful to block per IP but not needed (to avoid someone requesting forget password feature every hour for all users and basically blocking the feature, not really an issue though) |
4672561
to
6c86af0
Compare
@tsteur Wondering what would be the userfriendliest and securest implementation.
|
Ideally it would use the new brute force feature added in 3.8.0. We would add a new But it's a bit more work and #13813 is not even scheduled yet and not really important so would be too much work for now. I would probably simply allow 3 calls per hour for now or whatever is easiest. |
@tsteur guess make sense to reuse that. This was just a quick approach to implement a simple rate limit. Maybe we should simply close this and implement it properly later? |
This PR fixes the most important issue (you can endlessly spam users), so I would say this is good for now. And I wouldn't create a too complex logic as I still think that one day the password reset should be replaced with the "normal" way (#11071) and then there wouldn't be a difference between resending a reset and sending a new reset. |
just a quick feedback: reckon it should be possible to request again the password reset in the hour. For example if the mailserver was buggy and email didn't get sent, or any other problem. 3 times per hour sounds good and would not be considered spammy? |
@sgiehl I set it back to |
9f2f6ee
to
0bf3839
Compare
Changed it so only 3 requests are allowed per hour. |
Any chance to create an integration test for this @sgiehl ? |
0bf3839
to
bbe17f4
Compare
I've added some tests |
f57f140
to
709117c
Compare
currently only prevents from requesting new password resets within one hour. Maybe we could even increase the time limit.
fixes #13813