MitchDrage opened a new pull request, #14655:
URL: https://github.com/apache/cloudstack/pull/14655
### Description
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.
Fixes: #13604
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] 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
- [x] 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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
