This PR fixes a second forgot-password request silently doing nothing while the first token is still valid.
Each forgot-password request now issues a new token with a full TTL. The new token replaces the previous one, so only the latest email's link works.
Adds user.password.reset.email.cooldown (seconds, default 300, 0 disables). Requests for the same user within the cooldown send no email and are logged at INFO. The cooldown is claimed before the email is sent and kept if the send fails, so a mail outage does not remove the limit. It is best effort: requests arriving at the same instant can each pass the check. A per-user GlobalLock would close that gap, but each held lock opens its own database connection outside the pool. The lock would be held for the whole SMTP send on an unauthenticated endpoint, so I chose the cooldown claim instead.
Each reset detail write now removes all existing rows for that detail first. Duplicate rows left by parallel requests previously survived every later request and locked the user out, since no emailed link would work.
Adds SMTPMailSender.trySendMail, which returns whether the SMTP server accepted the message.
Why: sendMail logs and swallows every failure, so the reset code cannot tell when an email never went out. Saving a new token after a failed send would invalidate the user's working link with nothing delivered. The token is now saved only after trySendMail returns true.
Other callers: sendMail keeps its void signature and now calls trySendMail, ignoring the result. The send logic, logging and exception handling are unchanged. Its other callers (AlertManagerImpl, ProjectManagerImpl, QuotaAlertManagerImpl, UsageAlertManagerImpl) need no changes, and their existing tests pass.
The forgot-password success message now holds for every outcome, including the cooldown, and does not confirm the account exists. Only en.json is updated; other locales are left for translators.
The wait between reset emails was previously tied to user.password.reset.ttl. It is now set separately by user.password.reset.email.cooldown. Since anyone can request a reset for a known username, repeated requests can replace a user's pending link once per cooldown. The 300-second default keeps that to a nuisance, and the user can always request a fresh link.
Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)
Feature/Enhancement Scale or Bug Severity
Bug Severity
BLOCKER
Critical
Major
Minor
Trivial
Screenshots (if appropriate):
How Has This Been Tested?
Unit tests for the cooldown (first request, within and after the cooldown, disabled, future issue time, invalid value, large value), the send path (new token saved, send failure keeps the existing token and cooldown), duplicate cleanup and cleanup after a reset.
SMTPMailSenderTest covers trySendMail returning true and false. The existing tests for the other sendMail callers (alerts, quota, usage, projects) pass unchanged.
End-to-end on a simulator management server with Mailpit as the SMTP server (cooldown 30s): first request and reset via the link, repeat requests within and after the cooldown, old and reused links, an expired link, an SMTP outage, cooldown disabled, a token from before the upgrade, and bursts of 10 parallel requests.
How did you try to break this feature and the system with this change?
Parallel bursts, an SMTP outage mid-flow, expired and reused links, clock skew between management servers, and a malformed or very large cooldown value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a second forgot-password request silently doing nothing while the first token is still valid.
The wait between reset emails was previously tied to user.password.reset.ttl. It is now set separately by user.password.reset.email.cooldown. Since anyone can request a reset for a known username, repeated requests can replace a user's pending link once per cooldown. The 300-second default keeps that to a nuisance, and the user can always request a fresh link.
Fixes: #13604
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?