Feat/reset password#5
Conversation
- Add Command interface for cross-module write operations - Add CommandBus interface for synchronous command execution - Add CommandHandler interface with type-safe generics - Add CommandResult sealed interface (Success, Error) - Add exception hierarchy (CommandBusException, CommandHandlerNotFoundException, CommandExecutionException) - Add comprehensive tests for CommandResult (17 tests passing) Implements CQRS pattern for synchronous writes across bounded contexts. Mirrors QueryBus pattern for consistency. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add Spring-based CommandBus implementation with auto-discovery - Constructor injection discovers all CommandHandler beans - Fail-fast validation: exactly one handler per command type - Comprehensive logging (debug, info, error) - Type-safe handler lookup and execution - 9 tests covering registration, duplicate detection, execution, errors Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add ChangePasswordCommand with userId and plain password - Add ChangePasswordCommandHandler in user module - Handler validates password strength via PasswordStrengthPolicy - Handler hashes password via PasswordHasher - Handler changes password and persists user - Returns CommandResult for error handling - 5 tests covering success, validation failure, user not found, save failure Enables cross-module password changes via CommandBus. Password concerns properly scoped to user module. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Remove DomainEventPublisher dependency from ResetPasswordService - Add CommandBus dependency - Execute ChangePasswordCommand synchronously instead of emitting events - Check command result before marking token as used - All operations within single @transactional method - 6 tests covering success, failures, token validation BREAKING CHANGE: Password changes are now synchronous and transactional. Token is marked as used ONLY after password change succeeds. Fixes critical bug where password hash was generated but never used. Prevents race condition where token marked used before password changes. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ARCHITECTURE.md changes: - Add comprehensive cross-module communication section - Document QueryBus (synchronous reads) - Document CommandBus (synchronous writes) - Document Domain Events (asynchronous side effects) - Add decision matrix with clear guidance - Add examples and anti-patterns to avoid PASSWORD_RESET_IMPLEMENTATION_GUIDE.md changes: - Mark Step 2.2 as completed with CommandBus refactoring - Update test examples to show CommandBus usage - Update implementation to show synchronous password changes - Mark Step 8 (event-based integration) as obsolete - Note session revocation needs updating - Update checklist to reflect CommandBus completion Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…cture) Domain layer: - Add PasswordResetToken aggregate with validation - Add ResetTokenId and ResetTokenHash value objects - Add PasswordResetRequestedEvent and PasswordResetCompletedEvent - Add InvalidResetTokenException - 26 domain tests passing Application layer: - Add RequestPasswordResetUseCase and service - Add ResetPasswordUseCase and service (uses CommandBus) - Add VerifyResetTokenUseCase and service - Add DTOs and port interfaces - Add GetUserByEmailQuery and handler Infrastructure layer: - Add database migration V7 for password_reset_tokens table - Add JPA entity, repository, and adapter - Add MacResetTokenHasher (HMAC-SHA256) - Add SecureResetTokenGenerator (32-byte random) - Configure token expiration and pepper in application.yml All tests passing. Ready for web layer integration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Agent documentation: - Add CODE_STYLE.md with strict comment policy - Add TESTING.md with layer-specific testing guidelines - Add WORKFLOW.md with git conventions and PR process - Move ARCHITECTURE.md to docs/agent/ CLAUDE.md updates: - Simplify to lean operating guide - Reference detailed docs in docs/agent/ - Add cost control reminders - Clarify TDD and verification requirements Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Format Query, QueryBus, QueryHandler, QueryResult interfaces - Format QueryBus exception hierarchy - Format SpringQueryBus and tests - Format query contracts and handlers - Format repository interfaces No functional changes, only formatting compliance. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Format LoginService, LogoutService, RefreshTokenService - Format corresponding tests - Format PasswordHasher port and Argon2 implementation - Format AuthUser and IdentityExceptionCodes No functional changes, only formatting compliance. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Move architecture docs to docs/agent/ for Claude Code - Delete old docs/ root-level files - Consolidate agent-specific guidelines Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Common Command/CQRS Infrastructure common/src/main/java/com/tickon/common/commands/Command.java, .../CommandBus.java, .../CommandHandler.java, .../CommandResult.java, .../exceptions/* |
Adds Command API, CommandBus/CommandHandler contracts, sealed CommandResult with Success/Error helpers, and a small CommandBus exception hierarchy. |
Spring Command Bus & Tests services/identity-service/src/main/java/com/tickon/identity/shared/infrastructure/SpringCommandBus.java, services/identity-service/src/test/.../SpringCommandBusTest.java |
Implements Spring-based command bus with handler registration, timing/logging and error wrapping; comprehensive unit tests. |
Query infra tweaks common/src/main/java/com/tickon/common/queries/*, services/.../SpringQueryBus.java |
Removed Javadoc; changed QueryResult.map/orElseThrow behavior to propagate NotFound/Error and throw on non-success. |
Password reset domain & events services/identity-service/src/main/java/com/tickon/identity/auth/domain/PasswordResetToken.java, .../valueobjects/ResetTokenId.java, .../ResetTokenHash.java, .../events/*, .../exceptions/InvalidResetTokenException.java |
Adds PasswordResetToken aggregate, token id/hash value objects, request/completed domain events, and InvalidResetTokenException. |
Application use-cases & services services/identity-service/src/main/java/com/tickon/identity/auth/application/services/*.java, .../ports/in/*UseCase.java, .../dto/*Command.java, *Result.java |
Adds Request/Verify/Reset use-case interfaces and implementations; transactional flows include QueryBus lookups, token gen/hash, persistence, CommandBus-driven password change, and event publishing. |
Persistence, entities & mappers services/identity-service/src/main/java/.../persistence/JpaPasswordResetTokenRepository.java, PasswordResetTokenRepositoryAdapter.java, entities/PasswordResetTokenEntity.java, mappers/PasswordResetTokenPersistenceMapper.java |
Adds JPA entity/table mapping, repository queries (findByTokenHash, invalidate), adapter mapping between entity and domain. |
Token hashing & generation .../infrastructure/security/MacResetTokenHasher.java, .../SecureResetTokenGenerator.java |
Implements HMAC-SHA256 peppered hasher and 256-bit secure random token generator (URL-safe Base64). |
Email integration & config services/identity-service/src/main/java/.../email/EmailProperties.java, .../SendGridEmailSender.java, services/identity-service/pom.xml, application.yml |
Adds SendGrid dependency, email properties binding, SendGrid-based EmailSender implementation, and password-reset config (expiration, pepper). |
Async event handlers .../infrastructure/events/SendPasswordResetEmailHandler.java, RevokeSessionsOnPasswordResetHandler.java |
Async listeners: send reset email on request event; revoke user sessions on password-reset-completed event. |
Session revocation extensions .../application/ports/out/SessionRepository.java, .../infrastructure/persistence/JpaSessionRepository.java, SessionRepositoryAdapter.java, .../domain/valueobjects/RevokeReason.java |
Adds revokeAllByUserId(UserId, Instant, RevokeReason), JPA query for active sessions, adapter implementation, and PASSWORD_RESET revoke reason. |
Web API & DTOs .../infrastructure/web/PasswordResetController.java, .../dto/*.java, .../mappers/PasswordResetMapper.java |
Adds REST endpoints for request/verify/reset, DTOs with validation, and mapper helpers to application commands. |
User lookup by email services/identity-service/src/main/java/.../shared/contracts/queries/GetUserByEmailQuery.java, .../user/application/queryhandlers/GetUserByEmailQueryHandler.java, .../persistence/repositories/JpaUserRepository.java, UserRepositoryAdapter.java |
Adds GetUserByEmail query, handler and repository method to find users by email; handler returns QueryResult.NotFound when absent. |
PasswordHasher API change services/identity-service/src/main/java/.../ports/out/PasswordHasher.java, .../user/infrastructure/security/Argon2PasswordHasher.java |
Adds PasswordHash hash(String) to PasswordHasher interface; Argon2 implementation updated to implement both relevant interfaces. |
Email VO migration common/src/main/java/com/tickon/common/identity/domain/valueobjects/Email.java, services/... (imports updated) |
Moves Email VO to com.tickon.common.identity.domain.valueobjects, updates imports across code/tests, and changes invalid-email error to IllegalArgumentException. |
DB migration services/identity-service/src/main/resources/db/migration/V7__add_password_reset_tokens.sql |
Adds password_reset_tokens table, constraints, and indexes (unique token_hash, user_id index, created_at, partial index for active tokens). |
Tests: extensive coverage common/src/test/.../CommandResultTest.java, services/identity-service/src/test/... (many new tests) |
Adds/updates unit tests for CommandResult, command bus, password reset services, token domain/value objects, ChangePassword handler, and related fixtures. |
Documentation reorg docs/*.md (deleted legacy), docs/agent/* (new), docs/plans/_template.md, CLAUDE.md |
Removes legacy ARCHITECTURE/CONTRIBUTING/TESTING/WORKFLOW docs; introduces agent-focused architecture, code style, testing, workflow guides and a planning template; updates CLAUDE.md to a lean operating guide. |
Sequence Diagram(s)
sequenceDiagram
actor User
participant Controller as PasswordResetController
participant Service as RequestPasswordResetService
participant QueryBus as QueryBus
participant UserQuery as GetUserByEmailQueryHandler
participant Repository as ResetTokenRepository
participant Hasher as ResetTokenHasher
participant Generator as ResetTokenGenerator
participant EventPublisher as DomainEventPublisher
User->>Controller: POST /v1/auth/password-reset/request {email}
Controller->>Service: requestPasswordReset(RequestPasswordResetCommand)
Service->>QueryBus: execute(GetUserByEmailQuery)
QueryBus->>UserQuery: handle(GetUserByEmailQuery)
UserQuery-->>QueryBus: QueryResult<UserAuthDataDTO>
alt User Found
Service->>Generator: generateSecureToken()
Generator-->>Service: plainToken
Service->>Hasher: hash(plainToken)
Hasher-->>Service: ResetTokenHash
Service->>Repository: invalidateAllForUser(userId, now)
Repository-->>Service: (ok)
Service->>Repository: save(PasswordResetToken)
Repository-->>Service: (persisted)
Service->>EventPublisher: publish(PasswordResetRequestedEvent)
EventPublisher-->>Service: (published)
else User Not Found
Note over Service: return generic success (no side effects)
end
Service-->>Controller: RequestPasswordResetResult.success()
Controller-->>User: 200 {message}
sequenceDiagram
actor User
participant Controller as PasswordResetController
participant Service as ResetPasswordService
participant Hasher as ResetTokenHasher
participant Repository as ResetTokenRepository
participant CommandBus as CommandBus
participant PasswordHandler as ChangePasswordCommandHandler
participant EventPublisher as DomainEventPublisher
participant SessionRevoke as RevokeSessionsOnPasswordResetHandler
User->>Controller: POST /v1/auth/password-reset/reset {token, newPassword}
Controller->>Service: resetPassword(ResetPasswordCommand)
Service->>Hasher: hash(token)
Hasher-->>Service: ResetTokenHash
Service->>Repository: findByTokenHash(hash)
Repository-->>Service: Optional<PasswordResetToken>
alt Token Found & Valid
Service->>CommandBus: execute(ChangePasswordCommand)
CommandBus->>PasswordHandler: handle(ChangePasswordCommand)
PasswordHandler-->>CommandBus: CommandResult.Success<Void>
Service->>Repository: save(token.markAsUsed(now))
Repository-->>Service: (persisted)
Service->>EventPublisher: publish(PasswordResetCompletedEvent)
EventPublisher-->>Service: (published)
EventPublisher->>SessionRevoke: async handle(PasswordResetCompletedEvent)
SessionRevoke->>SessionRevoke: revokeAllByUserId(userId, now, PASSWORD_RESET)
else Token Not Found/Invalid
Service-->>Controller: throw InvalidResetTokenException
end
Controller-->>User: 200 OK / error
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.51% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'Feat/reset password' directly and clearly summarizes the main change: implementing reset password functionality. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/reset-password
Comment @coderabbitai help to get the list of available commands and usage tips.
🧪 Test Results210 tests 210 ✅ 12s ⏱️ Results for commit c192b30. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In `@common/src/main/java/com/tickon/common/commands/CommandResult.java`:
- Around line 20-25: The toOptional() implementation in CommandResult uses
Optional.of(...) which throws NPE when Success<T> holds a null value; update the
toOptional() method to return Optional.ofNullable(success.value()) for the
Success<T> branch (leaving Error<T> -> Optional.empty()) so a Success containing
null maps to Optional.empty() safely; locate the toOptional() default method in
interface CommandResult and replace Optional.of(...) with
Optional.ofNullable(...).
In `@docs/agent/ARCHITECTURE.md`:
- Around line 94-98: Update the CommandBus section to explicitly define
transaction boundaries: state whether domain events referenced in the use case
coordination step (step 4) are persisted/published inside the same transaction
as CommandBus operations (CommandBus returning CommandResult<T>) or are deferred
until post-commit; describe failure semantics when synchronous handlers fail;
and document recommended patterns to ensure consistency (e.g., Outbox pattern:
persist events within the same DB transaction and publish asynchronously,
TransactionalEventListener-like post-commit publishing, or a two-phase approach
with compensating actions). Reference CommandBus, CommandResult<T>, and the
domain event publishing step in the use case coordination so readers know where
each pattern applies and what to expect on handler failures.
- Line 88: Update the ARCHITECTURE.md to define explicit error-handling
contracts for QueryResult<T> and CommandResult<T>: specify a canonical Result<T,
E> structure with clear success/failure discriminants (e.g., isSuccess, value,
error), describe semantics for error propagation (when to throw vs. return
Result), enumerate standard error categories and codes or an exception hierarchy
(e.g., NotFound, ValidationError, DomainError, InfrastructureError) and include
short usage patterns showing how handlers produce and callers consume
QueryResult<T> and CommandResult<T> (including mapping, bubbling, and
translating infra errors to domain errors); reference the symbols
QueryResult<T>, CommandResult<T>, and Result<T, E> so readers can locate the
definitions.
- Line 19: Update the line under shared/domain/ that reads "shared abstractions
(not owned models)" to include explicit guidance: enumerate which artifacts are
safe to put in shared/domain/ (e.g., marker interfaces, DTOs, serializer
contracts, base types without behavior, common enums) and which must remain
module-private (e.g., entities, aggregates, value objects containing business
rules, domain services, repository interfaces tied to a bounded context), and
add a short rule-of-thumb sentence and one concrete example for each category to
prevent domain model leakage; reference the existing phrase "shared abstractions
(not owned models)" and the directory name "shared/domain/" so the change is
easy to locate and review.
In `@services/identity-service/PASSWORD_RESET_IMPLEMENTATION_GUIDE.md`:
- Around line 1152-1161: The sample logs currently include the user's email via
to.value() in the success, failure and exception paths (log.info, log.error and
the catch(IOException e) block); replace that direct PII logging with a non-PII
representation (either omit the email or call a maskEmail helper such as
maskEmail(to.value()) or log a non-identifying user id) and use that masked
value in the three log calls so PII is not written to logs; implement
maskEmail(email) near the email-sending code and use it in the log.info and both
log.error calls as well as the exception log.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/application/dto/ResetPasswordCommand.java`:
- Line 3: The record ResetPasswordCommand currently exposes sensitive fields
resetToken and newPassword via the auto-generated toString(); override
toString() on the ResetPasswordCommand record to return a redacted
representation (e.g., mask or replace resetToken and newPassword with fixed
placeholders) so sensitive values never appear in logs or stack traces,
implement the override directly inside the ResetPasswordCommand record
definition.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/application/services/RequestPasswordResetService.java`:
- Line 63: RequestPasswordResetService currently generates plainToken via
resetTokenGenerator.generateSecureToken() but never delivers it; update the flow
so the plain token is propagated: modify RequestPasswordResetResult.success(...)
to accept and carry the plainToken (and update RequestPasswordResetResult type)
and return that token from RequestPasswordResetService after generating and
persisting the hashed token, or alternatively emit a PasswordResetRequested
event containing plainToken for mail adapters to consume; ensure the controller
or event publisher consumes the new token value (fix same omission around the
later block noted at lines 76-79) and adjust any callers/tests accordingly.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/application/services/ResetPasswordService.java`:
- Around line 59-61: ResetPasswordService currently calls token.markAsUsed(),
resetTokenRepository.save(token), and token.clearEvents() but never publishes
the domain events; inject the DomainEventPublisher (e.g., eventPublisher) into
ResetPasswordService, and after saving the token call
eventPublisher.publishAll(token.pullDomainEvents() or
eventPublisher.publishAll(token)) (matching the module pattern used by
RequestPasswordResetService/LoginService/LogoutService/RefreshTokenService)
before calling token.clearEvents() so the PasswordResetCompletedEvent is
published.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/persistence/JpaPasswordResetTokenRepository.java`:
- Around line 16-18: The `@Modifying` bulk update in
JpaPasswordResetTokenRepository (method invalidateAllForUser) can leave stale
entities in the persistence context; modify the `@Modifying` annotation on
invalidateAllForUser to include clearAutomatically = true so the persistence
context is cleared after the update, ensuring loaded PasswordResetTokenEntity
instances reflect the updated usedAt values.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/security/MacResetTokenHasher.java`:
- Around line 27-35: In MacResetTokenHasher.hash, validate the plainToken
argument up front and reject null or blank values by throwing an
IllegalArgumentException with a clear message (e.g., "plainToken must not be
null or blank") before performing any HMAC work; keep the existing HMAC logic
(Mac.getInstance, mac.init, mac.doFinal, ResetTokenHash.from(...)) and preserve
the existing catch that wraps crypto failures in an IllegalStateException.
- Around line 20-24: Remove the insecure default pepper by updating the
MacResetTokenHasher constructor's `@Value` injection to not provide
"changeme-dev-only", so Spring will fail startup when
security.password-reset.token-pepper is missing; keep the existing null/blank
guard and the use of pepper.getBytes(StandardCharsets.UTF_8) in the
MacResetTokenHasher constructor to validate and store the pepper.
In
`@services/identity-service/src/main/java/com/tickon/identity/user/application/commandhandlers/ChangePasswordCommandHandler.java`:
- Around line 30-50: The handle method in ChangePasswordCommandHandler is
catching all Exceptions and converting passwordPolicy.validate failures into a
generic error; update ChangePasswordCommandHandler.handle to catch the
validation exception type thrown by passwordPolicy.validate (e.g.,
PasswordStrengthPolicy.ValidationException or the concrete exception your policy
uses) in a separate catch block, return a CommandResult.Error containing the
validation exception message (or propagate it) so callers receive specific
feedback, and keep a final catch for other unexpected exceptions that logs and
returns the existing generic CommandResult.Error; reference
passwordPolicy.validate, ChangePasswordCommandHandler.handle, and
CommandResult.Error when making the change.
In `@services/identity-service/src/main/resources/application.yml`:
- Around line 46-53: Remove the hardcoded fallback value for token peppers and
enforce presence at startup: delete the default `"changeme-dev-only"` from the
YAML key password-reset.token-pepper and remove any default on
refresh-token-pepper so both properties (refresh-token-pepper and
password-reset.token-pepper) read only from their environment variables; add a
startup validation (e.g., in your application bootstrap or config loader) that
checks these two properties and fails fast with a clear error if either is empty
or missing to prevent accidental use of a development pepper.
🧹 Nitpick comments (17)
services/identity-service/src/main/java/com/tickon/identity/user/infrastructure/security/Argon2PasswordHasher.java (1)
11-12: Decouple user infrastructure from auth application layer.
Argon2PasswordHasherimplements both the user and authPasswordHasherports, creating cross-module coupling where user infrastructure directly depends on the auth application layer. While the implementations currently share identical signatures and a commonPasswordHashtype, this violates the module boundary principle and risks future divergence. Consider moving one port to a shared module or creating a dedicated auth adapter that delegates to this bean.docs/agent/ARCHITECTURE.md (3)
104-108: Address async event failure handling."Fire-and-forget" and "eventual consistency" imply no error handling, which is risky for critical side effects. Without guidance, developers may implement inconsistent failure strategies.
Consider documenting:
- Retry policies for transient failures
- Dead letter queues for persistent failures
- Monitoring and alerting requirements
- When to use compensating transactions vs. manual intervention
- Critical vs. non-critical event classification
82-117: Add testing guidance for cross-module patterns.The document defines three communication patterns but provides no testing strategy. This could lead to fragile integration points and unclear test boundaries.
Suggest adding a testing section covering:
- Unit testing: How to test QueryHandler/CommandHandler in isolation (mocking dependencies)
- Integration testing: How to test cross-module flows end-to-end
- Contract testing: Ensuring query/command contracts remain stable
- Event testing: Testing async event handlers with test event publishers
82-117: Consider security guidance for cross-module communication.No mention of authentication/authorization across module boundaries. Without explicit guidance, security checks may be inconsistent or missing.
Suggest documenting:
- Where to enforce permissions (controller, use case, or handler level)
- How to propagate security context across QueryBus/CommandBus
- Whether commands/queries should carry caller identity
- How to audit cross-module operations
services/identity-service/src/main/java/com/tickon/identity/auth/domain/valueobjects/ResetTokenHash.java (1)
4-8: Minor: redundantisEmpty()check.
isBlank()already returnstruefor empty strings, makingisEmpty()redundant.♻️ Simplified validation
public ResetTokenHash { - if (value == null || value.isEmpty() || value.isBlank()) { + if (value == null || value.isBlank()) { throw new IllegalArgumentException("Reset token hash cannot be null or empty"); } }services/identity-service/src/main/java/com/tickon/identity/auth/application/ports/out/ResetTokenHasher.java (1)
1-7: Clean outbound port definition.Proper hexagonal architecture port returning a domain value object. Single-method interface follows ISP.
Consider whether a constant-time
verify(String plainToken, ResetTokenHash hash)method should be added to prevent timing attacks during token validation, rather than relying on callers to re-hash and compare.services/identity-service/PASSWORD_RESET_IMPLEMENTATION_GUIDE.md (1)
1535-1543: Add language specifiers to fenced code blocks.Per static analysis: lines 1535 and 1540 have code blocks without language identifiers, which affects syntax highlighting and linting.
-``` +```text ResetPasswordService → emit PasswordResetCompletedEvent → ChangePasswordOnResetEventHandler → change password-
+text
ResetPasswordService → execute ChangePasswordCommand via CommandBus → ChangePasswordCommandHandler → change passwordservices/identity-service/src/main/java/com/tickon/identity/auth/application/services/ResetPasswordService.java (1)
35-62: Consider consolidating token validation.Lines 44-50 duplicate the expired/used checks that
markAsUsed(Line 59) also performs internally (per domain code). The explicit checks throwInvalidResetTokenExceptionwhilemarkAsUsedthrowsIllegalStateException. This is defensive, but the dual validation paths may confuse maintainers.Option: rely on domain validation with exception mapping
- if (token.isExpired(now)) { - throw new InvalidResetTokenException(); - } - - if (token.isUsed()) { - throw new InvalidResetTokenException(); - } - UserId userId = token.userId(); CommandResult<Void> result = commandBus.execute(new ChangePasswordCommand(userId, command.newPassword())); if (result instanceof CommandResult.Error<Void> error) { throw new IllegalStateException("Failed to change password: " + error.message(), error.cause()); } - token.markAsUsed(now); + try { + token.markAsUsed(now); + } catch (IllegalStateException e) { + throw new InvalidResetTokenException(); + }services/identity-service/src/test/java/com/tickon/identity/auth/application/services/RequestPasswordResetServiceTest.java (1)
86-98: Consider adding test for inactive/suspended user.The test verifies behavior when user doesn't exist, but the service receives
UserAuthDataDTOwith astatusfield. If the system should reject resets for inactive users, that scenario warrants a test.services/identity-service/src/main/java/com/tickon/identity/user/application/commandhandlers/ChangePasswordCommandHandler.java (1)
15-16: Missing@Transactionalannotation.This handler performs read-modify-write on
User(Lines 38-42) but lacks transaction demarcation. It currently relies on the caller's transaction (e.g.,ResetPasswordService), which works but is implicit. Adding@Transactionalmakes the boundary explicit and ensures correctness if the handler is invoked from a non-transactional context.Add transaction annotation
+import org.springframework.transaction.annotation.Transactional; + `@Component` +@Transactional public class ChangePasswordCommandHandler implements CommandHandler<ChangePasswordCommand, Void> {services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/persistence/PasswordResetTokenRepositoryAdapter.java (1)
30-33: Consider adding@Transactional(readOnly = true)for read operation.While the current implementation works, annotating read-only operations helps Hibernate optimize by skipping dirty checking and allows database-level read replicas if configured.
Suggested change
`@Override` +@Transactional(readOnly = true) public Optional<PasswordResetToken> findByTokenHash(String tokenHash) { return jpaRepository.findByTokenHash(tokenHash).map(mapper::toDomain); }services/identity-service/src/main/resources/db/migration/V7__add_password_reset_tokens.sql (2)
4-4: Redundant UNIQUE constraint.The
UNIQUEconstraint ontoken_hash(line 4) already creates an implicit unique index. The explicitCREATE UNIQUE INDEX idx_password_reset_tokens_token_hash(lines 14-15) is redundant and adds maintenance overhead.Remove one of them—typically keep the explicit index if you want a named index, or keep the column constraint for simplicity.
♻️ Option: Remove the explicit index since the column constraint suffices
--- Remove lines 13-15 --- Index for lookups by token hash (most common query) -CREATE UNIQUE INDEX idx_password_reset_tokens_token_hash - ON password_reset_tokens(token_hash);Also applies to: 14-15
5-5: Consider ON DELETE behavior for user_id FK.When a user is deleted, orphan tokens will remain. Consider
ON DELETE CASCADEto auto-cleanup, orON DELETE SET NULLif you need audit trails (would require makinguser_idnullable).services/identity-service/src/main/java/com/tickon/identity/auth/application/services/RequestPasswordResetService.java (1)
55-60: Unchecked cast on QueryResult.Line 60 casts to
QueryResult.Successassuming onlyNotFoundorSuccessstates. IfQueryResult.Errorexists, this throwsClassCastException.Pattern matching or explicit type checks improve safety.
♻️ Safer pattern with instanceof
- if (result instanceof QueryResult.NotFound) { - - return RequestPasswordResetResult.success(); - } - - UserAuthDataDTO userDTO = ((QueryResult.Success<UserAuthDataDTO>) result).value(); + if (!(result instanceof QueryResult.Success<UserAuthDataDTO> success)) { + return RequestPasswordResetResult.success(); + } + + UserAuthDataDTO userDTO = success.value();services/identity-service/src/main/java/com/tickon/identity/auth/domain/PasswordResetToken.java (1)
14-32: Missing equals/hashCode for aggregate identity.Aggregate roots typically implement
equals/hashCodebased on their identifier for proper collection behavior and comparison semantics.♻️ Add identity-based equals/hashCode
+ `@Override` + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof PasswordResetToken that)) return false; + return Objects.equals(id, that.id); + } + + `@Override` + public int hashCode() { + return Objects.hash(id); + }services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/persistence/entities/PasswordResetTokenEntity.java (2)
11-33: Public fields break encapsulation.JPA entities typically use private/package-private fields with accessors. Public fields expose internal state directly, reducing flexibility for future changes (e.g., adding validation, lazy loading).
♻️ Use private fields with getters/setters
- `@Id` - public UUID id; - - `@Column`(name = "token_hash", nullable = false, unique = true) - public String tokenHash; + `@Id` + private UUID id; + + `@Column`(name = "token_hash", nullable = false, unique = true) + private String tokenHash; + + // Add getters/setters...
35-46: Lifecycle callbacks useInstant.now()directly.This couples timestamps to system clock, making integration tests harder to control. Consider injecting timestamps via the mapper/service layer instead of relying on JPA callbacks if deterministic testing is needed.
Minor concern—acceptable if timestamps don't require precise test control.
| @Override | ||
| public ResetTokenHash hash(String plainToken) { | ||
| try { | ||
| Mac mac = Mac.getInstance(HMAC_ALG); | ||
| mac.init(new SecretKeySpec(pepperBytes, HMAC_ALG)); | ||
| byte[] digest = mac.doFinal(plainToken.getBytes(StandardCharsets.UTF_8)); | ||
| return ResetTokenHash.from(Base64.getUrlEncoder().withoutPadding().encodeToString(digest)); | ||
| } catch (Exception e) { | ||
| throw new IllegalStateException("Unable to compute reset token HMAC", e); |
There was a problem hiding this comment.
Validate plainToken before hashing.
Null/blank tokens currently get wrapped as an IllegalStateException and could hash empty input. Reject early with a clear argument error.
🔧 Suggested change
`@Override`
public ResetTokenHash hash(String plainToken) {
+ if (plainToken == null || plainToken.isBlank()) {
+ throw new IllegalArgumentException("Reset token must be provided");
+ }
try {
Mac mac = Mac.getInstance(HMAC_ALG);🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 29-29: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Mac.getInstance(HMAC_ALG)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
[warning] 29-29: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Mac.getInstance(HMAC_ALG)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
🤖 Prompt for AI Agents
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/security/MacResetTokenHasher.java`
around lines 27 - 35, In MacResetTokenHasher.hash, validate the plainToken
argument up front and reject null or blank values by throwing an
IllegalArgumentException with a clear message (e.g., "plainToken must not be
null or blank") before performing any HMAC work; keep the existing HMAC logic
(Mac.getInstance, mac.init, mac.doFinal, ResetTokenHash.from(...)) and preserve
the existing catch that wraps crypto failures in an IllegalStateException.
Move Email from user module to common module to fix boundary violation. Email is a shared value object used across multiple bounded contexts. Changes: - Move Email.java to common/identity/domain/valueobjects - Update 22 files to use new import path - Change exception from InvalidEmailException to IllegalArgumentException - Update ARCHITECTURE.md with cross-module import rules Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update event to include plain token for email sending. Event handlers need the unhashed token to generate password reset URLs. Changes: - Add plainToken parameter to PasswordResetRequestedEvent - Update PasswordResetToken.create() signature - Update RequestPasswordResetService to pass plainToken - Fix 23 test calls with new signature Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implement async email sending with SendGrid for password reset flow. Emails are sent via domain event handlers with HTML and plain text templates. Components: - EmailProperties: Spring Boot configuration properties - SendGridEmailSender: Email sender implementation with templates - SendPasswordResetEmailHandler: Async event handler - application.yml: Email configuration with env variables Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Expose password reset flow via REST API with 3 endpoints: - POST /v1/auth/password-reset/request: Request password reset - POST /v1/auth/password-reset/verify: Verify token validity - POST /v1/auth/password-reset/reset: Complete password reset Components: - PasswordResetController: REST controller with validation - 4 DTOs: Request/Response objects with Jakarta validation - PasswordResetMapper: DTO to command mappings Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update Email import to use common module path. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Automatically revoke all active user sessions when password is reset for security. Implemented using event-driven architecture to keep modules decoupled. Changes: - Add PASSWORD_RESET to RevokeReason enum - Add revokeAllByUserId to SessionRepository - Implement revokeAllByUserId in SessionRepositoryAdapter - Add JPA query: findByUserIdAndRevokedAtIsNull - Create RevokeSessionsOnPasswordResetHandler (async event listener) - Emit PasswordResetCompletedEvent from ResetPasswordService - Update ResetPasswordServiceTest with eventPublisher mock All 201 tests passing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@services/identity-service/pom.xml`:
- Around line 74-80: Update the SendGrid dependency declaration
(groupId=com.sendgrid, artifactId=sendgrid-java, version=4.10.2) to use 4.10.3
and then run mvn dependency:tree to enumerate transitive dependencies; for any
vulnerable transitive artifacts reported (e.g., ones tied to CVE-2025-8885,
CVE-2024-34447, CVE-2024-30172) either add explicit safe versions of those
artifacts to the pom or override them via a dependencyManagement section to
force upgraded versions, then re-run the dependency scan and tests to verify no
vulnerable transitive deps remain.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/application/services/RequestPasswordResetService.java`:
- Around line 53-61: The current code in RequestPasswordResetService calls
queryBus.execute(new GetUserByEmailQuery(...)) and only checks for
QueryResult.NotFound before casting to QueryResult.Success, which will
ClassCastException on QueryResult.Error; update the handling to explicitly check
for QueryResult.Success before casting (e.g., if (result instanceof
QueryResult.Success) { UserAuthDataDTO userDTO =
((QueryResult.Success<UserAuthDataDTO>) result).value(); UserId userId = new
UserId(userDTO.id()); ... } else if (result instanceof QueryResult.NotFound) {
return RequestPasswordResetResult.success(); } else /* QueryResult.Error */ {
log the error and return an appropriate failure result }, ensuring you reference
queryBus.execute, GetUserByEmailQuery, UserAuthDataDTO, UserId and
RequestPasswordResetResult in the new branches.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/email/SendGridEmailSender.java`:
- Around line 24-30: The SendGridEmailSender constructor currently only
validates the API key; also validate emailProperties.getFromEmail() and
emailProperties.getResetUrlBase() for null/blank and fail fast by throwing
IllegalStateException with clear messages if either is missing. Update the
SendGridEmailSender(EmailProperties emailProperties) constructor to check
getApiKey(), getFromEmail(), and getResetUrlBase() before creating the SendGrid
client and assigning this.emailProperties so the class never runs with missing
configuration.
- Around line 33-43: sendPasswordResetEmail in SendGridEmailSender currently
concatenates resetToken into resetUrl and injects recipientName and resetUrl
into HTML without escaping; fix by URL-encoding the resetToken when building
resetUrl (use a standard encoder like URLEncoder with UTF-8) and by escaping
HTML-special characters in recipientName and any values interpolated into
buildHtmlContent (and escape resetUrl when used in HTML attributes), ensuring
buildTextContent uses the encoded resetUrl as well; update references in
sendPasswordResetEmail, buildHtmlContent, and buildTextContent to use the
encoded/escaped values.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/events/RevokeSessionsOnPasswordResetHandler.java`:
- Around line 25-30: The async handler
RevokeSessionsOnPasswordResetHandler.handle should defensively catch exceptions
from sessionRepository.revokeAllByUserId so failures aren’t silently swallowed
by the async executor; wrap the revokeAllByUserId call in a try-catch (catch
Exception) and log a clear error (including event.userId().value() and the
exception) via the class logger; do not swallow useful context—include
stacktrace in the log—and keep the method annotated with `@Async/`@EventListener
so behavior stays the same (alternatively note that a global
AsyncUncaughtExceptionHandler can be configured separately).
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/events/SendPasswordResetEmailHandler.java`:
- Line 24: The log in SendPasswordResetEmailHandler currently logs
event.email().value() exposing PII; change it to avoid full email by logging a
non-PII identifier (e.g., event.userId() if available) or a masked email via a
small utility (e.g., maskEmail(String email) that keeps domain and first char
only like "u***@domain.com"); update the log.info call to use the user id or the
masked value and add the maskEmail helper near the handler (or reuse a common
util) so no raw email is written to logs.
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/web/PasswordResetController.java`:
- Around line 43-47: The verifyToken endpoint is accepting blank tokens;
annotate the controller class with `@Validated` and add `@NotBlank` to the token
parameter of the verifyToken method to enforce request validation (consistent
with other endpoints). Update the verifyToken signature that currently calls
verifyResetToken.verifyResetToken(PasswordResetMapper.toVerifyResetTokenCommand(token))
to accept only non-blank tokens and let Spring validation return 400 for invalid
requests; no changes to the mapper or service call are required.
🧹 Nitpick comments (10)
services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/events/SendPasswordResetEmailHandler.java (2)
28-30: Silent failure: Consider alerting or metrics for email delivery failures.Swallowing the exception means failed emails are only visible in logs. For a critical flow like password reset, consider emitting a metric or alert so operational issues are detectable.
26-26: Fragile name extraction from email.Splitting on
@assumes a standard email format and may produce odd names (e.g.,john.doe+tagforjohn.doe+tag@example.com). Acceptable for now, but consider a fallback or user profile lookup if display quality matters.services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/email/EmailProperties.java (1)
6-12: Consider adding validation annotations for required properties.Adding
@Validatedon the class and@NotBlankon required fields (apiKey, fromEmail, resetUrlBase) enables fail-fast startup if configuration is missing.♻️ Proposed enhancement
package com.tickon.identity.auth.infrastructure.email; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.stereotype.Component; +import org.springframework.validation.annotation.Validated; +import jakarta.validation.constraints.NotBlank; `@Component` `@ConfigurationProperties`(prefix = "email.sendgrid") +@Validated public class EmailProperties { + `@NotBlank` private String apiKey; + `@NotBlank` private String fromEmail; private String fromName; + `@NotBlank` private String resetUrlBase;services/identity-service/src/test/java/com/tickon/identity/auth/application/services/VerifyResetTokenServiceTest.java (1)
49-50: Note:PasswordResetToken.create()registers a domain event internally.If domain events are auto-published in tests, this could trigger side effects. Currently fine with mocks, but be aware if integration tests surface unexpected behavior.
services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/email/SendGridEmailSender.java (1)
55-65: Surface SendGrid failures to allow retries/alerting.Non‑2xx responses are logged but treated as success; callers can’t react. Consider throwing (or returning a failure) so higher layers can retry or route to a DLQ.
As per coding guidelines, "Verify proper exception handling and mapping".
services/identity-service/src/main/java/com/tickon/identity/auth/application/services/RequestPasswordResetService.java (1)
74-77: Ensure event publication is transaction-safe.Publishing domain events inside the transaction can send emails even if the DB write rolls back. Ensure
DomainEventPublisheris outbox/transaction‑aware or publish after commit.As per coding guidelines, "Review transaction boundaries".
services/identity-service/src/test/java/com/tickon/identity/auth/application/services/RequestPasswordResetServiceTest.java (1)
59-116: Add an edge‑case test for QueryResult.Error.Once the service handles non‑success results, add a test for
QueryResult.Errorto lock in behavior.As per coding guidelines, "Ensure edge cases are covered".
services/identity-service/src/test/java/com/tickon/identity/auth/domain/PasswordResetTokenTest.java (1)
178-191: Consider verifying the specific null parameter names in exception messages.Currently these assertions only check for
NullPointerExceptionbut don't verify which parameter failed. The domain code uses named null checks (e.g.,"id","tokenHash").💡 Optional enhancement for more precise assertions
- assertThatThrownBy(() -> PasswordResetToken.create(null, ResetTokenHash.from("hash"), USER_ID, EMAIL, ONE_HOUR, - FIXED_INSTANT, "test-plain-token")).isInstanceOf(NullPointerException.class); + assertThatThrownBy(() -> PasswordResetToken.create(null, ResetTokenHash.from("hash"), USER_ID, EMAIL, ONE_HOUR, + FIXED_INSTANT, "test-plain-token")).isInstanceOf(NullPointerException.class).hasMessage("id");services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/persistence/SessionRepositoryAdapter.java (1)
50-61: N+1 save pattern and redundant guard check.
The
isRevoked()check (line 56) is redundant—findByUserIdAndRevokedAtIsNullalready filters to non-revoked sessions.Unlike
revokeAllByFamilyIdwhich uses a bulk@Modifyingquery, this method loads each entity, converts to domain, revokes, converts back, and saves individually. For users with many sessions, this creates N+1 database writes.♻️ Consider a bulk update approach for consistency
Add a bulk update query to
JpaSessionRepository:`@Modifying` `@Query`("UPDATE SessionEntity s SET s.revokedAt = :revokedAt, s.revokeReason = :revokeReason " + "WHERE s.userId = :userId AND s.revokedAt IS NULL") void revokeAllByUserId(`@Param`("userId") UUID userId, `@Param`("revokedAt") Instant revokedAt, `@Param`("revokeReason") String revokeReason);Then simplify the adapter:
- public void revokeAllByUserId(UserId userId, Instant now, RevokeReason reason) { - List<SessionEntity> sessions = jpaRepository.findByUserIdAndRevokedAtIsNull(userId.value()); - for (SessionEntity entity : sessions) { - Session session = mapper.toDomain(entity); - if (!session.isRevoked()) { - session.revoke(now, reason); - jpaRepository.save(mapper.toEntity(session)); - } - } - } + public void revokeAllByUserId(UserId userId, Instant now, RevokeReason reason) { + jpaRepository.revokeAllByUserId(userId.value(), now, reason.name()); + }Note: The current approach preserves domain event emission via
session.revoke(). If those events are required, keep the current pattern but remove the redundant guard.services/identity-service/src/main/java/com/tickon/identity/auth/application/ports/out/SessionRepository.java (1)
19-21: Minor naming inconsistency between similar methods.
revokeAllByFamilyIduses parameterrevokedAtwhilerevokeAllByUserIdusesnow. Both represent the revocation timestamp. Consider aligning for API consistency.💡 Align parameter naming
- void revokeAllByUserId(UserId userId, Instant now, RevokeReason reason); + void revokeAllByUserId(UserId userId, Instant revokedAt, RevokeReason reason);
|
|
||
| <!-- SendGrid Email --> | ||
| <dependency> | ||
| <groupId>com.sendgrid</groupId> | ||
| <artifactId>sendgrid-java</artifactId> | ||
| <version>4.10.2</version> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest version com.sendgrid sendgrid-java maven
💡 Result:
- Latest stable release:
4.10.3(published Sep 18, 2024) [1][2] - Latest version overall (pre-release):
5.0.0-rc.1(published Jun 27, 2024) [3][4]
Maven coordinates
<dependency>
<groupId>com.sendgrid</groupId>
<artifactId>sendgrid-java</artifactId>
<version>4.10.3</version>
</dependency>Sources: [1] [2] [3] [4]
🌐 Web query:
com.sendgrid sendgrid-java 4.10.2 CVE security advisory
💡 Result:
For com.sendgrid:sendgrid-java:4.10.2 there does not appear to be a published direct CVE/GitHub Security Advisory against the SendGrid Java helper library itself (Snyk reports 0 direct vulnerabilities for 4.10.2). [2]
However, 4.10.2 pulls in transitive dependencies that have known CVEs; MvnRepository flags multiple “vulnerabilities from dependencies” for this version, including CVE-2025-8885, CVE-2025-52999, CVE-2024-34447, CVE-2024-30172, CVE-2024-30171, CVE-2024-29857 (and others). [1]
What to do
- Treat this as an indirect (dependency) vulnerability situation: run
mvn dependency:tree(or Gradle equivalent) to identify which transitive artifacts map to those CVEs, then upgrade/override those dependencies. - Consider upgrading SendGrid Java itself to a newer release line (MvnRepository notes newer versions exist), but you still must verify the transitive dependency versions after upgrading. [1]
Sources: MvnRepository entry for 4.10.2 (dependency CVE list) [1]; Snyk package page (no direct vulnerabilities reported) [2].
🌐 Web query:
sendgrid java sdk security vulnerabilities 2024 2025
💡 Result:
SendGrid Java SDK (com.sendgrid:sendgrid-java): 2024–2025 security vulnerabilities
- No “direct” (library-internal) vulnerabilities were reported for
com.sendgrid:sendgrid-javain common vulnerability databases during 2024–2025 (per Snyk’s package page, which shows 0 direct vulnerabilities across recent releases). [1] - Releases in 2024 included 4.10.2 (Feb 14, 2024) and 4.10.3 (Sep 18, 2024) (and 5.0.0-rc.1 (Jun 27, 2024)). [2]
What you may still see: transitive-dependency CVEs
- Some older
sendgrid-javaversions show vulnerabilities coming from dependencies (i.e., not in SendGrid’s own code), as reflected by Maven vulnerability aggregation on specific versions. [3]- Action: run a dependency scan (e.g., OWASP Dependency-Check /
mvn dependency:tree) and upgrade the affected transitive libraries or bumpsendgrid-javato a newer release.
- Action: run a dependency scan (e.g., OWASP Dependency-Check /
Common confusion: “SendGrid” CVEs that are not the Java SDK
Several 2024–2025 CVEs mentioning “SendGrid” apply to WordPress plugins (e.g., “SendGrid for WordPress”, “SMTP for SendGrid – YaySMTP”), not the Java SDK:
- CVE-2024-43965 – SQL injection in SendGrid for WordPress plugin (≤ 1.4). [4]
- CVE-2025-23423 – missing authorization in SendGrid for WordPress plugin (through 1.4). [5]
Sources:
[1] Snyk package page for com.sendgrid:sendgrid-java (security.snyk.io)
[2] Maven Repository listing for com.sendgrid:sendgrid-java versions/dates (mvnrepository.com)
[3] Example Maven version page showing “Vulnerabilities from dependencies” (mvnrepository.com)
[4] CVE-2024-43965 details (SendGrid for WordPress plugin) (cvedetails.com)
[5] NVD entry for CVE-2025-23423 (SendGrid for WordPress plugin) (nvd.nist.gov)
Citations:
- 1: https://security.snyk.io/package/maven/com.sendgrid%3Asendgrid-java?utm_source=openai
- 2: https://mvnrepository.com/artifact/com.sendgrid/sendgrid-java?utm_source=openai
- 3: https://mvnrepository.com/artifact/com.sendgrid/sendgrid-java/4.6.0?utm_source=openai
- 4: https://www.cvedetails.com/cve/CVE-2024-43965/?utm_source=openai
- 5: https://nvd.nist.gov/vuln/detail/CVE-2025-23423?utm_source=openai
Upgrade SendGrid SDK to 4.10.3 and audit transitive dependencies.
Version 4.10.2 (Feb 2024) is outdated; 4.10.3 (Sep 2024) is the latest stable release. While the library itself has no direct CVEs, version 4.10.2 pulls in transitive dependencies with known vulnerabilities (CVE-2025-8885, CVE-2024-34447, CVE-2024-30172, and others). Run mvn dependency:tree to identify affected transitive artifacts and either upgrade SendGrid Java to 4.10.3 or override vulnerable dependencies.
🤖 Prompt for AI Agents
In `@services/identity-service/pom.xml` around lines 74 - 80, Update the SendGrid
dependency declaration (groupId=com.sendgrid, artifactId=sendgrid-java,
version=4.10.2) to use 4.10.3 and then run mvn dependency:tree to enumerate
transitive dependencies; for any vulnerable transitive artifacts reported (e.g.,
ones tied to CVE-2025-8885, CVE-2024-34447, CVE-2024-30172) either add explicit
safe versions of those artifacts to the pom or override them via a
dependencyManagement section to force upgraded versions, then re-run the
dependency scan and tests to verify no vulnerable transitive deps remain.
| QueryResult<UserAuthDataDTO> result = queryBus.execute(new GetUserByEmailQuery(command.email().value())); | ||
|
|
||
| if (result instanceof QueryResult.NotFound) { | ||
|
|
||
| return RequestPasswordResetResult.success(); | ||
| } | ||
|
|
||
| UserAuthDataDTO userDTO = ((QueryResult.Success<UserAuthDataDTO>) result).value(); | ||
| UserId userId = new UserId(userDTO.id()); |
There was a problem hiding this comment.
Guard against QueryResult.Error to avoid ClassCastException.
If queryBus.execute returns QueryResult.Error, the cast to QueryResult.Success will blow up. Handle non‑success explicitly.
✅ Suggested guard
- if (result instanceof QueryResult.NotFound) {
- return RequestPasswordResetResult.success();
- }
-
- UserAuthDataDTO userDTO = ((QueryResult.Success<UserAuthDataDTO>) result).value();
+ if (result instanceof QueryResult.NotFound) {
+ return RequestPasswordResetResult.success();
+ }
+ if (!(result instanceof QueryResult.Success<UserAuthDataDTO> success)) {
+ // TODO: decide whether to return error or generic success + log
+ return RequestPasswordResetResult.success();
+ }
+ UserAuthDataDTO userDTO = success.value();🤖 Prompt for AI Agents
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/application/services/RequestPasswordResetService.java`
around lines 53 - 61, The current code in RequestPasswordResetService calls
queryBus.execute(new GetUserByEmailQuery(...)) and only checks for
QueryResult.NotFound before casting to QueryResult.Success, which will
ClassCastException on QueryResult.Error; update the handling to explicitly check
for QueryResult.Success before casting (e.g., if (result instanceof
QueryResult.Success) { UserAuthDataDTO userDTO =
((QueryResult.Success<UserAuthDataDTO>) result).value(); UserId userId = new
UserId(userDTO.id()); ... } else if (result instanceof QueryResult.NotFound) {
return RequestPasswordResetResult.success(); } else /* QueryResult.Error */ {
log the error and return an appropriate failure result }, ensuring you reference
queryBus.execute, GetUserByEmailQuery, UserAuthDataDTO, UserId and
RequestPasswordResetResult in the new branches.
| @Async | ||
| @EventListener | ||
| public void handle(PasswordResetCompletedEvent event) { | ||
| log.info("Revoking all sessions for user {} due to password reset", event.userId().value()); | ||
| sessionRepository.revokeAllByUserId(event.userId(), clock.instant(), RevokeReason.PASSWORD_RESET); | ||
| } |
There was a problem hiding this comment.
Async event handler lacks error handling.
If revokeAllByUserId throws an exception, it will be silently swallowed by the async executor (unless a custom AsyncUncaughtExceptionHandler is configured). Consider:
- Adding try-catch with explicit error logging
- Configuring a global
AsyncUncaughtExceptionHandler
🛡️ Add defensive error handling
`@Async`
`@EventListener`
public void handle(PasswordResetCompletedEvent event) {
- log.info("Revoking all sessions for user {} due to password reset", event.userId().value());
- sessionRepository.revokeAllByUserId(event.userId(), clock.instant(), RevokeReason.PASSWORD_RESET);
+ log.info("Revoking all sessions for user {} due to password reset", event.userId().value());
+ try {
+ sessionRepository.revokeAllByUserId(event.userId(), clock.instant(), RevokeReason.PASSWORD_RESET);
+ } catch (Exception e) {
+ log.error("Failed to revoke sessions for user {} after password reset", event.userId().value(), e);
+ throw e; // Re-throw for async exception handler
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Async | |
| @EventListener | |
| public void handle(PasswordResetCompletedEvent event) { | |
| log.info("Revoking all sessions for user {} due to password reset", event.userId().value()); | |
| sessionRepository.revokeAllByUserId(event.userId(), clock.instant(), RevokeReason.PASSWORD_RESET); | |
| } | |
| `@Async` | |
| `@EventListener` | |
| public void handle(PasswordResetCompletedEvent event) { | |
| log.info("Revoking all sessions for user {} due to password reset", event.userId().value()); | |
| try { | |
| sessionRepository.revokeAllByUserId(event.userId(), clock.instant(), RevokeReason.PASSWORD_RESET); | |
| } catch (Exception e) { | |
| log.error("Failed to revoke sessions for user {} after password reset", event.userId().value(), e); | |
| throw e; // Re-throw for async exception handler | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/events/RevokeSessionsOnPasswordResetHandler.java`
around lines 25 - 30, The async handler
RevokeSessionsOnPasswordResetHandler.handle should defensively catch exceptions
from sessionRepository.revokeAllByUserId so failures aren’t silently swallowed
by the async executor; wrap the revokeAllByUserId call in a try-catch (catch
Exception) and log a clear error (including event.userId().value() and the
exception) via the class logger; do not swallow useful context—include
stacktrace in the log—and keep the method annotated with `@Async/`@EventListener
so behavior stays the same (alternatively note that a global
AsyncUncaughtExceptionHandler can be configured separately).
| @Async | ||
| @EventListener | ||
| public void handle(PasswordResetRequestedEvent event) { | ||
| log.info("Sending password reset email to {}", event.email().value()); |
There was a problem hiding this comment.
Compliance: Avoid logging user email addresses.
Logging event.email().value() exposes PII. Consider logging a user identifier or masked email (e.g., u***@domain.com) instead.
🔒 Proposed fix
- log.info("Sending password reset email to {}", event.email().value());
+ log.info("Sending password reset email for userId={}", event.userId().value());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.info("Sending password reset email to {}", event.email().value()); | |
| log.info("Sending password reset email for userId={}", event.userId().value()); |
🤖 Prompt for AI Agents
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/events/SendPasswordResetEmailHandler.java`
at line 24, The log in SendPasswordResetEmailHandler currently logs
event.email().value() exposing PII; change it to avoid full email by logging a
non-PII identifier (e.g., event.userId() if available) or a masked email via a
small utility (e.g., maskEmail(String email) that keeps domain and first char
only like "u***@domain.com"); update the log.info call to use the user id or the
masked value and add the maskEmail helper near the handler (or reuse a common
util) so no raw email is written to logs.
| @ResponseStatus(HttpStatus.OK) | ||
| @PostMapping("/verify") | ||
| public VerifyResetTokenResponse verifyToken(@RequestParam String token) { | ||
| var result = verifyResetToken.verifyResetToken(PasswordResetMapper.toVerifyResetTokenCommand(token)); | ||
| return new VerifyResetTokenResponse(result.valid()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "PasswordResetController.java" -type fRepository: sabucds/tickon-api
Length of output: 176
🏁 Script executed:
fd "PasswordResetController" --extension javaRepository: sabucds/tickon-api
Length of output: 174
🏁 Script executed:
rg "PasswordResetController" -t java -lRepository: sabucds/tickon-api
Length of output: 174
🏁 Script executed:
cat -n services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/web/PasswordResetController.javaRepository: sabucds/tickon-api
Length of output: 3107
🏁 Script executed:
fd "PasswordResetMapper" --extension javaRepository: sabucds/tickon-api
Length of output: 178
🏁 Script executed:
fd "VerifyResetTokenUseCase" --extension javaRepository: sabucds/tickon-api
Length of output: 176
🏁 Script executed:
cat -n services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/web/mappers/PasswordResetMapper.javaRepository: sabucds/tickon-api
Length of output: 1301
🏁 Script executed:
cat -n services/identity-service/src/main/java/com/tickon/identity/auth/application/ports/in/VerifyResetTokenUseCase.javaRepository: sabucds/tickon-api
Length of output: 440
🏁 Script executed:
rg "class.*VerifyResetTokenUseCase" -t java -A 20Repository: sabucds/tickon-api
Length of output: 3280
🏁 Script executed:
rg "@NotBlank" -t java -B 2 -A 2Repository: sabucds/tickon-api
Length of output: 4744
🏁 Script executed:
rg "@Validated" -t java -B 2Repository: sabucds/tickon-api
Length of output: 44
Validate the token request parameter.
Empty tokens currently pass through. Add @NotBlank (and @Validated on the controller) to enforce validation of request parameters, consistent with codebase patterns used in other endpoints.
Suggested change
+import jakarta.validation.constraints.NotBlank;
+import org.springframework.validation.annotation.Validated;
...
-@RestController
+@Validated
+@RestController
`@RequestMapping`("/v1/auth/password-reset")
public class PasswordResetController {
...
`@ResponseStatus`(HttpStatus.OK)
`@PostMapping`("/verify")
- public VerifyResetTokenResponse verifyToken(`@RequestParam` String token) {
+ public VerifyResetTokenResponse verifyToken(`@RequestParam` `@NotBlank` String token) {
var result = verifyResetToken.verifyResetToken(PasswordResetMapper.toVerifyResetTokenCommand(token));
return new VerifyResetTokenResponse(result.valid());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ResponseStatus(HttpStatus.OK) | |
| @PostMapping("/verify") | |
| public VerifyResetTokenResponse verifyToken(@RequestParam String token) { | |
| var result = verifyResetToken.verifyResetToken(PasswordResetMapper.toVerifyResetTokenCommand(token)); | |
| return new VerifyResetTokenResponse(result.valid()); | |
| `@ResponseStatus`(HttpStatus.OK) | |
| `@PostMapping`("/verify") | |
| public VerifyResetTokenResponse verifyToken(`@RequestParam` `@NotBlank` String token) { | |
| var result = verifyResetToken.verifyResetToken(PasswordResetMapper.toVerifyResetTokenCommand(token)); | |
| return new VerifyResetTokenResponse(result.valid()); | |
| } |
🤖 Prompt for AI Agents
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/infrastructure/web/PasswordResetController.java`
around lines 43 - 47, The verifyToken endpoint is accepting blank tokens;
annotate the controller class with `@Validated` and add `@NotBlank` to the token
parameter of the verifyToken method to enforce request validation (consistent
with other endpoints). Update the verifyToken signature that currently calls
verifyResetToken.verifyResetToken(PasswordResetMapper.toVerifyResetTokenCommand(token))
to accept only non-blank tokens and let Spring validation return 400 for invalid
requests; no changes to the mapper or service call are required.
- Fix NPE in CommandResult.toOptional() when Success holds null value - Add toString() override to ResetPasswordCommand to redact sensitive fields (token, password) from logs - Add clearAutomatically to @Modifying annotation to prevent stale JPA entities - Remove try-catch from ChangePasswordCommandHandler to let exceptions propagate to GlobalExceptionHandler - Update tests to expect exceptions instead of CommandResult.Error Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@services/identity-service/src/main/java/com/tickon/identity/auth/application/services/ResetPasswordService.java`:
- Around line 55-56: The call to commandBus.execute(new
ChangePasswordCommand(userId, command.newPassword())) ignores the CommandResult
and can hide failures; capture the result and call orElseThrow() (or otherwise
propagate the error) before mutating the reset token state so failures trigger
rollback. Specifically, replace the blind execute call with a checked result
from commandBus.execute(...) and only after result.orElseThrow() proceed to
token.markUsed() and persist via tokenRepository.save(...) (ensuring
token.userId()/UserId and command.newPassword() are used the same way).
- Around line 58-64: The code creates a manual PasswordResetCompletedEvent while
token.markAsUsed(now) already registers the same event on the aggregate and then
clears those events, so replace the manual event publishing with publishing the
token's domain events to follow the project's DDD pattern: after
resetTokenRepository.save(token) call
eventPublisher.publishAll(token.domainEvents()) and then call
token.clearEvents(), and remove the manually constructed
PasswordResetCompletedEvent and eventPublisher.publish(...) call so the
aggregate-driven events are used instead.
In
`@services/identity-service/src/main/java/com/tickon/identity/user/application/commandhandlers/ChangePasswordCommandHandler.java`:
- Around line 15-16: The ChangePasswordCommandHandler currently does a
read-then-write (calls findById then save) without a transaction boundary;
annotate the handler to ensure atomicity by adding Spring's `@Transactional` to
the handle method (or the ChangePasswordCommandHandler class) so the sequence
involving findById and save is executed within a single transaction and prevents
lost updates and inconsistent state.
In
`@services/identity-service/src/test/java/com/tickon/identity/auth/application/services/ResetPasswordServiceTest.java`:
- Around line 175-176: The Spotless violation is caused by a long chained
constructor call in ResetPasswordServiceTest; break the chained constructors and
arguments across multiple lines so each nested constructor/new expression is on
its own line: split the doThrow(new
CommandExecutionException(ChangePasswordCommand.class, new
IllegalArgumentException("Password too
weak"))).when(commandBus).execute(any(ChangePasswordCommand.class)); into a
multi-line statement so new CommandExecutionException(...), new
IllegalArgumentException(...), and the when(...).execute(...) call are on
separate lines, keeping the same symbols (doThrow, CommandExecutionException,
ChangePasswordCommand, IllegalArgumentException, commandBus.execute) and
behavior.
- Around line 70-79: Add verification that the domain event is published after a
successful reset: in ResetPasswordServiceTest, after calling
service.resetPassword(new ResetPasswordCommand(plainToken, newPassword)) and the
existing assertions, verify that eventPublisher.publish(...) was invoked with a
PasswordResetCompletedEvent (or an argument matching that event type); use
verify(eventPublisher).publish(argThat(...)) or an equivalent Mockito matcher to
assert the published event's type and relevant payload.
🧹 Nitpick comments (2)
services/identity-service/src/main/resources/application.yml (1)
54-59: Avoid a production fallback for reset links.
Defaultingreset-url-baseto localhost can silently ship broken reset links if the env var is missed. Prefer failing fast by removing the default.🛠️ Proposed change
- reset-url-base: "${RESET_URL_BASE:http://localhost:3000/reset-password}" + reset-url-base: "${RESET_URL_BASE}"common/src/test/java/com/tickon/common/commands/CommandResultTest.java (1)
172-187: Good null-value edge case coverage.Tests correctly verify that
Success(null)is valid andtoOptional()maps it to empty. Consider adding a test fororElseThrow()onSuccess(null)to document expected behavior (returns null without throwing).
| when(commandBus.execute(any(ChangePasswordCommand.class))).thenReturn(new CommandResult.Success<>(null)); | ||
|
|
||
| // Act | ||
| service.resetPassword(new ResetPasswordCommand(plainToken, newPassword)); | ||
|
|
||
| // Assert | ||
| assertThat(token.isUsed()).isTrue(); | ||
| assertThat(token.usedAt()).isEqualTo(fixedInstant); | ||
| verify(commandBus).execute(any(ChangePasswordCommand.class)); | ||
| verify(resetTokenRepository).save(token); |
There was a problem hiding this comment.
Missing verification of domain event publication.
The test verifies commandBus.execute and resetTokenRepository.save but doesn't verify eventPublisher.publish(...). Add assertion to ensure the PasswordResetCompletedEvent is published on success.
Add verification
verify(commandBus).execute(any(ChangePasswordCommand.class));
verify(resetTokenRepository).save(token);
+ verify(eventPublisher).publish(any(PasswordResetCompletedEvent.class));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| when(commandBus.execute(any(ChangePasswordCommand.class))).thenReturn(new CommandResult.Success<>(null)); | |
| // Act | |
| service.resetPassword(new ResetPasswordCommand(plainToken, newPassword)); | |
| // Assert | |
| assertThat(token.isUsed()).isTrue(); | |
| assertThat(token.usedAt()).isEqualTo(fixedInstant); | |
| verify(commandBus).execute(any(ChangePasswordCommand.class)); | |
| verify(resetTokenRepository).save(token); | |
| when(commandBus.execute(any(ChangePasswordCommand.class))).thenReturn(new CommandResult.Success<>(null)); | |
| // Act | |
| service.resetPassword(new ResetPasswordCommand(plainToken, newPassword)); | |
| // Assert | |
| assertThat(token.isUsed()).isTrue(); | |
| assertThat(token.usedAt()).isEqualTo(fixedInstant); | |
| verify(commandBus).execute(any(ChangePasswordCommand.class)); | |
| verify(resetTokenRepository).save(token); | |
| verify(eventPublisher).publish(any(PasswordResetCompletedEvent.class)); |
🤖 Prompt for AI Agents
In
`@services/identity-service/src/test/java/com/tickon/identity/auth/application/services/ResetPasswordServiceTest.java`
around lines 70 - 79, Add verification that the domain event is published after
a successful reset: in ResetPasswordServiceTest, after calling
service.resetPassword(new ResetPasswordCommand(plainToken, newPassword)) and the
existing assertions, verify that eventPublisher.publish(...) was invoked with a
PasswordResetCompletedEvent (or an argument matching that event type); use
verify(eventPublisher).publish(argThat(...)) or an equivalent Mockito matcher to
assert the published event's type and relevant payload.
| doThrow(new CommandExecutionException(ChangePasswordCommand.class, new IllegalArgumentException("Password too weak"))) | ||
| .when(commandBus).execute(any(ChangePasswordCommand.class)); |
There was a problem hiding this comment.
Fix Spotless formatting violation.
Pipeline failure indicates this line violates formatting rules. Break the chained constructor calls across lines.
Proposed fix
- doThrow(new CommandExecutionException(ChangePasswordCommand.class, new IllegalArgumentException("Password too weak")))
- .when(commandBus).execute(any(ChangePasswordCommand.class));
+ doThrow(new CommandExecutionException(
+ ChangePasswordCommand.class,
+ new IllegalArgumentException("Password too weak")))
+ .when(commandBus).execute(any(ChangePasswordCommand.class));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| doThrow(new CommandExecutionException(ChangePasswordCommand.class, new IllegalArgumentException("Password too weak"))) | |
| .when(commandBus).execute(any(ChangePasswordCommand.class)); | |
| doThrow(new CommandExecutionException( | |
| ChangePasswordCommand.class, | |
| new IllegalArgumentException("Password too weak"))) | |
| .when(commandBus).execute(any(ChangePasswordCommand.class)); |
🤖 Prompt for AI Agents
In
`@services/identity-service/src/test/java/com/tickon/identity/auth/application/services/ResetPasswordServiceTest.java`
around lines 175 - 176, The Spotless violation is caused by a long chained
constructor call in ResetPasswordServiceTest; break the chained constructors and
arguments across multiple lines so each nested constructor/new expression is on
its own line: split the doThrow(new
CommandExecutionException(ChangePasswordCommand.class, new
IllegalArgumentException("Password too
weak"))).when(commandBus).execute(any(ChangePasswordCommand.class)); into a
multi-line statement so new CommandExecutionException(...), new
IllegalArgumentException(...), and the when(...).execute(...) call are on
separate lines, keeping the same symbols (doThrow, CommandExecutionException,
ChangePasswordCommand, IllegalArgumentException, commandBus.execute) and
behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@services/identity-service/src/test/java/com/tickon/identity/auth/application/services/ResetPasswordServiceTest.java`:
- Line 70: The test uses new CommandResult.Success<>(null) inside
ResetPasswordServiceTest (the
when(commandBus.execute(any(ChangePasswordCommand.class))).thenReturn(...) line)
but lacks an import for CommandResult; add the appropriate import for
CommandResult at the top of the test (or replace the usage with the fully
qualified class name) so the constructor new CommandResult.Success<>(null)
compiles.
🧹 Nitpick comments (2)
services/identity-service/src/test/java/com/tickon/identity/user/application/commandhandlers/ChangePasswordCommandHandlerTest.java (2)
79-97: Test reveals implementation ordering: hash computed before user lookup.The mock setup (lines 87-88) indicates the handler hashes the password before checking user existence. This works but performs unnecessary computation when user doesn't exist. If this is intentional (e.g., constant-time behavior for security), consider adding a comment in the production code. Otherwise, consider reordering in the handler.
The test itself correctly validates the exception path and ensures
save()is never called.
122-136: Consider adding boundary edge cases for password validation.Current test covers a weak password. Additional scenarios to strengthen coverage:
- Null password input
- Empty string password
- Password at exact minimum length boundary
These would ensure the validation policy handles all edge inputs gracefully.
`@Test` void shouldThrowException_WhenPasswordIsNull() { UserId userId = new UserId(UUID.randomUUID()); ChangePasswordCommand command = new ChangePasswordCommand(userId, null); assertThatThrownBy(() -> handler.handle(command)) .isInstanceOf(InvalidPasswordException.class); verify(passwordHasher, never()).hash(any()); verify(userRepository, never()).save(any()); }
Previously only checked for QueryResult.NotFound before casting to Success, which would throw ClassCastException if QueryResult.Error occurred. Now properly handles all three QueryResult variants: - Success: proceeds with password reset flow - NotFound: returns success (silent failure per security best practice) - Error: logs error and returns success (silent failure) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The read-then-write pattern (findById → save) without a transaction boundary could lead to lost updates if concurrent modifications occur. Added @transactional to handle() method to ensure atomicity of: - findById(userId) - user.changePasswordHash() - userRepository.save(user) This prevents race conditions and ensures consistent state. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Previously commandBus.execute() result was ignored, potentially hiding failures and allowing token to be marked as used even if password change failed. Now explicitly captures CommandResult and calls orElseThrow() to ensure: 1. Password change completes successfully 2. Only then mark token as used 3. Only then persist token state This ensures proper sequencing and prevents token invalidation when password change fails, maintaining transactional consistency. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replaced if-else chain with switch expression for cleaner, more idiomatic code: - Exhaustive pattern matching (compiler-enforced) - Record patterns for direct destructuring - No intermediate variables needed - More concise and readable Before: if/else with instanceof checks After: switch with QueryResult.Success(UserAuthDataDTO userDTO) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…. deleted unused files
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests