Skip to content

Role based access#3

Open
lucie-engineer wants to merge 11 commits into
mainfrom
role-Based-access
Open

Role based access#3
lucie-engineer wants to merge 11 commits into
mainfrom
role-Based-access

Conversation

@lucie-engineer

@lucie-engineer lucie-engineer commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

Release Notes

  • New Features
    • User registration with email validation and secure password requirements
    • JWT-based authentication system for secure API access
    • Role-based access control supporting Admin, Loan Officer, and Member roles
    • Secure password encryption for user accounts
    • Cross-Origin Resource Sharing (CORS) support for API integration

@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@lucie-engineer has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 20 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8cc54034-5d84-4f73-8d9f-3ab5984d9525

📥 Commits

Reviewing files that changed from the base of the PR and between b0d88ab and 2df1c39.

📒 Files selected for processing (5)
  • src/main/java/com/saccoplus/dto/request/RegisterRequest.java
  • src/main/java/com/saccoplus/entity/IndividualUser.java
  • src/main/java/com/saccoplus/entity/Wallet.java
  • src/main/java/com/saccoplus/security/JwtUtils.java
  • src/main/java/com/saccoplus/service/AuthService.java
📝 Walkthrough

Walkthrough

This PR introduces a Spring Boot authentication system with JWT-based security. It adds a Spring Boot application entry point, configures HTTP security with JWT filtering and CORS, defines JWT token utilities, creates a registration request DTO, updates domain entities with nationalId and BigDecimal balance fields, and establishes an AuthService interface for auth operations.

Changes

JWT Authentication & Security Configuration

Layer / File(s) Summary
Entity & DTO Shapes
src/main/java/com/saccoplus/entity/IndividualUser.java, src/main/java/com/saccoplus/entity/Wallet.java, src/main/java/com/saccoplus/dto/request/RegisterRequest.java
IndividualUser adds a unique nationalId column; Wallet.balance changes from primitive double to BigDecimal with precision 19, scale 4; RegisterRequest DTO introduces validated fields for registration: fullName, email, password (min 8 chars), phoneNumber, nationalId, with default role = Role.MEMBER.
JWT Token Handling
src/main/java/com/saccoplus/security/JwtUtils.java, src/main/java/com/saccoplus/security/JwtFilter.java
JwtUtils becomes a Spring @Component that extracts JWT username, role, and expiration via injected @Value properties (app.jwt.secret, app.jwt.expiration-ms, app.jwt.refresh-expiry-ms) and provides validation helpers (isTokenValid, isTokenExpired). JwtFilter intercepts requests, reads the Authorization: Bearer <token> header, validates the token via JwtUtils, and populates SecurityContextHolder with a UsernamePasswordAuthenticationToken bearing ROLE_{role} authority.
Security & CORS Configuration
src/main/java/com/saccoplus/config/SecurityConfig.java
Spring Security configuration establishes stateless HTTP security, disables CSRF, inserts JwtFilter before UsernamePasswordAuthenticationFilter, permits unauthenticated access to /api/v1/auth/**, /api/v1/users/register, and /error, requires role-based authorization for /api/v1/admin/**, /api/v1/loans/**, and /api/v1/member/**, provides a BCryptPasswordEncoder bean, and configures CorsConfigurationSource allowing specified origins, methods, and credentials.
Application Bootstrap
src/main/java/com/saccoplus/SaccoPlusApplication.java
Adds Spring Boot @SpringBootApplication entry point with main method invoking SpringApplication.run().
Service Interface
src/main/java/com/saccoplus/service/AuthService.java
Defines AuthService interface with methods for register(RegisterRequest), login(LoginRequest), refreshToken(String), and getCurrentUser(String email), each returning appropriate response DTOs.
Project Configuration
.idea/misc.xml, .gitignore
IDE Java SDK language level downgraded from JDK 22 to JDK 21; standard .gitignore added to exclude build artifacts, IDE metadata, and log files.

Sequence Diagram

sequenceDiagram
    participant Client
    participant JwtFilter
    participant JwtUtils
    participant SecurityConfig
    participant SecurityContext
    participant Backend

    Client->>JwtFilter: HTTP Request + Authorization: Bearer <token>
    JwtFilter->>JwtFilter: Extract token from header
    JwtFilter->>JwtUtils: Validate token (extractUsername, extractRole, isTokenExpired)
    JwtUtils-->>JwtFilter: Return username, role, validity status
    alt Token Valid
        JwtFilter->>SecurityContext: Create UsernamePasswordAuthenticationToken<br/>with ROLE_{role} authority
        JwtFilter->>SecurityContext: Set authentication
        JwtFilter->>Backend: Continue filter chain
        Backend-->>Client: Process authorized request
    else Token Invalid/Expired
        JwtFilter->>JwtFilter: Log warning, skip auth set
        JwtFilter->>Backend: Continue filter chain (unauthenticated)
        Backend->>SecurityConfig: Check authorization rules
        alt Public Endpoint
            Backend-->>Client: Allow access
        else Protected Endpoint
            Backend-->>Client: 403/401 Unauthorized
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

Suggested reviewers

  • Eddy-dashner

Poem

🐰 With tokens secure and passwords hashed,
A filter guards each request that's passed,
Spring Security weaves its protective charm,
While JWT keeps the system warm,
Authentication blooms anew! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Role based access' accurately describes the main change: implementing role-based access control (RBAC) through SecurityConfig, JwtFilter, JwtUtils, and role validation in the authentication flow.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch role-Based-access

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.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (3)
src/main/java/com/saccoplus/service/impl/AuthServiceImpl.java (1)

11-14: Stub implementation - request parameter is unused.

The register method ignores the RegisterRequest parameter and returns a hardcoded string. When implementing the actual registration logic, note that RegisterRequest currently lacks getters/setters to access field values.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/saccoplus/service/impl/AuthServiceImpl.java` around lines
11 - 14, The register method in AuthServiceImpl currently ignores the
RegisterRequest parameter and returns a hardcoded string; update
AuthServiceImpl.register to consume RegisterRequest (validate inputs, extract
username/email/password, call user creation/persistence logic and return an
appropriate result or token) and remove the stubbed return; if RegisterRequest
lacks accessors, add proper getters/setters or make its fields accessible (e.g.,
getEmail(), getPassword(), getUsername()) so AuthServiceImpl can read the
values, perform validation, hashing, and persistence via your User
repository/service.
.idea/misc.xml (1)

1-12: Exclude IDE-specific files from version control.

.idea/ directory contains user-specific IDE settings and should typically be added to .gitignore to avoid polluting the repository with local configuration changes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.idea/misc.xml around lines 1 - 12, The .idea/misc.xml file (containing
components like ExternalStorageConfigurationManager and ProjectRootManager) is
IDE-specific and should be excluded from version control; update the repository
by adding an entry to .gitignore to ignore the .idea/ directory (or at least
.idea/misc.xml), remove the tracked file from git history with git rm --cached
(or git rm -r --cached .idea) and commit the change so the file is no longer
tracked, and then push the updated commit containing the new .gitignore.
target/classes/application.yml (1)

1-108: Remove target/classes/application.yml from version control.

This is build output, so edits here are overwritten on the next build and can drift from the real source config. The canonical file should live under src/main/resources, and target/ should stay ignored.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@target/classes/application.yml` around lines 1 - 108, The committed build
artifact target/classes/application.yml should be removed from version control
and ignored; delete target/classes/application.yml from the repo (git rm
--cached or remove and commit) and add the target/ (or target/classes/) pattern
to .gitignore so it isn’t re-added, ensuring the canonical config lives at
src/main/resources/application.yml and that file is tracked instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/com/saccoplus/config/SecurityConfig.java`:
- Around line 31-61: The security chain in SecurityConfig is missing CORS setup
so preflight requests are blocked; update the HttpSecurity configuration to call
.cors(Customizer.withDefaults()) (e.g., insert .cors(Customizer.withDefaults())
into the http builder before authorizeHttpRequests) and add a
CorsConfigurationSource bean that reads the app.cors.allowed-origins property
from configuration and constructs the CorsConfiguration (allowedOrigins,
allowedMethods, allowedHeaders, allowCredentials) so Spring Security applies
CORS before jwtFilter/authorization; reference SecurityConfig, jwtFilter, and
the CorsConfigurationSource bean when implementing this.

In `@src/main/java/com/saccoplus/dto/request/RegisterRequest.java`:
- Line 8: Change the RegisterRequest.nationalId field from int to String and
update any related getters/setters/constructors or builders in the
RegisterRequest class to use String; also adjust any validation/parsing logic
that assumed numeric input (e.g., length checks) and ensure call sites that call
IndividualUserRepository.existsByNationalId(...) pass the String nationalId
directly (no numeric conversions) so leading zeros and non-numeric characters
are preserved.
- Around line 3-11: The RegisterRequest DTO currently has private fields
(firstName, lastName, phone, nationalId, password) but no accessors, so
deserialization and callers will fail; fix by adding either Lombok annotations
(e.g., `@Data` or `@Getter/`@Setter) to the RegisterRequest class or implement
explicit public getters and setters for each field (getFirstName/setFirstName,
getLastName/setLastName, getPhone/setPhone, getNationalId/setNationalId,
getPassword/setPassword) so Jackson and other code can read/write the
properties.

In `@src/main/java/com/saccoplus/entity/IndividualUser.java`:
- Around line 28-29: The nationalId field in IndividualUser is declared as int
but should be a String to preserve leading zeros and match repository method
existsByNationalId(String); change the field type of nationalId in the
IndividualUser class from int to String, update its `@Column` annotation to
include unique = true and an appropriate length (or nullable as needed), and add
optional validation annotations (e.g., `@Size` or `@Pattern`) to enforce format
constraints; also scan for any usages/constructors/getters/setters referring to
nationalId and update their types/signatures accordingly to String so they align
with the repository method existsByNationalId(String).

In `@src/main/java/com/saccoplus/entity/Wallet.java`:
- Line 17: Replace the primitive double balance in class Wallet with
java.math.BigDecimal to avoid rounding errors: change the field type (balance),
update its getter/setter (getBalance/setBalance) and any constructors or methods
that mutate it (e.g., deposit/withdraw if present) to accept/return BigDecimal,
and annotate the JPA mapping for balance with an explicit precision and scale
(e.g., `@Column`(precision=19, scale=4)) so the database column stores fixed-point
amounts; also update equals/hashCode/toString uses and any code that performs
arithmetic to use BigDecimal methods (add/subtract) instead of double
arithmetic.

In `@src/main/java/com/saccoplus/security/JwtFilter.java`:
- Around line 44-60: JwtFilter currently trusts jwtUtils.extractRole(token) and
builds a SimpleGrantedAuthority("ROLE_" + role) even when role is null/blank;
update JwtFilter to require both a non-null, non-blank role and a non-null
username before creating the SimpleGrantedAuthority and
UsernamePasswordAuthenticationToken and setting the SecurityContextHolder, e.g.,
call jwtUtils.extractRole(token), trim/check isEmpty (or isBlank) and only
proceed to create new SimpleGrantedAuthority("ROLE_" + role) and set
authentication when the role passes validation; if role is missing/blank, do not
set the SecurityContext and handle/log the invalid token case accordingly.

In `@src/main/java/com/saccoplus/security/JwtUtils.java`:
- Around line 18-21: The getSigningKey() method is incorrectly calling
Base64.getEncoder().encode(...) on SECRET_KEY; replace that call with the
correct handling: if SECRET_KEY is a Base64-encoded string, decode it using
Base64.getDecoder().decode(SECRET_KEY) before passing to Keys.hmacShaKeyFor,
otherwise (for a raw secret) pass SECRET_KEY.getBytes(StandardCharsets.UTF_8)
directly; update getSigningKey() to remove Base64.getEncoder().encode and use
the appropriate bytes source for Keys.hmacShaKeyFor.
- Around line 42-44: isTokenValid currently only compares the extracted username
and ignores token expiry; update JwtUtils.isTokenValid to also verify the token
is not expired by calling the existing expiration helper (e.g.,
extractExpiration or isTokenExpired) for the given token and confirming the
expiration is after now before returning true; reference JwtUtils.isTokenValid
and the helper used to extract/validate expiration (extractExpiration /
isTokenExpired) so the method returns true only when username matches AND token
is not expired.
- Line 15: The SECRET_KEY field in JwtUtils is currently an empty hardcoded
string which will cause WeakKeyException at runtime; replace this with a
configuration-injected value (e.g., use `@Value` or constructor injection to read
a jwt.secret property) in the JwtUtils class, validate that the injected secret
is present and meets required length (>= 256 bits for HMAC-SHA) and throw a
clear exception during startup if invalid, and update any places that reference
SECRET_KEY to use the injected value (or its validated byte[] form) instead of
the empty literal.

In `@src/main/java/com/saccoplus/service/AuthService.java`:
- Around line 3-5: The interface AuthService is missing the register method that
AuthServiceImpl overrides; add a matching method declaration to AuthService
(e.g., the register signature used in AuthServiceImpl.register) so the interface
and implementation align; ensure the method name, parameter types and return
type match AuthServiceImpl.register() exactly to resolve the compilation error.

In `@target/classes/application.yml`:
- Around line 4-5: Remove fallback defaults in application.yml so profile and
JWT cannot silently "fail open": stop using ${SPRING_PROFILES_ACTIVE:dev} and
any default for JWT (e.g., ${JWT_SECRET:...}), require callers to set
SPRING_PROFILES_ACTIVE and JWT_SECRET exactly (no colon/default) and ensure
dangerous defaults like spring.jpa.hibernate.ddl-auto: update and debug logging
are not enabled by default in the file; instead set safe production defaults or
omit those keys so environments must explicitly configure ddl-auto and logging
levels. Update references for spring.profiles.active and JWT_SECRET (and any
spring.jpa.hibernate.ddl-auto/logging.level entries) to remove inline defaults
and document that these env vars are mandatory.

---

Nitpick comments:
In @.idea/misc.xml:
- Around line 1-12: The .idea/misc.xml file (containing components like
ExternalStorageConfigurationManager and ProjectRootManager) is IDE-specific and
should be excluded from version control; update the repository by adding an
entry to .gitignore to ignore the .idea/ directory (or at least .idea/misc.xml),
remove the tracked file from git history with git rm --cached (or git rm -r
--cached .idea) and commit the change so the file is no longer tracked, and then
push the updated commit containing the new .gitignore.

In `@src/main/java/com/saccoplus/service/impl/AuthServiceImpl.java`:
- Around line 11-14: The register method in AuthServiceImpl currently ignores
the RegisterRequest parameter and returns a hardcoded string; update
AuthServiceImpl.register to consume RegisterRequest (validate inputs, extract
username/email/password, call user creation/persistence logic and return an
appropriate result or token) and remove the stubbed return; if RegisterRequest
lacks accessors, add proper getters/setters or make its fields accessible (e.g.,
getEmail(), getPassword(), getUsername()) so AuthServiceImpl can read the
values, perform validation, hashing, and persistence via your User
repository/service.

In `@target/classes/application.yml`:
- Around line 1-108: The committed build artifact target/classes/application.yml
should be removed from version control and ignored; delete
target/classes/application.yml from the repo (git rm --cached or remove and
commit) and add the target/ (or target/classes/) pattern to .gitignore so it
isn’t re-added, ensuring the canonical config lives at
src/main/resources/application.yml and that file is tracked instead.
🪄 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: d8af79a7-7e11-47a9-a2d0-746c0e96c851

📥 Commits

Reviewing files that changed from the base of the PR and between 07585cb and 01ad641.

⛔ Files ignored due to path filters (7)
  • target/classes/com/saccoplus/dto/request/RegisterRequest.class is excluded by !**/*.class
  • target/classes/com/saccoplus/entity/IndividualUser.class is excluded by !**/*.class
  • target/classes/com/saccoplus/entity/Role.class is excluded by !**/*.class
  • target/classes/com/saccoplus/entity/Wallet.class is excluded by !**/*.class
  • target/classes/com/saccoplus/repository/IndividualUserRepository.class is excluded by !**/*.class
  • target/classes/com/saccoplus/security/JwtUtils.class is excluded by !**/*.class
  • target/classes/com/saccoplus/service/AuthService.class is excluded by !**/*.class
📒 Files selected for processing (14)
  • .idea/misc.xml
  • src/main/java/com/saccoplus/SaccoPlusApplication.java
  • src/main/java/com/saccoplus/config/SecurityConfig.java
  • src/main/java/com/saccoplus/dto/request/RegisterRequest.java
  • src/main/java/com/saccoplus/entity/IndividualUser.java
  • src/main/java/com/saccoplus/entity/Role.java
  • src/main/java/com/saccoplus/entity/Wallet.java
  • src/main/java/com/saccoplus/repository/IndividualUserRepository.java
  • src/main/java/com/saccoplus/security/JwtFilter.java
  • src/main/java/com/saccoplus/security/JwtUtils.java
  • src/main/java/com/saccoplus/service/AuthService.java
  • src/main/java/com/saccoplus/service/impl/AuthServiceImpl.java
  • target/classes/application.yml
  • target/classes/templates/email/.gitkeep

Comment thread src/main/java/com/saccoplus/config/SecurityConfig.java
Comment thread src/main/java/com/saccoplus/dto/request/RegisterRequest.java Outdated
Comment thread src/main/java/com/saccoplus/dto/request/RegisterRequest.java Outdated
Comment thread src/main/java/com/saccoplus/entity/IndividualUser.java Outdated
Comment thread src/main/java/com/saccoplus/entity/Wallet.java Outdated
Comment thread src/main/java/com/saccoplus/security/JwtUtils.java Outdated
Comment thread src/main/java/com/saccoplus/security/JwtUtils.java Outdated
Comment thread src/main/java/com/saccoplus/security/JwtUtils.java
Comment thread src/main/java/com/saccoplus/service/AuthService.java Outdated
Comment thread target/classes/application.yml Outdated
@Eddy-dashner
Eddy-dashner self-requested a review April 30, 2026 09:17

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/com/saccoplus/security/JwtFilter.java`:
- Around line 44-45: Wrap the calls to jwtUtils.extractUsername(token) and
jwtUtils.extractRole(token) in a try/catch that catches JJWT parsing exceptions
(e.g. io.jsonwebtoken.JwtException) and IllegalArgumentException so
malformed/expired/tampered tokens do not propagate; on catch, log at debug/info
and simply do not set an Authentication in JwtFilter (allow the filter chain to
continue as unauthenticated) instead of throwing a 500. Ensure the try/catch
surrounds both extractUsername and extractRole (which delegate to
parseClaimsJws) and only set SecurityContext authentication when no exception
occurs and token validation succeeds.
🪄 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: 33ea61d2-7287-4498-bfa2-db76df3698db

📥 Commits

Reviewing files that changed from the base of the PR and between 01ad641 and 78f32d3.

📒 Files selected for processing (1)
  • src/main/java/com/saccoplus/security/JwtFilter.java

Comment thread src/main/java/com/saccoplus/security/JwtFilter.java Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/main/java/com/saccoplus/config/SecurityConfig.java (2)

30-31: 💤 Low value

Remove leftover development comment.

The inline comment // ← ADD THIS appears to be a development reminder that should be cleaned up before merging.

♻️ Proposed fix
-    `@Value`("${app.cors.allowed-origins}")  // ← ADD THIS
+    `@Value`("${app.cors.allowed-origins}")
     private String allowedOrigins;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/saccoplus/config/SecurityConfig.java` around lines 30 - 31,
