password reset via email #9
Open
lucie-engineer wants to merge 22 commits into
Open
Conversation
added 18 commits
April 22, 2026 01:47
…, RegisterRequest, AuthService, Wallet BigDecimal
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements a complete password reset feature with JWT-based authentication. It introduces a token-based password recovery flow, enhances JWT utilities with configuration injection and validation methods, integrates JWT filtering into the Spring Security chain, and applies financial precision to wallet balances. ChangesPassword Reset & JWT Authentication
Sequence Diagram(s)sequenceDiagram
participant User
participant PasswordResetController
participant PasswordResetService
participant PasswordResetTokenRepository
participant JavaMailSender
User->>PasswordResetController: POST /forgot-password
PasswordResetController->>PasswordResetService: sendResetLink(email)
PasswordResetService->>PasswordResetTokenRepository: deleteByEmail
PasswordResetService->>PasswordResetTokenRepository: save(token)
PasswordResetService->>JavaMailSender: send reset link
User->>PasswordResetController: POST /reset-password
PasswordResetController->>PasswordResetService: resetPassword(token, password)
PasswordResetService->>PasswordResetTokenRepository: findByToken
PasswordResetService->>PasswordResetTokenRepository: save(used=true)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/saccoplus/entity/IndividualUser.java (1)
14-36:⚠️ Potential issue | 🔴 CriticalAdd
IndividualUserentity.The repository method
findByEmail(String email)on line 12 ofIndividualUserRepository.javarequires a correspondingIndividualUserentity, which is currently missing. Spring Data JPA will fail at runtime with a property reference exception when this method is invoked.Add the field:
`@Column`(unique = true) private String email;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/saccoplus/entity/IndividualUser.java` around lines 14 - 36, The IndividualUser entity is missing an email property required by IndividualUserRepository.findByEmail(String); add a new field to the IndividualUser class: annotate with `@Column`(unique = true) and declare private String email (place it alongside the other identity fields such as phone and nationalId) so JPA and Spring Data can map findByEmail correctly. Ensure imports/annotations already present (javax.persistence.Column) are used and run compilation to verify no other mappings need adjustment.
🧹 Nitpick comments (7)
src/main/java/com/saccoplus/dto/request/RegisterRequest.java (1)
30-30: 💤 Low valueThis file change appears unrelated to the PR's password reset objective.
The PR is focused on implementing password reset via email with JWT-based authentication, but this file (
RegisterRequest.java) is a DTO for user registration. The only change here is a formatting adjustment to the closing brace, which has no functional impact.Consider whether this incidental formatting fix should be included in this PR or committed separately to keep the PR focused on password reset functionality.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/saccoplus/dto/request/RegisterRequest.java` at line 30, The change to RegisterRequest (the DTO class RegisterRequest) only alters formatting of the closing brace and is unrelated to the password-reset work; revert the trivial formatting edit or move it into its own commit/PR so the current password-reset/JWT email changes remain focused—restore the original closing brace formatting in RegisterRequest or create a separate commit containing just that formatting change.src/main/java/com/saccoplus/entity/PasswordResetToken.java (1)
32-32: ⚡ Quick winConsider adding
@Builder.Defaultto theusedfield.The
usedfield defaults tofalsevia Java's primitive boolean default, but when using Lombok's@Builder, it's clearer to explicitly annotate it with@Builder.Defaultto ensure the default value is always set during builder construction.📝 Suggested improvement
+import lombok.Builder.Default; + `@Data` `@Builder` `@AllArgsConstructor` `@NoArgsConstructor` `@Entity` `@Table`(name = "password_reset_tokens") public class PasswordResetToken { `@Id` `@GeneratedValue`(strategy = GenerationType.IDENTITY) private Long id; `@Column`(nullable = false, unique = true) private String token; `@Column`(nullable = false) private String email; `@Column`(nullable = false) private LocalDateTime expiryDate; + `@Builder.Default` private boolean used; + private boolean used = false;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/saccoplus/entity/PasswordResetToken.java` at line 32, The PasswordResetToken class's boolean field used should be annotated with Lombok's `@Builder.Default` and explicitly initialized to false so the builder sets the intended default; update the declaration of the used field in PasswordResetToken to include `@Builder.Default` and a = false initializer to ensure builder-created instances default to unused.src/main/java/com/saccoplus/security/JwtUtils.java (1)
32-55: ⚡ Quick winConsider validating the
typeclaim when extracting identity.
generateAccessTokenandgenerateRefreshTokendistinguish themselves only with a"type"claim (access/refresh), but nothing inJwtUtilsorJwtFilterchecks it. A refresh token presented in anAuthorization: Bearer ...header will currently authenticate the user just like an access token (the role claim will simply be missing, but if you later relax the role check or add roles to refresh tokens this becomes a real bypass).Recommend adding a helper like
extractTokenType(token)and rejecting non-accesstokens inJwtFilter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/saccoplus/security/JwtUtils.java` around lines 32 - 55, Add explicit token-type validation: implement a helper method (e.g., extractTokenType(String token)) in JwtUtils that parses the "type" claim from a token and returns it, then update JwtFilter’s authentication flow to call this helper and reject tokens whose type is not "access" (fail authentication/return 401). Ensure generateAccessToken and generateRefreshToken remain unchanged but that JwtFilter uses extractTokenType(token) before trusting subject/roles, and log/handle missing or unexpected "type" claims as invalid tokens.src/main/java/com/saccoplus/config/SecurityConfig.java (1)
33-36: ⚖️ Poor tradeoff
@Lazyworks but masks the real circular dependency.
JwtFilteris a@Componentinjected intoSecurityConfig, which builds the filter chain that containsJwtFilter— hence the cycle. A cleaner fix is to drop@ComponentfromJwtFilter, declare it as a@BeaninsideSecurityConfig(or a dedicatedJwtConfig), and injectJwtUtilsdirectly. Not blocking for this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/saccoplus/config/SecurityConfig.java` around lines 33 - 36, The constructor uses `@Lazy` on JwtFilter to hide a circular dependency between SecurityConfig and the `@Component` JwtFilter; instead remove `@Component` from the JwtFilter class and expose it as a `@Bean` inside SecurityConfig (or a new JwtConfig) that constructs JwtFilter with its JwtUtils dependency injected directly; update SecurityConfig to inject the JwtFilter bean normally (remove `@Lazy`) or inject JwtUtils into the bean factory method so JwtFilter is created from that bean method, eliminating the circular dependency.src/main/java/com/saccoplus/service/impl/PasswordResetServiceImpl.java (2)
26-94: ⚡ Quick winAdd
@Transactionalto both methods.Both flows do multi-step writes that should be atomic:
sendResetLink:deleteByEmail+save(a failure between leaves the user without a usable token).resetPassword:userRepository.save(user)+tokenRepository.save(resetToken)(a crash between leaves the password changed but the token not marked used, so it can be reused).Annotate both methods (or the class) with
@Transactional. Also consider catchingMailExceptionfrommailSender.sendand rolling back the persisted token if the email fails to send.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/saccoplus/service/impl/PasswordResetServiceImpl.java` around lines 26 - 94, Annotate the transactional flows so multi-step writes are atomic: add `@Transactional` to sendResetLink and resetPassword (or the class) to cover tokenRepository.deleteByEmail + tokenRepository.save in sendResetLink and userRepository.save + tokenRepository.save in resetPassword; additionally wrap mailSender.send in sendResetLink with a try/catch for MailException and on failure roll back the persisted token (delete the saved PasswordResetToken or rethrow to trigger rollback) so you don't leave an unused token when email delivery fails.
30-86: ⚡ Quick winReplace bare
RuntimeExceptionwith typed exceptions mapped to proper HTTP statuses.
throw new RuntimeException("...")currently bubbles up to Spring's default handler and returns HTTP 500 for what are actually 400/404 conditions ("Invalid or expired reset link", "Reset link has already been used", "User not found"). UseResponseStatusExceptionor a custom exception hierarchy with a@ControllerAdviceso clients get appropriate 4xx responses and the API surface is consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/saccoplus/service/impl/PasswordResetServiceImpl.java` around lines 30 - 86, The code in PasswordResetServiceImpl is throwing bare RuntimeException in methods that should return 4xx errors; replace those throws in send/reset flows (references: the code that calls userRepository.findByEmail(...), tokenRepository.findByToken(...), expiry check, isUsed check, and userRepository.findByEmail(resetToken.getEmail())) with typed exceptions (e.g. throw new ResponseStatusException(HttpStatus.NOT_FOUND, "No account found with this email") or HttpStatus.BAD_REQUEST for invalid/expired/used token) or create custom exceptions mapped via `@ControllerAdvice`; ensure each message maps to the correct HTTP status (No account/User not found -> 404, Invalid/expired/used token -> 400) and remove RuntimeException usage so clients receive proper 4xx responses.src/main/java/com/saccoplus/security/JwtFilter.java (1)
68-71: 💤 Low valueNarrow the
catch (Exception ...)to JWT-specific exceptions.Catching
Exceptionhere will silently swallow unrelated runtime issues (e.g., NPEs from future refactors) as "Invalid JWT token". Prefer the specific JWT exception hierarchy so non-JWT bugs surface normally.♻️ Suggested change
- } catch (Exception e) { - - logger.warn("Invalid JWT token: " + e.getMessage()); - } + } catch (io.jsonwebtoken.JwtException | IllegalArgumentException e) { + logger.warn("Invalid JWT token: " + e.getMessage()); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/saccoplus/security/JwtFilter.java` around lines 68 - 71, The catch block in JwtFilter that currently uses "catch (Exception e)" should be narrowed to only JWT-related exceptions (e.g., io.jsonwebtoken.JwtException and its subtypes like ExpiredJwtException, MalformedJwtException, SignatureException, etc.) in the method where the token is parsed/validated (JwtFilter -> doFilterInternal or the token-processing helper). Replace the broad catch with catches for the JWT library's exception types and handle them the same way (log a warning and continue), while allowing other runtime exceptions (NPEs, IllegalStateException, etc.) to propagate so they are not misreported as invalid tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.idea/misc.xml:
- Line 11: The repo now ignores .idea but this PR modifies .idea/misc.xml,
creating an inconsistency; resolve by choosing one of two actions: (1) Stop
tracking IDE files: remove .idea from the index (git rm --cached on the .idea
directory) and remove .idea/misc.xml from this commit so the new .gitignore
takes effect, or (2) Keep tracking IDE configuration: remove the .idea entry
from .gitignore and keep the .idea/misc.xml change in the commit so the
JDK/version change remains tracked; apply the chosen action and update the
commit accordingly.
- Line 11: The IDE config uses ProjectRootManager with languageLevel="JDK_21"
which conflicts with the Maven pom.xml's <java.version>17; decide whether the
project should target Java 17 or be upgraded to 21 and make them consistent:
either change ProjectRootManager's languageLevel to "JDK_17" to match
<java.version>17, or update pom.xml's <java.version> to 21 if upgrading, then
run a full project rebuild and confirm with the team that ProjectRootManager and
<java.version> are aligned.
In `@src/main/java/com/saccoplus/config/SecurityConfig.java`:
- Around line 30-31: The current SecurityConfig stores app.cors.allowed-origins
into a single String field allowedOrigins and then uses List.of(allowedOrigins),
which produces a single-element entry when multiple origins are configured;
change the binding to a List<String> (e.g.,
`@Value`("${app.cors.allowed-origins}") private List<String> allowedOrigins) so
Spring auto-splits comma-separated values, or alternatively keep the String and
split it into a List<String> (e.g.,
Arrays.stream(allowedOrigins.split(",")).map(String::trim).filter(s->!s.isEmpty()).collect(Collectors.toList()))
wherever List.of(allowedOrigins) is used (look for usages in the CORS
configuration method in SecurityConfig) to ensure each origin is a separate list
element.
In `@src/main/java/com/saccoplus/controller/PasswordResetController.java`:
- Around line 19-25: The forgot-password endpoint currently leaks account
existence because passwordResetService.sendResetLink(request.getEmail()) throws
for unknown emails; change PasswordResetController.forgotPassword to always
return the same generic 200 response (e.g., "If an account exists an email has
been sent") regardless of service outcome, and update
PasswordResetServiceImpl.sendResetLink to swallow or handle missing-user cases
asynchronously (queue email sending or log but do not throw) so no exception
bubbles up; ensure no different HTTP status or error message is emitted when the
email is unknown by catching service exceptions in forgotPassword or by making
sendResetLink return a no-op for unknown emails.
In `@src/main/java/com/saccoplus/entity/Wallet.java`:
- Around line 22-23: The Wallet entity uses BigDecimal for balance but the
service code uses primitive arithmetic; update GroupServiceImpl to set
.balance(BigDecimal.ZERO) instead of .balance(0.0), and in WalletServiceImpl
replace comparisons and arithmetic: use
wallet.getBalance().compareTo(request.getAmount()) (compareTo(...) < 0) for the
less-than check, replace any wallet.getBalance() - request.getAmount() with
wallet.getBalance().subtract(request.getAmount()), and replace any
wallet.getBalance() + request.getAmount() with
wallet.getBalance().add(request.getAmount()); ensure all uses reference the
BigDecimal methods rather than +, -, <, or > so precision is preserved.
- Around line 22-23: Wallet.balance was changed to BigDecimal but related code
still uses doubles and lacks a DB migration; update all assignments to use
BigDecimal (e.g., convert double updatedBalance in WalletServiceImpl and the 0.0
literal in GroupServiceImpl via BigDecimal.valueOf(...) or new
BigDecimal(String)), change TransactionResponse.balance to BigDecimal (and
adjust its serialization/deserialization if needed), and add an explicit
production migration script (Flyway/Liquibase) to ALTER the balance column to
DECIMAL(19,4) with safe data conversion/backfill so ddl-auto=validate succeeds
in prod.
In `@src/main/java/com/saccoplus/repository/PasswordResetTokenRepository.java`:
- Line 12: Add transaction boundaries to the password-reset flow by annotating
the service methods that perform multiple DB operations—sendResetLink and
resetPassword—with `@Transactional` (import
org.springframework.transaction.annotation.Transactional) so the calls
(including repository methods like
PasswordResetTokenRepository.deleteByEmail(...)) run atomically; apply the
annotation at the method level in the service class that declares sendResetLink
and resetPassword (or at the class level if all methods should be transactional)
and consider specifying rollbackFor = Exception.class if you need to ensure
rollback on checked exceptions.
In `@src/main/java/com/saccoplus/security/JwtFilter.java`:
- Around line 52-66: The filter currently only sets authentication when both
username and role are non-blank, causing valid tokens without a role (or wrong
token type) to be silently dropped; update JwtFilter (the block using username,
role, SecurityContextHolder) to explicitly handle the case where the token is
otherwise valid but role is missing or the token type is not "access": call
JwtUtils to validate the token type first, and if the token is valid but role is
null/blank, log a clear warning and send a 401 response (or throw an
AuthenticationException) instead of silently continuing; ensure this logic runs
before the existing UsernamePasswordAuthenticationToken creation so tokens
without required claims are rejected with a clear error.
In `@src/main/java/com/saccoplus/security/JwtUtils.java`:
- Around line 71-78: isTokenExpired currently calls extractExpiration(token)
which uses parseSignedClaims and will throw ExpiredJwtException for expired
tokens; change isTokenExpired to catch JwtException (or ExpiredJwtException)
around the call to extractExpiration and return true on ExpiredJwtException (or
return true when parsing fails due to expiry) and false for other non-expiry
parse exceptions as appropriate. Similarly update isTokenValid to wrap
extractUsername(token) and isTokenExpired(token) calls in a try/catch that
mirrors validateToken()’s pattern: catch JwtException and return false when
parsing fails (including expired tokens) so isTokenValid never throws but
returns a boolean. Ensure you reference and reuse the same exception handling
approach used in validateToken() and only update the methods isTokenExpired and
isTokenValid (leaving extractExpiration/extractUsername behavior unchanged).
In `@src/main/java/com/saccoplus/service/impl/PasswordResetServiceImpl.java`:
- Around line 37-47: The code currently persists the raw UUID token (in
PasswordResetServiceImpl when creating PasswordResetToken), which is insecure;
instead compute a secure hash (e.g. SHA-256) of the generated raw token and
store that hash in PasswordResetToken.token while sending the raw token to the
user via email; then update resetPassword (the method that calls
tokenRepository.findByToken) to hash the incoming token using the same SHA-256
function before lookup. Update any comments/field documentation to reflect that
PasswordResetToken.token stores a hash, and ensure you use a well-known hashing
utility (e.g. DigestUtils.sha256Hex or MessageDigest) consistently for both
store and lookup in PasswordResetServiceImpl and the tokenRepository
interactions.
- Around line 27-34: Replace the current behavior in sendResetLink so it does
not throw when userRepository.findByEmail(email) returns empty; instead treat
missing users as a no-op for outward behavior: look up the user with
userRepository.findByEmail(email) and if absent, skip any user-specific
operations (but still call tokenRepository.deleteByEmail(email) and optionally
log an internal warning), and if present continue creating/sending the reset
token as before; ensure the method does not propagate a RuntimeException so the
controller can always return the same generic success response for both existing
and non-existing emails.
- Around line 50-63: PasswordResetServiceImpl currently constructs a hardcoded
reset URL ("http://localhost:3000/auth/reset-password?token=...") when building
the SimpleMailMessage; move the frontend base URL into configuration (e.g., add
app.frontend.reset-password-url in application.yml), inject it into
PasswordResetServiceImpl with `@Value` (or a `@ConfigurationProperties` bean) and
compose the full link using that injected value plus the token when setting
message.setText(...); ensure mailSender.send(message) uses the composed URL so
environments can supply HTTPS/production host without code changes.
---
Outside diff comments:
In `@src/main/java/com/saccoplus/entity/IndividualUser.java`:
- Around line 14-36: The IndividualUser entity is missing an email property
required by IndividualUserRepository.findByEmail(String); add a new field to the
IndividualUser class: annotate with `@Column`(unique = true) and declare private
String email (place it alongside the other identity fields such as phone and
nationalId) so JPA and Spring Data can map findByEmail correctly. Ensure
imports/annotations already present (javax.persistence.Column) are used and run
compilation to verify no other mappings need adjustment.
---
Nitpick comments:
In `@src/main/java/com/saccoplus/config/SecurityConfig.java`:
- Around line 33-36: The constructor uses `@Lazy` on JwtFilter to hide a circular
dependency between SecurityConfig and the `@Component` JwtFilter; instead remove
`@Component` from the JwtFilter class and expose it as a `@Bean` inside
SecurityConfig (or a new JwtConfig) that constructs JwtFilter with its JwtUtils
dependency injected directly; update SecurityConfig to inject the JwtFilter bean
normally (remove `@Lazy`) or inject JwtUtils into the bean factory method so
JwtFilter is created from that bean method, eliminating the circular dependency.
In `@src/main/java/com/saccoplus/dto/request/RegisterRequest.java`:
- Line 30: The change to RegisterRequest (the DTO class RegisterRequest) only
alters formatting of the closing brace and is unrelated to the password-reset
work; revert the trivial formatting edit or move it into its own commit/PR so
the current password-reset/JWT email changes remain focused—restore the original
closing brace formatting in RegisterRequest or create a separate commit
containing just that formatting change.
In `@src/main/java/com/saccoplus/entity/PasswordResetToken.java`:
- Line 32: The PasswordResetToken class's boolean field used should be annotated
with Lombok's `@Builder.Default` and explicitly initialized to false so the
builder sets the intended default; update the declaration of the used field in
PasswordResetToken to include `@Builder.Default` and a = false initializer to
ensure builder-created instances default to unused.
In `@src/main/java/com/saccoplus/security/JwtFilter.java`:
- Around line 68-71: The catch block in JwtFilter that currently uses "catch
(Exception e)" should be narrowed to only JWT-related exceptions (e.g.,
io.jsonwebtoken.JwtException and its subtypes like ExpiredJwtException,
MalformedJwtException, SignatureException, etc.) in the method where the token
is parsed/validated (JwtFilter -> doFilterInternal or the token-processing
helper). Replace the broad catch with catches for the JWT library's exception
types and handle them the same way (log a warning and continue), while allowing
other runtime exceptions (NPEs, IllegalStateException, etc.) to propagate so
they are not misreported as invalid tokens.
In `@src/main/java/com/saccoplus/security/JwtUtils.java`:
- Around line 32-55: Add explicit token-type validation: implement a helper
method (e.g., extractTokenType(String token)) in JwtUtils that parses the "type"
claim from a token and returns it, then update JwtFilter’s authentication flow
to call this helper and reject tokens whose type is not "access" (fail
authentication/return 401). Ensure generateAccessToken and generateRefreshToken
remain unchanged but that JwtFilter uses extractTokenType(token) before trusting
subject/roles, and log/handle missing or unexpected "type" claims as invalid
tokens.
In `@src/main/java/com/saccoplus/service/impl/PasswordResetServiceImpl.java`:
- Around line 26-94: Annotate the transactional flows so multi-step writes are
atomic: add `@Transactional` to sendResetLink and resetPassword (or the class) to
cover tokenRepository.deleteByEmail + tokenRepository.save in sendResetLink and
userRepository.save + tokenRepository.save in resetPassword; additionally wrap
mailSender.send in sendResetLink with a try/catch for MailException and on
failure roll back the persisted token (delete the saved PasswordResetToken or
rethrow to trigger rollback) so you don't leave an unused token when email
delivery fails.
- Around line 30-86: The code in PasswordResetServiceImpl is throwing bare
RuntimeException in methods that should return 4xx errors; replace those throws
in send/reset flows (references: the code that calls
userRepository.findByEmail(...), tokenRepository.findByToken(...), expiry check,
isUsed check, and userRepository.findByEmail(resetToken.getEmail())) with typed
exceptions (e.g. throw new ResponseStatusException(HttpStatus.NOT_FOUND, "No
account found with this email") or HttpStatus.BAD_REQUEST for
invalid/expired/used token) or create custom exceptions mapped via
`@ControllerAdvice`; ensure each message maps to the correct HTTP status (No
account/User not found -> 404, Invalid/expired/used token -> 400) and remove
RuntimeException usage so clients receive proper 4xx responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e50dafca-e8b5-47ea-9a85-532b2c81d17e
⛔ Files ignored due to path filters (29)
target/classes/com/saccoplus/SaccoPlusApplication.classis excluded by!**/*.classtarget/classes/com/saccoplus/controller/AuthController.classis excluded by!**/*.classtarget/classes/com/saccoplus/controller/WalletController.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/request/DepositRequest.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/request/LoginRequest.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/request/RefreshTokenRequest.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/request/RegisterRequest.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/response/AuthResponse$AuthResponseBuilder.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/response/AuthResponse.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/response/TransactionResponse.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/response/UserProfileResponse$UserProfileResponseBuilder.classis excluded by!**/*.classtarget/classes/com/saccoplus/dto/response/UserProfileResponse.classis excluded by!**/*.classtarget/classes/com/saccoplus/entity/IndividualUser$IndividualUserBuilder.classis excluded by!**/*.classtarget/classes/com/saccoplus/entity/IndividualUser.classis excluded by!**/*.classtarget/classes/com/saccoplus/entity/Role.classis excluded by!**/*.classtarget/classes/com/saccoplus/entity/Transaction.classis excluded by!**/*.classtarget/classes/com/saccoplus/entity/User$UserBuilder.classis excluded by!**/*.classtarget/classes/com/saccoplus/entity/User.classis excluded by!**/*.classtarget/classes/com/saccoplus/entity/Wallet$WalletBuilder.classis excluded by!**/*.classtarget/classes/com/saccoplus/entity/Wallet.classis excluded by!**/*.classtarget/classes/com/saccoplus/exception/BusinessException.classis excluded by!**/*.classtarget/classes/com/saccoplus/repository/IndividualUserRepository.classis excluded by!**/*.classtarget/classes/com/saccoplus/repository/TransactionRepository.classis excluded by!**/*.classtarget/classes/com/saccoplus/repository/UserRepository.classis excluded by!**/*.classtarget/classes/com/saccoplus/security/JwtUtils.classis excluded by!**/*.classtarget/classes/com/saccoplus/service/AuthService.classis excluded by!**/*.classtarget/classes/com/saccoplus/service/WalletService.classis excluded by!**/*.classtarget/classes/com/saccoplus/service/impl/AuthServiceImpl.classis excluded by!**/*.classtarget/classes/com/saccoplus/service/impl/WalletServiceImpl.classis excluded by!**/*.class
📒 Files selected for processing (24)
.gitignore.idea/misc.xmlsrc/main/java/com/saccoplus/SaccoPlusApplication.javasrc/main/java/com/saccoplus/config/SecurityConfig.javasrc/main/java/com/saccoplus/controller/PasswordResetController.javasrc/main/java/com/saccoplus/dto/request/ForgotPasswordRequest.javasrc/main/java/com/saccoplus/dto/request/RegisterRequest.javasrc/main/java/com/saccoplus/dto/request/ResetPasswordRequest.javasrc/main/java/com/saccoplus/entity/IndividualUser.javasrc/main/java/com/saccoplus/entity/PasswordResetToken.javasrc/main/java/com/saccoplus/entity/Wallet.javasrc/main/java/com/saccoplus/repository/IndividualUserRepository.javasrc/main/java/com/saccoplus/repository/PasswordResetTokenRepository.javasrc/main/java/com/saccoplus/security/JwtFilter.javasrc/main/java/com/saccoplus/security/JwtUtils.javasrc/main/java/com/saccoplus/service/AuthService.javasrc/main/java/com/saccoplus/service/PasswordResetService.javasrc/main/java/com/saccoplus/service/impl/PasswordResetServiceImpl.javatarget/classes/application.ymltarget/classes/templates/email/.gitkeeptarget/maven-status/maven-compiler-plugin/compile/default-compile/createdFiles.lsttarget/maven-status/maven-compiler-plugin/compile/default-compile/inputFiles.lsttarget/maven-status/maven-compiler-plugin/testCompile/default-testCompile/createdFiles.lsttarget/maven-status/maven-compiler-plugin/testCompile/default-testCompile/inputFiles.lst
💤 Files with no reviewable changes (4)
- target/maven-status/maven-compiler-plugin/compile/default-compile/inputFiles.lst
- target/maven-status/maven-compiler-plugin/testCompile/default-testCompile/inputFiles.lst
- target/maven-status/maven-compiler-plugin/compile/default-compile/createdFiles.lst
- target/classes/application.yml
| </option> | ||
| </component> | ||
| <component name="ProjectRootManager" version="2" languageLevel="JDK_22" default="true" project-jdk-name="openjdk-22" project-jdk-type="JavaSDK" /> | ||
| <component name="ProjectRootManager" version="2" languageLevel="JDK_21" default="true" project-jdk-name="21" project-jdk-type="JavaSDK" /> |
There was a problem hiding this comment.
Inconsistency: .idea/ is now ignored but this file is being modified.
This PR adds .idea/ to .gitignore (line 3 of .gitignore) while simultaneously committing changes to .idea/misc.xml. This creates a conflict:
- Future changes to
.idea/misc.xmlwon't be tracked by Git - Team members will have diverging IDE configurations
- This commit's JDK version change will be lost for other developers
Resolution options:
-
Remove IDE files from tracking (recommended for team flexibility):
git rm -r --cached .idea/
Then remove this file from the current commit.
-
Keep tracking IDE configuration (recommended for consistency):
Remove.idea/from.gitignoreand keep this change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.idea/misc.xml at line 11, The repo now ignores .idea but this PR modifies
.idea/misc.xml, creating an inconsistency; resolve by choosing one of two
actions: (1) Stop tracking IDE files: remove .idea from the index (git rm
--cached on the .idea directory) and remove .idea/misc.xml from this commit so
the new .gitignore takes effect, or (2) Keep tracking IDE configuration: remove
the .idea entry from .gitignore and keep the .idea/misc.xml change in the commit
so the JDK/version change remains tracked; apply the chosen action and update
the commit accordingly.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check Java version configuration across project files
# Check pom.xml for Java version configuration
echo "=== Maven Java version configuration ==="
rg -n '<java\.version>|<maven\.compiler\.(source|target|release)>' pom.xml
# Check if there are any Java 22 specific features in the codebase
echo -e "\n=== Checking for Java 22 specific syntax ==="
rg -n 'record|sealed|permits' --type java
# Check application properties for Java version hints
echo -e "\n=== Application configuration ==="
fd -e yml -e yaml -e properties | xargs rg -l 'java\.version|jdk'Repository: SheCanCODE-Capstone-Projects/saccoPlus-BN
Length of output: 252
Align IDE Java version configuration with Maven build configuration.
The .idea/misc.xml file sets languageLevel="JDK_21", but pom.xml specifies <java.version>17</java.version>. Update one to match the other to prevent IDE/build inconsistencies during development. Verify with your team whether the project targets Java 17 or should be upgraded to 21.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.idea/misc.xml at line 11, The IDE config uses ProjectRootManager with
languageLevel="JDK_21" which conflicts with the Maven pom.xml's
<java.version>17; decide whether the project should target Java 17 or be
upgraded to 21 and make them consistent: either change ProjectRootManager's
languageLevel to "JDK_17" to match <java.version>17, or update pom.xml's
<java.version> to 21 if upgrading, then run a full project rebuild and confirm
with the team that ProjectRootManager and <java.version> are aligned.
Comment on lines
+19
to
+25
| @PostMapping("/forgot-password") | ||
| public ResponseEntity<String> forgotPassword( | ||
| @Valid @RequestBody ForgotPasswordRequest request) { | ||
|
|
||
| passwordResetService.sendResetLink(request.getEmail()); | ||
| return ResponseEntity.ok("Reset link sent to " + request.getEmail()); | ||
| } |
There was a problem hiding this comment.
Account enumeration: response differs based on whether email is registered.
passwordResetService.sendResetLink throws RuntimeException("No account found with this email") when the email is unknown, which (with the default error handling) returns a 500/4xx with that message — while a registered email returns 200 with "Reset link sent to …". Attackers can trivially enumerate registered accounts.
Always return the same generic 200 response regardless of whether the email exists; perform the lookup-and-send asynchronously (or just swallow the missing-user case) in the service. See the related comment on PasswordResetServiceImpl.sendResetLink.
🛡️ Suggested response change
- passwordResetService.sendResetLink(request.getEmail());
- return ResponseEntity.ok("Reset link sent to " + request.getEmail());
+ passwordResetService.sendResetLink(request.getEmail());
+ return ResponseEntity.ok("If an account exists for that email, a reset link has been sent.");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/saccoplus/controller/PasswordResetController.java` around
lines 19 - 25, The forgot-password endpoint currently leaks account existence
because passwordResetService.sendResetLink(request.getEmail()) throws for
unknown emails; change PasswordResetController.forgotPassword to always return
the same generic 200 response (e.g., "If an account exists an email has been
sent") regardless of service outcome, and update
PasswordResetServiceImpl.sendResetLink to swallow or handle missing-user cases
asynchronously (queue email sending or log but do not throw) so no exception
bubbles up; ensure no different HTTP status or error message is emitted when the
email is unknown by catching service exceptions in forgotPassword or by making
sendResetLink return a no-op for unknown emails.
Comment on lines
+22
to
+23
| @Column(precision = 19, scale = 4) | ||
| private BigDecimal balance = BigDecimal.ZERO; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all references to wallet.balance to verify BigDecimal usage
# Search for balance field access patterns
rg -n -C3 '\.getBalance\(\)|\.setBalance\(|\.balance' --type=java -g '!**/Wallet.java'
# Look for potential primitive arithmetic on balance
rg -n -P '(balance\s*[+\-*/]|\s[+\-*/]\s*balance)' --type=javaRepository: SheCanCODE-Capstone-Projects/saccoPlus-BN
Length of output: 4829
Fix arithmetic operations on wallet.balance to use BigDecimal methods.
The following code locations are using primitive operators instead of BigDecimal methods:
GroupServiceImpl.java:54-.balance(0.0)should passBigDecimal.ZEROWalletServiceImpl.java:34-.getBalance() < request.getAmount()should use.compareTo()WalletServiceImpl.java:38, 47- subtraction operations must use.subtract()WalletServiceImpl.java:69- addition operation must use.add()
Replace all primitive arithmetic (+, -, <, >) with corresponding BigDecimal methods (.add(), .subtract(), .compareTo()).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/saccoplus/entity/Wallet.java` around lines 22 - 23, The
Wallet entity uses BigDecimal for balance but the service code uses primitive
arithmetic; update GroupServiceImpl to set .balance(BigDecimal.ZERO) instead of
.balance(0.0), and in WalletServiceImpl replace comparisons and arithmetic: use
wallet.getBalance().compareTo(request.getAmount()) (compareTo(...) < 0) for the
less-than check, replace any wallet.getBalance() - request.getAmount() with
wallet.getBalance().subtract(request.getAmount()), and replace any
wallet.getBalance() + request.getAmount() with
wallet.getBalance().add(request.getAmount()); ensure all uses reference the
BigDecimal methods rather than +, -, <, or > so precision is preserved.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for database migration files that handle the balance type change
# Search for migration files (Flyway or Liquibase patterns)
fd -e sql -e xml | head -20
# Search for references to balance column alterations
rg -n -i 'alter.*wallet.*balance|balance.*decimal' --type=sqlRepository: SheCanCODE-Capstone-Projects/saccoPlus-BN
Length of output: 260
🏁 Script executed:
#!/bin/bash
# Check pom.xml for Flyway/Liquibase dependencies
echo "=== Checking pom.xml for migration framework dependencies ==="
rg -n "flyway|liquibase" pom.xml -i
# Search for migration directories
echo -e "\n=== Searching for migration directories ==="
fd -type d -name "*migrat*" -o -name "db" -o -name "schema"
# Search for all files with migration-related patterns
echo -e "\n=== Searching for migration-related files ==="
fd | rg -i "migrat|flyway|liquibase" | head -20
# Look for Java classes related to migrations
echo -e "\n=== Searching for Java migration classes ==="
rg -l "DbMigration|FlywayCallback|LiquibaseCallback" --type=java
# Search for any reference to balance column in the entire codebase
echo -e "\n=== Searching for balance references ==="
rg -n "balance" --type=java | head -20Repository: SheCanCODE-Capstone-Projects/saccoPlus-BN
Length of output: 940
🏁 Script executed:
#!/bin/bash
# Read complete Wallet.java to see full context
echo "=== Wallet.java (complete file) ==="
cat -n src/main/java/com/saccoplus/entity/Wallet.java
# Search for application properties files
echo -e "\n=== Application properties/configuration files ==="
find . -name "application*.properties" -o -name "application*.yml" -o -name "application*.yaml" | head -10
# Check TransactionResponse.java to understand the double/BigDecimal inconsistency
echo -e "\n=== TransactionResponse.java context ==="
cat -n src/main/java/com/saccoplus/dto/response/TransactionResponse.java
# Check WalletServiceImpl to see how balance is handled
echo -e "\n=== WalletServiceImpl.java around balance ==="
sed -n '30,45p' src/main/java/com/saccoplus/service/impl/WalletServiceImpl.java
# Check GroupServiceImpl to see how balance is initialized
echo -e "\n=== GroupServiceImpl.java around balance ==="
sed -n '50,60p' src/main/java/com/saccoplus/service/impl/GroupServiceImpl.java
# Look for git history of the Wallet.java file
echo -e "\n=== Recent changes to Wallet.java ==="
git log --oneline -n 5 -- src/main/java/com/saccoplus/entity/Wallet.java 2>/dev/null || echo "Git not available or file unchanged"Repository: SheCanCODE-Capstone-Projects/saccoPlus-BN
Length of output: 2622
🏁 Script executed:
#!/bin/bash
# Check application.yml for JPA/Hibernate configuration
echo "=== application.yml content ==="
cat -n src/main/resources/application.yml
# Check if there's any database DDL configuration
echo -e "\n=== Searching for DDL-related configuration ==="
rg -n "ddl-auto|schema|database" src/main/resources/ -i
# Verify the exact signature of setBalance method in Wallet
echo -e "\n=== Wallet class methods (checking setBalance) ==="
rg -A5 "setBalance" src/main/java/com/saccoplus/entity/Wallet.java
# Check WalletServiceImpl more completely around the balance assignment
echo -e "\n=== Full WalletServiceImpl method with balance logic ==="
sed -n '25,50p' src/main/java/com/saccoplus/service/impl/WalletServiceImpl.javaRepository: SheCanCODE-Capstone-Projects/saccoPlus-BN
Length of output: 4458
Fix type conversion bugs and establish a proper migration strategy for production.
The change from double to BigDecimal creates critical issues:
-
Type Conversion Bugs:
WalletServiceImpl(line 38) assignsdouble updatedBalanceto theBigDecimalfieldGroupServiceImpl(line 54) assigns0.0(double literal) to theBigDecimalfieldTransactionResponsestill usesdouble balanceinstead ofBigDecimal, breaking the contract
-
Production Deployment Risk:
- Dev profile uses
ddl-auto: update(Hibernate auto-generates schema), so it handles the type change automatically - Prod profile uses
ddl-auto: validate(validates schema only), requiring the database schema to exist beforehand with the correctDECIMAL(19,4)type - Without an explicit migration script or pre-deployed schema, production deployment will fail
- Dev profile uses
-
Required Fixes:
- Update all assignments to use
BigDecimalinstead ofdouble(useBigDecimal.valueOf()ornew BigDecimal()) - Update
TransactionResponse.balancetoBigDecimal - Create an explicit database migration (Flyway or Liquibase) for production environments to safely alter the
balancecolumn type fromDOUBLEtoDECIMAL(19,4)with data conversion logic
- Update all assignments to use
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/saccoplus/entity/Wallet.java` around lines 22 - 23,
Wallet.balance was changed to BigDecimal but related code still uses doubles and
lacks a DB migration; update all assignments to use BigDecimal (e.g., convert
double updatedBalance in WalletServiceImpl and the 0.0 literal in
GroupServiceImpl via BigDecimal.valueOf(...) or new BigDecimal(String)), change
TransactionResponse.balance to BigDecimal (and adjust its
serialization/deserialization if needed), and add an explicit production
migration script (Flyway/Liquibase) to ALTER the balance column to DECIMAL(19,4)
with safe data conversion/backfill so ddl-auto=validate succeeds in prod.
Comment on lines
+52
to
+66
| if (username != null && !username.isBlank() | ||
| && role != null && !role.isBlank() | ||
| && SecurityContextHolder.getContext().getAuthentication() == null) { | ||
|
|
||
| SimpleGrantedAuthority authority = new SimpleGrantedAuthority("ROLE_" + role); | ||
|
|
||
| UsernamePasswordAuthenticationToken authToken = | ||
| new UsernamePasswordAuthenticationToken( | ||
| username, null, List.of(authority)); | ||
|
|
||
| authToken.setDetails( | ||
| new WebAuthenticationDetailsSource().buildDetails(request)); | ||
|
|
||
| SecurityContextHolder.getContext().setAuthentication(authToken); | ||
| } |
There was a problem hiding this comment.
Requests with a valid token but no role claim authenticate as anonymous — confirm this is intended.
Because the authentication is only set when both username and role are non-blank, a successfully parsed access token that lacks the role claim (or whose role is blank) silently falls through unauthenticated. Combined with JwtUtils not validating the type claim, a refresh token will reach this branch, be parsed successfully, then be silently dropped — the client gets a 401/403 from downstream rules with no indication why. If role is expected to always be present on access tokens, consider returning 401 explicitly (or at least logging) when role is missing on an otherwise-valid token.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/saccoplus/security/JwtFilter.java` around lines 52 - 66,
The filter currently only sets authentication when both username and role are
non-blank, causing valid tokens without a role (or wrong token type) to be
silently dropped; update JwtFilter (the block using username, role,
SecurityContextHolder) to explicitly handle the case where the token is
otherwise valid but role is missing or the token type is not "access": call
JwtUtils to validate the token type first, and if the token is valid but role is
null/blank, log a clear warning and send a 401 response (or throw an
AuthenticationException) instead of silently continuing; ensure this logic runs
before the existing UsernamePasswordAuthenticationToken creation so tokens
without required claims are rejected with a clear error.
Comment on lines
+37
to
+47
| String token = UUID.randomUUID().toString(); | ||
|
|
||
| // Save token with 1 hour expiry | ||
| PasswordResetToken resetToken = PasswordResetToken.builder() | ||
| .token(token) | ||
| .email(email) | ||
| .expiryDate(LocalDateTime.now().plusHours(1)) | ||
| .used(false) | ||
| .build(); | ||
|
|
||
| tokenRepository.save(resetToken); |
There was a problem hiding this comment.
Store a hash of the reset token, not the raw value.
The raw UUID is persisted to the DB and also emailed to the user. Anyone with read access to the password_reset_token table (DB backup, log dump, read-only replica, SQL injection elsewhere) can complete a password reset for any user with an active token. Treat the reset token like a password: store only sha256(token) in PasswordResetToken.token, and look up by the same hash in resetPassword.
String rawToken = UUID.randomUUID().toString();
String tokenHash = DigestUtils.sha256Hex(rawToken); // store this
// email the raw token to the userAnd in resetPassword, hash the incoming token before calling findByToken.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/saccoplus/service/impl/PasswordResetServiceImpl.java`
around lines 37 - 47, The code currently persists the raw UUID token (in
PasswordResetServiceImpl when creating PasswordResetToken), which is insecure;
instead compute a secure hash (e.g. SHA-256) of the generated raw token and
store that hash in PasswordResetToken.token while sending the raw token to the
user via email; then update resetPassword (the method that calls
tokenRepository.findByToken) to hash the incoming token using the same SHA-256
function before lookup. Update any comments/field documentation to reflect that
PasswordResetToken.token stores a hash, and ensure you use a well-known hashing
utility (e.g. DigestUtils.sha256Hex or MessageDigest) consistently for both
store and lookup in PasswordResetServiceImpl and the tokenRepository
interactions.
Comment on lines
+50
to
+63
| SimpleMailMessage message = new SimpleMailMessage(); | ||
| message.setTo(email); | ||
| message.setSubject("SACCOPlus — Password Reset Request"); | ||
| message.setText( | ||
| "Hello " + user.getFirstName() + ",\n\n" + | ||
| "You requested to reset your password.\n\n" + | ||
| "Click the link below to reset your password:\n" + | ||
| "http://localhost:3000/auth/reset-password?token=" + token + "\n\n" + | ||
| "This link expires in 1 hour.\n\n" + | ||
| "If you did not request this, please ignore this email.\n\n" + | ||
| "SACCOPlus Rwanda" | ||
| ); | ||
|
|
||
| mailSender.send(message); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Hardcoded frontend reset URL — externalize to configuration.
http://localhost:3000/auth/reset-password?token=... will be wrong in every non-dev environment and forces a code change to deploy. Move the base URL to application.yml (e.g., app.frontend.reset-password-url) and inject via @Value. This also lets you use HTTPS in prod.
♻️ Suggested change
+ `@Value`("${app.frontend.reset-password-url}")
+ private String resetPasswordUrl;
@@
- "http://localhost:3000/auth/reset-password?token=" + token + "\n\n" +
+ resetPasswordUrl + "?token=" + token + "\n\n" +📝 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.
Suggested change
| SimpleMailMessage message = new SimpleMailMessage(); | |
| message.setTo(email); | |
| message.setSubject("SACCOPlus — Password Reset Request"); | |
| message.setText( | |
| "Hello " + user.getFirstName() + ",\n\n" + | |
| "You requested to reset your password.\n\n" + | |
| "Click the link below to reset your password:\n" + | |
| "http://localhost:3000/auth/reset-password?token=" + token + "\n\n" + | |
| "This link expires in 1 hour.\n\n" + | |
| "If you did not request this, please ignore this email.\n\n" + | |
| "SACCOPlus Rwanda" | |
| ); | |
| mailSender.send(message); | |
| SimpleMailMessage message = new SimpleMailMessage(); | |
| message.setTo(email); | |
| message.setSubject("SACCOPlus — Password Reset Request"); | |
| message.setText( | |
| "Hello " + user.getFirstName() + ",\n\n" + | |
| "You requested to reset your password.\n\n" + | |
| "Click the link below to reset your password:\n" + | |
| resetPasswordUrl + "?token=" + token + "\n\n" + | |
| "This link expires in 1 hour.\n\n" + | |
| "If you did not request this, please ignore this email.\n\n" + | |
| "SACCOPlus Rwanda" | |
| ); | |
| mailSender.send(message); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/saccoplus/service/impl/PasswordResetServiceImpl.java`
around lines 50 - 63, PasswordResetServiceImpl currently constructs a hardcoded
reset URL ("http://localhost:3000/auth/reset-password?token=...") when building
the SimpleMailMessage; move the frontend base URL into configuration (e.g., add
app.frontend.reset-password-url in application.yml), inject it into
PasswordResetServiceImpl with `@Value` (or a `@ConfigurationProperties` bean) and
compose the full link using that injected value plus the token when setting
message.setText(...); ensure mailSender.send(message) uses the composed URL so
environments can supply HTTPS/production host without code changes.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit