Add a password reset cooldown

[sebastian-lenz]

Description

Add a password reset cooldown to prevent the possibility of exploiting the password reset function

Related issues

#17337

[brandonkelly]

Couple issues with this:

• I don’t think we should be repurposing verificationCodeIssuedDate for this.
• It could introduce a new user enumeration vector, as the way it’s coded, you’d get an exception thrown immediately whenever you submit a reset-password form for a valid username/email multiple times, but not for invalid usernames/emails.

Given the second point, it might make more sense to simply set a rate limit on the users/send-password-reset-email action, regardless of which username/email is provided.

[sebastian-lenz]

Hi @brandonkelly — thanks for looking into this!

It seems like we're considering two different approaches here, so it would be helpful to know which direction you'd prefer before I update my pull request.

I don’t think we should be repurposing verificationCodeIssuedDate for this.

My intention was to avoid introducing a new database column just to track password resets. verificationCodeIssuedDate is currently only set via Users::setVerificationCodeOnUser, which is only called by Users::getEmailVerifyUrl and Users::getPasswordResetUrl. That’s why I thought it would be safe to reuse it for implementing a cooldown timer. That said, if you'd rather keep it separate, I’m happy to add a dedicated column instead.

It could introduce a new user enumeration vector [...]

I've updated the pull request so that, when the cooldown is active, it now fails silently instead of throwing a new exception. I also added an error boundary around the call to Users::_getUserRecordById to ensure no internal details are leaked. Unless preventUserEnumeration is disabled, I don’t believe this introduces a new enumeration vector — but let me know if you see something I missed.

[...] it might make more sense to simply set a rate limit on the users/send-password-reset-email action [...]

That’s a good idea too. From what I can tell, Yii doesn’t provide built-in support for request rate limiting, so we’d likely need to implement it ourselves. If that’s the preferred route, I’d be glad to work on a separate PR for that.