Remove the leftover inline development comment by deleting the trailing "// ←
ADD THIS" on the allowedOrigins field in the SecurityConfig class; ensure the
field declaration remains as a clean annotation and private String
allowedOrigins; so only the `@Value`("${app.cors.allowed-origins}") annotation and
the private String allowedOrigins; line remain.

82-87: ⚡ Quick win

Consider supporting multiple comma-separated origins.

List.of(allowedOrigins) treats the entire string as one origin. If CORS_ORIGINS contains comma-separated values like "http://a.com,http://b.com", this will fail to match any request origin. The current setup works for single-origin deployments, but consider splitting for flexibility.

♻️ Proposed fix to split comma-separated origins
+import java.util.Arrays;
 ...
     `@Bean`
     public CorsConfigurationSource corsConfigurationSource() {
         CorsConfiguration configuration = new CorsConfiguration();
 
-        configuration.setAllowedOrigins(List.of(allowedOrigins));
+        configuration.setAllowedOrigins(
+            Arrays.asList(allowedOrigins.split("\\s*,\\s*"))
+        );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/saccoplus/config/SecurityConfig.java` around lines 82 - 87,
The corsConfigurationSource method currently calls List.of(allowedOrigins) which
treats the entire CORS_ORIGINS string as one origin; update
corsConfigurationSource to split the allowedOrigins string on commas (and trim
whitespace) and pass the resulting List<String> into
configuration.setAllowedOrigins(...) so multiple comma-separated origins (e.g.
"http://a.com,http://b.com") are recognized; refer to the
corsConfigurationSource method and the allowedOrigins field/variable when making
this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/com/saccoplus/security/JwtUtils.java`:
- Around line 20-22: The getSigningKey method uses SECRET_KEY.getBytes() which
depends on the platform default charset; change it to use an explicit charset
(e.g., StandardCharsets.UTF_8) so the key bytes are consistent across
JVMs—update the getSigningKey() implementation to convert SECRET_KEY to bytes
using a fixed charset reference and import/use java.nio.charset.StandardCharsets
to avoid platform-dependent signature failures.

---

Nitpick comments:
In `@src/main/java/com/saccoplus/config/SecurityConfig.java`:
- Around line 30-31: Remove the leftover inline development comment by deleting
the trailing "// ← ADD THIS" on the allowedOrigins field in the SecurityConfig
class; ensure the field declaration remains as a clean annotation and private
String allowedOrigins; so only the `@Value`("${app.cors.allowed-origins}")
annotation and the private String allowedOrigins; line remain.
- Around line 82-87: The corsConfigurationSource method currently calls
List.of(allowedOrigins) which treats the entire CORS_ORIGINS string as one
origin; update corsConfigurationSource to split the allowedOrigins string on
commas (and trim whitespace) and pass the resulting List<String> into
configuration.setAllowedOrigins(...) so multiple comma-separated origins (e.g.
"http://a.com,http://b.com") are recognized; refer to the
corsConfigurationSource method and the allowedOrigins field/variable when making
this change.
🪄 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: 69179706-a989-410c-9ee2-720058e3c667

📥 Commits

Reviewing files that changed from the base of the PR and between 78f32d3 and 898bc79.

📒 Files selected for processing (7)
  • .gitignore
  • src/main/java/com/saccoplus/config/SecurityConfig.java
  • src/main/java/com/saccoplus/dto/request/RegisterRequest.java
  • src/main/java/com/saccoplus/entity/IndividualUser.java
  • src/main/java/com/saccoplus/entity/Wallet.java
  • src/main/java/com/saccoplus/security/JwtUtils.java
  • src/main/java/com/saccoplus/service/AuthService.java
✅ Files skipped from review due to trivial changes (3)
  • .gitignore
  • src/main/java/com/saccoplus/entity/Wallet.java
  • src/main/java/com/saccoplus/entity/IndividualUser.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/java/com/saccoplus/service/AuthService.java
  • src/main/java/com/saccoplus/dto/request/RegisterRequest.java

Comment thread src/main/java/com/saccoplus/security/JwtUtils.java

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/main/java/com/saccoplus/security/JwtFilter.java (1)

68-71: ⚡ Quick win

Narrow the catch clause to avoid silencing programming errors in the filter body.

catch (Exception e) intercepts not only JWT parsing failures (JwtException, IllegalArgumentException) but also any NullPointerException or other bugs introduced inside the try block. These get logged as "Invalid JWT token" and silently swallowed, making them very hard to diagnose.

♻️ Proposed refactor
-        } 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 the current code and only fix it if needed.

