security: process account recovery requests asynchronously#74
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3d7ce1f3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| log.info("Password reset email sent - username: {}, email: {}", | ||
| username, | ||
| LoggingUtil.maskEmail(email)); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
When createTokenAndSendResetEmail throws after PasswordResetService.createPasswordResetTokenForEmail has set actionType/userId but before it clears them (for example a token repository save failure), this catch logs and then leaves the MDC on the reused account-recovery-* executor thread. Previously the request interceptor cleared MDC at the end of the HTTP request, but this work now runs outside that lifecycle, so later recovery tasks on the same thread can be logged with the wrong user context unless the async task clears MDC in a finally block.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Noticed the forgot-password endpoint answers almost instantly for unknown accounts but only after a token insert plus an email-provider round trip for real ones, so response timing leaks account existence even though the message is generic. Moved all the account-dependent work into a new AccountRecoveryService running on a bounded async executor (AbortPolicy on purpose — CallerRunsPolicy would put the work back on the request thread), so the controller now returns the generic message immediately either way; a rejected submission is logged and still gets the same reply. One caveat: the bounded in-memory executor reduces the timing side channel but is not a durable message queue, so queued work can be lost during application shutdown or restart. Reworked the controller tests and added service, masking and executor wiring tests —
mvn testwent from 185 to 193 tests, all passing (0 failures, 0 errors).