In `@src/main/java/com/saccoplus/security/JwtFilter.java` around lines 68 - 71,
The catch-all Exception in JwtFilter is hiding programming errors; narrow it to
only JWT/parsing failures by replacing catch (Exception e) with a multi-catch
for the JWT-related exceptions (e.g., catch (io.jsonwebtoken.JwtException |
IllegalArgumentException e)) so only token parsing errors are logged via
logger.warn("Invalid JWT token: " + e.getMessage()), and allow other runtime
exceptions (NPEs, etc.) to propagate rather than being swallowed in the
JwtFilter try block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/main/java/com/saccoplus/security/JwtFilter.java`:
- Around line 68-71: The catch-all Exception in JwtFilter is hiding programming
errors; narrow it to only JWT/parsing failures by replacing catch (Exception e)
with a multi-catch for the JWT-related exceptions (e.g., catch
(io.jsonwebtoken.JwtException | IllegalArgumentException e)) so only token
parsing errors are logged via logger.warn("Invalid JWT token: " +
e.getMessage()), and allow other runtime exceptions (NPEs, etc.) to propagate
rather than being swallowed in the JwtFilter try block.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d4b1bafe-ed99-4dd2-8cf4-217b234dc6f6

📥 Commits

Reviewing files that changed from the base of the PR and between 898bc79 and b0d88ab.

📒 Files selected for processing (1)
  • src/main/java/com/saccoplus/security/JwtFilter.java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant