jwt api implementation #110
Conversation
|
Warning Rate limit exceeded@indraniBan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 46 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
WalkthroughThe changes in this pull request primarily involve the addition of JSON Web Token (JWT) functionality to the application. This includes the introduction of several new dependencies in the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (9)
src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java (1)
58-60: Ensure consistent use of case sensitivity in path comparisonsThe path comparison methods are inconsistent;
equalsis used for/user/userAuthenticate, whileequalsIgnoreCaseis used for/user/logOutUserFromConcurrentSession. This inconsistency might lead to unexpected behavior if the case sensitivity of the paths matters.Consider using the same method for all path comparisons to ensure consistent behavior. For example, if paths are case-insensitive, use
equalsIgnoreCasefor all comparisons:-if (path.equals(contextPath + "/user/userAuthenticate") || +if (path.equalsIgnoreCase(contextPath + "/user/userAuthenticate") || path.equalsIgnoreCase(contextPath + "/user/logOutUserFromConcurrentSession") || path.startsWith(contextPath + "/public")) {src/main/java/com/iemr/hwc/utils/JwtUtil.java (2)
21-21: UseTimeUnitfor Better ReadabilityTo improve readability and prevent potential calculation errors with time units, consider using
TimeUnit.DAYS.toMillis(1)for setting theEXPIRATION_TIME.Apply this change:
-private static final long EXPIRATION_TIME = 24 * 60 * 60 * 1000; // 1 day in milliseconds +private static final long EXPIRATION_TIME = TimeUnit.DAYS.toMillis(1); // 1 day in milliseconds
28-28: Specify Charset When ConvertingSECRET_KEYto BytesUsing
SECRET_KEY.getBytes()without specifying a charset relies on the platform's default charset, which can lead to inconsistent behavior. Specify the charset explicitly to ensure consistent encoding.Apply this change:
-return Keys.hmacShaKeyFor(SECRET_KEY.getBytes()); +return Keys.hmacShaKeyFor(SECRET_KEY.getBytes(StandardCharsets.UTF_8));Don't forget to import the
StandardCharsetsclass:import java.nio.charset.StandardCharsets;src/main/java/com/iemr/hwc/utils/JwtAuthenticationUtil.java (2)
36-60: Return Consistent Response Types invalidateJwtTokenMethodThe method mixes returning
ResponseEntity<String>with different HTTP statuses and messages. For better RESTful practices, consider creating a dedicated response object or throwing exceptions that are globally handled to produce consistent responses.
62-90: Handle Specific Exceptions invalidateUserIdAndJwtTokenMethodCatching a general
Exceptionmay mask underlying issues. Catch specific exceptions likeJwtException,NumberFormatException, etc., to handle different error scenarios appropriately.Apply this change:
- } catch (Exception e) { + } catch (JwtException | IllegalArgumentException e) { logger.error("Validation failed: " + e.getMessage(), e); throw new IEMRException("Validation error: " + e.getMessage(), e); }Also, import necessary exception classes:
import io.jsonwebtoken.JwtException;src/main/java/com/iemr/hwc/utils/CookieUtil.java (1)
30-33: Set SameSite Attribute for CookiesTo enhance security against Cross-Site Request Forgery (CSRF) attacks, consider setting the
SameSiteattribute on cookies.Apply this change:
cookie.setHttpOnly(true); // Prevent JavaScript access for security cookie.setSecure(true); // Ensure the cookie is sent only over HTTPS + cookie.setPath("/"); // Make the cookie available for the entire application + cookie.setMaxAge(60 * 60 * 24); // 1 day expiration time + cookie.setAttribute("SameSite", "Strict"); // Prevent CSRF response.addCookie(cookie); // Add the cookie to the responseNote: Setting
SameSitemay require additional configuration depending on your server and servlet version.src/main/environment/common_uat.properties (1)
Line range hint
89-93: Comprehensive JWT Security Implementation RecommendationsTo ensure a robust and secure JWT implementation, consider implementing these additional security measures:
Token Management:
- Implement token expiration (exp claim)
- Add token refresh mechanism
- Include standard claims (iss, sub, aud)
- Implement token revocation capability
Security Headers:
- Add security headers (e.g.,
jtifor unique token ID)- Implement signature verification
- Use strong encryption algorithms
Error Handling:
- Implement proper error responses for token validation failures
- Add logging for security events
- Handle token refresh scenarios
Would you like me to provide example implementations for any of these security measures?
src/main/environment/common_test.properties (1)
93-96: Remove unnecessary blank linesThe JWT configuration is separated by multiple blank lines at the end of the file.
jwt.secret=${JWT_SECRET_KEY} -pom.xml (1)
281-300: Consider adding Spring Security dependencySince you're implementing JWT authentication, consider adding Spring Security for a comprehensive security solution.
Add the following dependency:
<dependency> <groupId>org.springframework.boot</groupId> <artifactId>spring-boot-starter-security</artifactId> </dependency>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
pom.xml(1 hunks)src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_dev.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/environment/common_prod.properties(1 hunks)src/main/environment/common_test.properties(1 hunks)src/main/environment/common_uat.properties(1 hunks)src/main/java/com/iemr/hwc/utils/CookieUtil.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtAuthenticationUtil.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtUtil.java(1 hunks)
🔇 Additional comments (3)
src/main/environment/common_prod.properties (1)
91-91: Verify JWT configuration in production
The production environment requires additional security considerations:
- Ensure proper secret rotation mechanisms are in place
- Implement monitoring for JWT-related security events
- Consider adding JWT configuration parameters like token expiration time
Additional JWT configuration properties to consider:
jwt.token.validity=3600
jwt.token.issuer=hwc-api
jwt.token.audience=hwc-clientssrc/main/environment/common_uat.properties (1)
93-93: Ensure consistent JWT configuration across environments
The UAT environment should mirror production configuration to ensure proper testing:
- Token validation behavior
- Secret rotation mechanisms
- Error handling for invalid/expired tokens
Run this script to check JWT configuration consistency across environments:
✅ Verification successful
JWT configuration is consistent across all environments
The verification shows that all environment files (CI, DEV, TEST, UAT, and PROD) have identical JWT secret configuration patterns. Each environment file contains the same placeholder <Enter_Your_Secret_Key>, indicating a consistent configuration structure. This suggests that:
- The token validation behavior will be uniform
- The secret configuration mechanism is standardized
- The placeholder indicates proper secret management practices where actual values are injected during deployment
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check JWT configuration consistency across environment files
echo "Checking JWT configuration patterns across environment files..."
rg -p "jwt\." src/main/environment/common_*.properties
Length of output: 956
pom.xml (1)
281-300: 🛠️ Refactor suggestion
Improve JWT dependency management
Several improvements needed for JWT dependencies:
- Use properties for version management
- Update to the latest stable version (0.12.3)
- Group with other security-related dependencies
Apply these changes:
+ <properties>
+ <!-- Existing properties -->
+ <jjwt.version>0.12.3</jjwt.version>
+ </properties>
<!-- JWT Dependencies -->
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-api</artifactId>
- <version>0.11.5</version>
+ <version>${jjwt.version}</version>
</dependency>
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-impl</artifactId>
- <version>0.11.5</version>
+ <version>${jjwt.version}</version>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-jackson</artifactId>
- <version>0.11.5</version>
+ <version>${jjwt.version}</version>
<scope>runtime</scope>
</dependency>Let's verify the security of the JWT dependencies:
✅ Verification successful
Let me verify the JWT usage in the codebase to ensure the upgrade is safe.
Let me check the specific JWT usage patterns in the JwtUtil class to ensure compatibility with the newer version.
Upgrade of JWT dependencies to 0.12.3 is safe and recommended
The code review suggestion is valid. Based on the verification:
- No security vulnerabilities found for the JWT library
- Current JWT usage in
JwtUtil.javais compatible with 0.12.3:- Uses modern APIs like
parserBuilder()andKeys.hmacShaKeyFor() - All method calls (
builder(),signWith(),parseClaimsJws()) are supported in the newer version
- Uses modern APIs like
- Version management through properties will improve maintainability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for known vulnerabilities in JWT dependencies
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "io.jsonwebtoken:jjwt-api") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 359
Script:
#!/bin/bash
# Search for JWT usage patterns
ast-grep --pattern 'Jwts.$_($$$)'
# Also check for any direct version references
rg -l "0.11.5" --type java
Length of output: 312
Script:
#!/bin/bash
# Check the full content of JwtUtil.java to verify JWT usage patterns
cat src/main/java/com/iemr/hwc/utils/JwtUtil.java
Length of output: 2390
| try { | ||
| // Retrieve JWT token from cookies | ||
| String jwtTokenFromCookie = getJwtTokenFromCookies(request); | ||
| logger.info("JWT token from cookie: " + jwtTokenFromCookie); |
There was a problem hiding this comment.
Avoid logging JWT tokens to prevent security risks
Logging JWT tokens can expose sensitive information and pose security risks if the logs are accessed by unauthorized individuals.
Consider modifying the log statement to avoid including the JWT token. Apply this diff to remove the token from the logs:
-logger.info("JWT token from cookie: " + jwtTokenFromCookie);
+logger.info("JWT token from cookie received");📝 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.
| logger.info("JWT token from cookie: " + jwtTokenFromCookie); | |
| logger.info("JWT token from cookie received"); |
|
|
||
| // Log headers for debugging | ||
| String jwtTokenFromHeader = request.getHeader("Jwttoken"); | ||
| logger.info("JWT token from header: " + jwtTokenFromHeader); |
There was a problem hiding this comment.
Avoid logging JWT tokens to prevent security risks
Logging JWT tokens can expose sensitive information and pose security risks if the logs are accessed by unauthorized individuals.
Consider modifying the log statement to avoid including the JWT token. Apply this diff to remove the token from the logs:
-logger.info("JWT token from header: " + jwtTokenFromHeader);
+logger.info("JWT token from header received");📝 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.
| logger.info("JWT token from header: " + jwtTokenFromHeader); | |
| logger.info("JWT token from header received"); |
| // If token is valid, allow the request to proceed | ||
| filterChain.doFilter(servletRequest, servletResponse); | ||
| } else { | ||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid JWT token"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use generic error messages to prevent information leakage
Providing specific reasons for authentication failure can aid attackers by revealing aspects of the security implementation.
Consider using a generic error message. Apply this diff:
-response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid JWT token");
+response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized");📝 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.
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid JWT token"); | |
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); |
| // Determine which token (cookie or header) to validate | ||
| String jwtToken = jwtTokenFromCookie != null ? jwtTokenFromCookie : jwtTokenFromHeader; | ||
| if (jwtToken == null) { | ||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "JWT token not found in cookies or headers"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use generic error messages to prevent information leakage
Providing specific reasons for authentication failure can aid attackers by revealing aspects of the security implementation.
Consider using a generic error message. Apply this diff:
-response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "JWT token not found in cookies or headers");
+response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized");📝 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.
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "JWT token not found in cookies or headers"); | |
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized"); |
| logger.error("Authorization error: ", e); | ||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Authorization error: " + e.getMessage()); |
There was a problem hiding this comment.
Avoid exposing exception details in the HTTP response
Sending detailed exception messages to the client can reveal sensitive information about the application's internals and aid malicious actors.
Consider sending a generic error message instead of including the exception message. Apply this diff:
-response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Authorization error: " + e.getMessage());
+response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Authorization error");📝 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.
| logger.error("Authorization error: ", e); | |
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Authorization error: " + e.getMessage()); | |
| logger.error("Authorization error: ", e); | |
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Authorization error"); |
| return Arrays.stream(request.getCookies()) | ||
| .filter(cookie -> "Jwttoken".equals(cookie.getName())) | ||
| .map(Cookie::getValue) | ||
| .findFirst() | ||
| .orElse(null); | ||
| } |
There was a problem hiding this comment.
Prevent NullPointerException When Retrieving Cookies
request.getCookies() can return null if there are no cookies present. This can lead to a NullPointerException when using Arrays.stream(). Add a null check before processing.
Apply this change:
public String getJwtTokenFromCookie(HttpServletRequest request) {
+ Cookie[] cookies = request.getCookies();
+ if (cookies == null) {
+ return null;
+ }
- return Arrays.stream(request.getCookies())
+ return Arrays.stream(cookies)
.filter(cookie -> "Jwttoken".equals(cookie.getName()))
.map(Cookie::getValue)
.findFirst()
.orElse(null);
}📝 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.
| return Arrays.stream(request.getCookies()) | |
| .filter(cookie -> "Jwttoken".equals(cookie.getName())) | |
| .map(Cookie::getValue) | |
| .findFirst() | |
| .orElse(null); | |
| } | |
| public String getJwtTokenFromCookie(HttpServletRequest request) { | |
| Cookie[] cookies = request.getCookies(); | |
| if (cookies == null) { | |
| return null; | |
| } | |
| return Arrays.stream(cookies) | |
| .filter(cookie -> "Jwttoken".equals(cookie.getName())) | |
| .map(Cookie::getValue) | |
| .findFirst() | |
| .orElse(null); | |
| } |
| @@ -86,6 +86,7 @@ prescription=TMPrescription SMS | |||
|
|
|||
| ### Redis IP | |||
| spring.redis.host=localhost | |||
| jwt.secret=<Enter_Your_Secret_Key> | |||
There was a problem hiding this comment.
Secure Handling of jwt.secret Configuration
Ensure that the jwt.secret is not hardcoded in configuration files, especially those checked into source control. Use environment variables or a secret management tool to securely provide this value in different environments.
| @@ -86,6 +86,7 @@ prescription=TMPrescription SMS | |||
|
|
|||
| ### Redis IP | |||
| spring.redis.host=localhost | |||
| jwt.secret=<Enter_Your_Secret_Key> | |||
There was a problem hiding this comment.
Secure Handling of jwt.secret Configuration
The jwt.secret should not be stored in plain text within configuration files. Consider using environment variables or secure parameter stores to manage sensitive information in CI environments.
| @@ -86,6 +86,7 @@ prescription=TMPrescription SMS | |||
|
|
|||
| ### Redis IP | |||
| spring.redis.host=localhost | |||
| jwt.secret=<Enter_Your_Secret_Key> | |||
There was a problem hiding this comment.
Critical: Improve JWT secret management
The current implementation of JWT secret management raises several security concerns:
- Using placeholder values (
<Enter_Your_Secret_Key>) in property files is risky as it might lead to deployment with default values - Storing secrets in property files is not a secure practice
- The same placeholder is used across all environments (dev, prod, uat)
Consider these security improvements:
- Use environment variables or a secure secret management service (e.g., HashiCorp Vault, AWS Secrets Manager)
- Ensure different JWT secrets for each environment
- Add validation at startup to prevent the application from running with placeholder secrets
Example implementation using environment variables:
-jwt.secret=<Enter_Your_Secret_Key>
+jwt.secret=${JWT_SECRET}📝 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.
| jwt.secret=<Enter_Your_Secret_Key> | |
| jwt.secret=${JWT_SECRET} |
| @@ -90,6 +90,7 @@ prescription=TMPrescription SMS | |||
|
|
|||
| ### Redis IP | |||
| spring.redis.host=localhost | |||
| jwt.secret=<Enter_Your_Secret_Key> | |||
There was a problem hiding this comment.
Enhance JWT secret configuration security
The JWT secret configuration needs additional security considerations:
- Use a strong, randomly generated key (at least 256 bits)
- Consider using environment variables or a secure vault instead of properties file
- Add documentation about the required format and minimum length
Consider these improvements:
- Use environment variable:
-jwt.secret=<Enter_Your_Secret_Key>
+jwt.secret=${JWT_SECRET_KEY}- Add documentation:
# JWT secret key - Must be at least 256 bits (32 characters) long
# Configure using environment variable JWT_SECRET_KEY
jwt.secret=${JWT_SECRET_KEY}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/main/java/com/iemr/hwc/utils/JwtUtil.java (1)
15-63: Consider additional security measuresThe JWT implementation could benefit from these security enhancements:
- Add token blacklisting for revoked tokens
- Implement rate limiting for token generation
- Add security event logging
- Consider adding token refresh mechanism
Would you like me to provide implementation details for any of these security enhancements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
pom.xml(7 hunks)src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_dev.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/environment/common_prod.properties(1 hunks)src/main/environment/common_test.properties(1 hunks)src/main/environment/common_uat.properties(1 hunks)src/main/java/com/iemr/hwc/utils/CookieUtil.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtAuthenticationUtil.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtUtil.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- src/main/environment/common_prod.properties
- src/main/environment/common_example.properties
- src/main/environment/common_ci.properties
- src/main/environment/common_uat.properties
- src/main/environment/common_test.properties
- src/main/environment/common_dev.properties
- pom.xml
- src/main/java/com/iemr/hwc/utils/JwtAuthenticationUtil.java
- src/main/java/com/iemr/hwc/utils/CookieUtil.java
- src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java
🔇 Additional comments (4)
src/main/java/com/iemr/hwc/utils/JwtUtil.java (4)
18-19:
Security Concern: Store SECRET_KEY Securely
Storing the SECRET_KEY as a String may expose it to memory dumps and increase the risk of unauthorized access. Consider storing it as a char[] or using a secure vault service to manage sensitive keys.
43-49:
Improve Exception Handling in validateToken Method
Catching a general Exception and returning null can make debugging difficult and may lead to NullPointerExceptions elsewhere.
15-63: Verify JWT implementation against security guidelines
Let's verify the implementation against OWASP JWT guidelines and check for potential vulnerabilities.
43-62: 🛠️ Refactor suggestion
Remove code duplication in token parsing logic
The token parsing logic is duplicated between validateToken and extractAllClaims.
Apply this refactor:
-public Claims validateToken(String token) {
+public Optional<Claims> validateToken(String token) {
try {
- return Jwts.parser().setSigningKey(getSigningKey()).build().parseClaimsJws(token).getBody();
+ return Optional.of(extractAllClaims(token));
} catch (Exception e) {
- return null;
+ logger.error("Invalid JWT token: {}", e.getMessage());
+ return Optional.empty();
}
}
public <T> T extractClaim(String token, Function<Claims, T> claimsResolver) {
- final Claims claims = extractAllClaims(token);
+ return validateToken(token)
+ .map(claimsResolver)
+ .orElseThrow(() -> new JwtException("Invalid token"));
- return claimsResolver.apply(claims);
}
private Claims extractAllClaims(String token) {
+ if (token == null || token.trim().isEmpty()) {
+ throw new IllegalArgumentException("Token cannot be null or empty");
+ }
return Jwts.parser().setSigningKey(getSigningKey()).build().parseClaimsJws(token).getBody();
}Likely invalid or redundant comment.
| private Key getSigningKey() { | ||
| if (SECRET_KEY == null || SECRET_KEY.isEmpty()) { | ||
| throw new IllegalStateException("JWT secret key is not set in application.properties"); | ||
| } | ||
| return Keys.hmacShaKeyFor(SECRET_KEY.getBytes()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance key generation robustness
Consider these improvements:
- Specify UTF-8 encoding explicitly when converting to bytes
- Add key length validation
Apply this change:
private Key getSigningKey() {
if (SECRET_KEY == null || SECRET_KEY.isEmpty()) {
throw new IllegalStateException("JWT secret key is not set in application.properties");
}
- return Keys.hmacShaKeyFor(SECRET_KEY.getBytes());
+ byte[] keyBytes = SECRET_KEY.getBytes(StandardCharsets.UTF_8);
+ if (keyBytes.length < 32) {
+ throw new IllegalStateException("JWT secret key must be at least 256 bits long");
+ }
+ return Keys.hmacShaKeyFor(keyBytes);
}📝 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.
| private Key getSigningKey() { | |
| if (SECRET_KEY == null || SECRET_KEY.isEmpty()) { | |
| throw new IllegalStateException("JWT secret key is not set in application.properties"); | |
| } | |
| return Keys.hmacShaKeyFor(SECRET_KEY.getBytes()); | |
| } | |
| private Key getSigningKey() { | |
| if (SECRET_KEY == null || SECRET_KEY.isEmpty()) { | |
| throw new IllegalStateException("JWT secret key is not set in application.properties"); | |
| } | |
| byte[] keyBytes = SECRET_KEY.getBytes(StandardCharsets.UTF_8); | |
| if (keyBytes.length < 32) { | |
| throw new IllegalStateException("JWT secret key must be at least 256 bits long"); | |
| } | |
| return Keys.hmacShaKeyFor(keyBytes); | |
| } |
| public String generateToken(String username, String userId) { | ||
| Date now = new Date(); | ||
| Date expiryDate = new Date(now.getTime() + EXPIRATION_TIME); | ||
|
|
||
| // Include the userId in the JWT claims | ||
| return Jwts.builder().setSubject(username).claim("userId", userId) // Add userId as a claim | ||
| .setIssuedAt(now).setExpiration(expiryDate).signWith(getSigningKey(), SignatureAlgorithm.HS256) | ||
| .compact(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and make expiration time configurable
The token generation lacks input validation and uses a hardcoded expiration time.
Apply these changes:
+@Value("${jwt.expiration:86400000}") // Default to 24 hours
+private long expirationTime;
+
public String generateToken(String username, String userId) {
+ if (username == null || username.trim().isEmpty()) {
+ throw new IllegalArgumentException("Username cannot be null or empty");
+ }
+ if (userId == null || userId.trim().isEmpty()) {
+ throw new IllegalArgumentException("UserId cannot be null or empty");
+ }
Date now = new Date();
- Date expiryDate = new Date(now.getTime() + EXPIRATION_TIME);
+ Date expiryDate = new Date(now.getTime() + expirationTime);📝 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.
| public String generateToken(String username, String userId) { | |
| Date now = new Date(); | |
| Date expiryDate = new Date(now.getTime() + EXPIRATION_TIME); | |
| // Include the userId in the JWT claims | |
| return Jwts.builder().setSubject(username).claim("userId", userId) // Add userId as a claim | |
| .setIssuedAt(now).setExpiration(expiryDate).signWith(getSigningKey(), SignatureAlgorithm.HS256) | |
| .compact(); | |
| } | |
| @Value("${jwt.expiration:86400000}") // Default to 24 hours | |
| private long expirationTime; | |
| public String generateToken(String username, String userId) { | |
| if (username == null || username.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Username cannot be null or empty"); | |
| } | |
| if (userId == null || userId.trim().isEmpty()) { | |
| throw new IllegalArgumentException("UserId cannot be null or empty"); | |
| } | |
| Date now = new Date(); | |
| Date expiryDate = new Date(now.getTime() + expirationTime); | |
| // Include the userId in the JWT claims | |
| return Jwts.builder().setSubject(username).claim("userId", userId) // Add userId as a claim | |
| .setIssuedAt(now).setExpiration(expiryDate).signWith(getSigningKey(), SignatureAlgorithm.HS256) | |
| .compact(); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/main/java/com/iemr/hwc/utils/JwtAuthenticationUtil.java (1)
24-37:⚠️ Potential issueIncomplete constructor injection pattern
The constructor doesn't initialize all dependencies (redisTemplate and userLoginRepo are missing), and field-level @Autowired annotations are still present.
Apply this diff to fix the dependency injection:
- @Autowired private CookieUtil cookieUtil; - @Autowired private JwtUtil jwtUtil; - @Autowired - private RedisTemplate<String, Object> redisTemplate; - @Autowired - private UserLoginRepo userLoginRepo; + private final RedisTemplate<String, Object> redisTemplate; + private final UserLoginRepo userLoginRepo; - public JwtAuthenticationUtil(CookieUtil cookieUtil, JwtUtil jwtUtil) { + public JwtAuthenticationUtil(CookieUtil cookieUtil, JwtUtil jwtUtil, + RedisTemplate<String, Object> redisTemplate, UserLoginRepo userLoginRepo) { this.cookieUtil = cookieUtil; this.jwtUtil = jwtUtil; + this.redisTemplate = redisTemplate; + this.userLoginRepo = userLoginRepo; }
🧹 Nitpick comments (3)
src/main/java/com/iemr/hwc/config/RedisConfig.java (1)
18-21: No-op configuration is acceptable if no specialized Redis action is needed.Returning
ConfigureRedisAction.NO_OPis a valid approach if you need to skip any cluster or keyspace notifications. If future requirements necessitate additional configuration for Redis (e.g., cluster events), then update this method accordingly.src/main/java/com/iemr/hwc/repo/login/UserLoginRepo.java (1)
36-36: Repository method correctly queries by userID.The new
getUserByUserIDmethod aligns well with existing patterns. For performance, confirm that there is an index onuserIDto optimize lookups, particularly if the table is large. Also consider adding documentation for future maintainers about when to use this method instead ofgetUserByUsername.src/main/java/com/iemr/hwc/Application.java (1)
65-68: Registering the FHIR servlet with a descriptive name.The addition of a new FHIR servlet is well-labeled, which clarifies the endpoint’s purpose. Remember to keep an eye on the potential performance impact of multiple active servlets if your application grows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_dev.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/environment/common_prod.properties(1 hunks)src/main/environment/common_test.properties(1 hunks)src/main/environment/common_uat.properties(1 hunks)src/main/java/com/iemr/hwc/Application.java(2 hunks)src/main/java/com/iemr/hwc/config/RedisConfig.java(1 hunks)src/main/java/com/iemr/hwc/data/login/Users.java(1 hunks)src/main/java/com/iemr/hwc/repo/login/UserLoginRepo.java(1 hunks)src/main/java/com/iemr/hwc/utils/CookieUtil.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtAuthenticationUtil.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtUtil.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/main/environment/common_dev.properties
- src/main/environment/common_prod.properties
- src/main/environment/common_uat.properties
- src/main/environment/common_example.properties
- src/main/environment/common_ci.properties
- src/main/java/com/iemr/hwc/utils/CookieUtil.java
- src/main/environment/common_test.properties
- src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java
- src/main/java/com/iemr/hwc/utils/JwtUtil.java
🔇 Additional comments (5)
src/main/java/com/iemr/hwc/config/RedisConfig.java (2)
14-16: Well-structured class designation for Redis integration.
Declaring @Configuration and @EnableCaching in a dedicated configuration class is a clean approach. Just ensure system property or environment-based profiles are managed correctly if you anticipate multiple Redis configurations for different deployment environments.
23-36: Optimal usage of String and JSON Redis serializers for Users objects.
- Using
StringRedisSerializerfor keys is a great choice to keep them human-readable. - Using
Jackson2JsonRedisSerializerforUsersis convenient for object caching. However, ensure that the entity’s serialization will remain backward compatible if structure changes over time (e.g., versioning or schema evolution).
src/main/java/com/iemr/hwc/data/login/Users.java (1)
24-29: Serializable and JSON-ignorance flags improve maintainability.
- Implementing
Serializableis necessary to store Java objects in Redis reliably. @JsonIgnoreProperties(ignoreUnknown = true)ensures flexibility if the JSON structure changes across different services or versions.- Validate that fields intended to be serialized are properly annotated or excluded, as necessary, to avoid unintended data leakage during caching.
Also applies to: 37-38
src/main/java/com/iemr/hwc/Application.java (1)
35-38: New Redis-related imports are consistent with the added bean definition.
Ensure that spring-boot-starter-data-redis dependencies (and any transitive dependencies for serialization) match the versions needed for compatibility across your microservice environment.
src/main/java/com/iemr/hwc/utils/JwtAuthenticationUtil.java (1)
1-126: Verify JWT implementation security
Let's verify the JWT implementation in related files to ensure secure token handling.
| @Bean | ||
| public RedisTemplate<String, Object> redisTemplate(RedisConnectionFactory factory) { | ||
| RedisTemplate<String, Object> template = new RedisTemplate<>(); | ||
| template.setConnectionFactory(factory); | ||
|
|
||
| // Use StringRedisSerializer for keys (userId) | ||
| template.setKeySerializer(new StringRedisSerializer()); | ||
|
|
||
| // Use Jackson2JsonRedisSerializer for values (Users objects) | ||
| Jackson2JsonRedisSerializer<Users> serializer = new Jackson2JsonRedisSerializer<>(Users.class); | ||
| template.setValueSerializer(serializer); | ||
|
|
||
| return template; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Generic type usage in RedisTemplate can lead to type mismatches if not carefully handled.
- You are returning
RedisTemplate<String, Object>but only configuring the value serializer forUsers. If you plan to store other object types, consider a more universal approach or define multiple templates with narrower type parameters. - As with
RedisConfig, confirm that storedUsersobjects remain compatible through version changes.
| public boolean validateUserIdAndJwtToken(String jwtToken) throws IEMRException { | ||
| try { | ||
| // Validate JWT token and extract claims | ||
| Claims claims = jwtUtil.validateToken(jwtToken); | ||
|
|
||
| if (claims == null) { | ||
| throw new IEMRException("Invalid JWT token."); | ||
| } | ||
|
|
||
| String userId = claims.get("userId", String.class); | ||
|
|
||
| // Check if user data is present in Redis | ||
| Users user = getUserFromCache(userId); | ||
| if (user == null) { | ||
| // If not in Redis, fetch from DB and cache the result | ||
| user = fetchUserFromDB(userId); | ||
| } | ||
| if (user == null) { | ||
| throw new IEMRException("Invalid User ID."); | ||
| } | ||
|
|
||
| return true; // Valid userId and JWT token | ||
| } catch (Exception e) { | ||
| logger.error("Validation failed: " + e.getMessage(), e); | ||
| throw new IEMRException("Validation error: " + e.getMessage(), e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve error handling and input validation
The validateUserIdAndJwtToken method needs improvements in error handling and input validation:
Apply these changes:
public boolean validateUserIdAndJwtToken(String jwtToken) throws IEMRException {
+ if (jwtToken == null || jwtToken.trim().isEmpty()) {
+ throw new IEMRException("JWT token cannot be null or empty");
+ }
+
try {
// Validate JWT token and extract claims
Claims claims = jwtUtil.validateToken(jwtToken);
if (claims == null) {
throw new IEMRException("Invalid JWT token.");
}
- String userId = claims.get("userId", String.class);
+ Object userIdObj = claims.get("userId");
+ if (userIdObj == null) {
+ throw new IEMRException("User ID claim is missing from token");
+ }
+
+ String userId;
+ try {
+ userId = String.valueOf(userIdObj);
+ } catch (Exception e) {
+ throw new IEMRException("Invalid user ID format in token");
+ }
// Check if user data is present in Redis
Users user = getUserFromCache(userId);
if (user == null) {
// If not in Redis, fetch from DB and cache the result
user = fetchUserFromDB(userId);
}
if (user == null) {
throw new IEMRException("Invalid User ID.");
}
return true; // Valid userId and JWT token
- } catch (Exception e) {
+ } catch (IEMRException e) {
+ throw e;
+ } catch (NumberFormatException e) {
+ logger.error("Invalid user ID format: " + e.getMessage(), e);
+ throw new IEMRException("Invalid user ID format", e);
+ } catch (Exception e) {
logger.error("Validation failed: " + e.getMessage(), e);
throw new IEMRException("Validation error: " + e.getMessage(), e);
}
}📝 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.
| public boolean validateUserIdAndJwtToken(String jwtToken) throws IEMRException { | |
| try { | |
| // Validate JWT token and extract claims | |
| Claims claims = jwtUtil.validateToken(jwtToken); | |
| if (claims == null) { | |
| throw new IEMRException("Invalid JWT token."); | |
| } | |
| String userId = claims.get("userId", String.class); | |
| // Check if user data is present in Redis | |
| Users user = getUserFromCache(userId); | |
| if (user == null) { | |
| // If not in Redis, fetch from DB and cache the result | |
| user = fetchUserFromDB(userId); | |
| } | |
| if (user == null) { | |
| throw new IEMRException("Invalid User ID."); | |
| } | |
| return true; // Valid userId and JWT token | |
| } catch (Exception e) { | |
| logger.error("Validation failed: " + e.getMessage(), e); | |
| throw new IEMRException("Validation error: " + e.getMessage(), e); | |
| } | |
| } | |
| public boolean validateUserIdAndJwtToken(String jwtToken) throws IEMRException { | |
| if (jwtToken == null || jwtToken.trim().isEmpty()) { | |
| throw new IEMRException("JWT token cannot be null or empty"); | |
| } | |
| try { | |
| // Validate JWT token and extract claims | |
| Claims claims = jwtUtil.validateToken(jwtToken); | |
| if (claims == null) { | |
| throw new IEMRException("Invalid JWT token."); | |
| } | |
| Object userIdObj = claims.get("userId"); | |
| if (userIdObj == null) { | |
| throw new IEMRException("User ID claim is missing from token"); | |
| } | |
| String userId; | |
| try { | |
| userId = String.valueOf(userIdObj); | |
| } catch (Exception e) { | |
| throw new IEMRException("Invalid user ID format in token"); | |
| } | |
| // Check if user data is present in Redis | |
| Users user = getUserFromCache(userId); | |
| if (user == null) { | |
| // If not in Redis, fetch from DB and cache the result | |
| user = fetchUserFromDB(userId); | |
| } | |
| if (user == null) { | |
| throw new IEMRException("Invalid User ID."); | |
| } | |
| return true; // Valid userId and JWT token | |
| } catch (IEMRException e) { | |
| throw e; | |
| } catch (NumberFormatException e) { | |
| logger.error("Invalid user ID format: " + e.getMessage(), e); | |
| throw new IEMRException("Invalid user ID format", e); | |
| } catch (Exception e) { | |
| logger.error("Validation failed: " + e.getMessage(), e); | |
| throw new IEMRException("Validation error: " + e.getMessage(), e); | |
| } | |
| } |
| public ResponseEntity<String> validateJwtToken(HttpServletRequest request) { | ||
| Optional<String> jwtTokenOpt = cookieUtil.getCookieValue(request, "Jwttoken"); | ||
|
|
||
| if (jwtTokenOpt.isEmpty()) { | ||
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED) | ||
| .body("Error 401: Unauthorized - JWT Token is not set!"); | ||
| } | ||
|
|
||
| String jwtToken = jwtTokenOpt.get(); | ||
|
|
||
| // Validate the token | ||
| Claims claims = jwtUtil.validateToken(jwtToken); | ||
| if (claims == null) { | ||
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body("Error 401: Unauthorized - Invalid JWT Token!"); | ||
| } | ||
|
|
||
| // Extract username from token | ||
| String usernameFromToken = claims.getSubject(); | ||
| if (usernameFromToken == null || usernameFromToken.isEmpty()) { | ||
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED) | ||
| .body("Error 401: Unauthorized - Username is missing!"); | ||
| } | ||
|
|
||
| // Return the username if valid | ||
| return ResponseEntity.ok(usernameFromToken); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Security and maintainability improvements needed
Several improvements are recommended for the validateJwtToken method:
- Move the cookie name to a constant
- Use generic error messages to avoid exposing internal details
- Add security event logging
Apply these changes:
+ private static final String JWT_COOKIE_NAME = "Jwttoken";
+
public ResponseEntity<String> validateJwtToken(HttpServletRequest request) {
- Optional<String> jwtTokenOpt = cookieUtil.getCookieValue(request, "Jwttoken");
+ Optional<String> jwtTokenOpt = cookieUtil.getCookieValue(request, JWT_COOKIE_NAME);
+ logger.debug("Validating JWT token from request");
if (jwtTokenOpt.isEmpty()) {
+ logger.warn("JWT token not found in request");
return ResponseEntity.status(HttpStatus.UNAUTHORIZED)
- .body("Error 401: Unauthorized - JWT Token is not set!");
+ .body("Authentication required");
}
String jwtToken = jwtTokenOpt.get();
// Validate the token
Claims claims = jwtUtil.validateToken(jwtToken);
if (claims == null) {
+ logger.warn("Invalid JWT token detected");
return ResponseEntity.status(HttpStatus.UNAUTHORIZED)
- .body("Error 401: Unauthorized - Invalid JWT Token!");
+ .body("Authentication failed");
}
// Extract username from token
String usernameFromToken = claims.getSubject();
if (usernameFromToken == null || usernameFromToken.isEmpty()) {
+ logger.warn("JWT token missing username claim");
return ResponseEntity.status(HttpStatus.UNAUTHORIZED)
- .body("Error 401: Unauthorized - Username is missing!");
+ .body("Authentication failed");
}
+ logger.debug("JWT token validation successful for user: {}", usernameFromToken);
// Return the username if valid
return ResponseEntity.ok(usernameFromToken);
}Committable suggestion skipped: line range outside the PR's diff.
| private Users getUserFromCache(String userId) { | ||
| String redisKey = "user_" + userId; // The Redis key format | ||
| Users user = (Users) redisTemplate.opsForValue().get(redisKey); | ||
|
|
||
| if (user == null) { | ||
| logger.warn("User not found in Redis. Will try to fetch from DB."); | ||
| } else { | ||
| logger.info("User fetched successfully from Redis."); | ||
| } | ||
|
|
||
| return user; // Returns null if not found | ||
| } | ||
|
|
||
| private Users fetchUserFromDB(String userId) { | ||
| // This method will only be called if the user is not found in Redis. | ||
| String redisKey = "user_" + userId; // Redis key format | ||
|
|
||
| // Fetch user from DB | ||
| Users user = userLoginRepo.getUserByUserID(Long.parseLong(userId)); | ||
|
|
||
| if (user != null) { | ||
| // Cache the user in Redis for future requests (cache for 30 minutes) | ||
| redisTemplate.opsForValue().set(redisKey, user, 30, TimeUnit.MINUTES); | ||
|
|
||
| // Log that the user has been stored in Redis | ||
| logger.info("User stored in Redis with key: " + redisKey); | ||
| } else { | ||
| logger.warn("User not found for userId: " + userId); | ||
| } | ||
|
|
||
| return user; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve cache management configuration and error handling
The cache management methods need improvements in configuration and error handling:
Apply these changes:
+ private static final String REDIS_USER_KEY_PREFIX = "user_";
+ @Value("${redis.user.cache.ttl.minutes:30}")
+ private int userCacheTTLMinutes;
+
private Users getUserFromCache(String userId) {
- String redisKey = "user_" + userId; // The Redis key format
+ validateUserId(userId);
+ String redisKey = REDIS_USER_KEY_PREFIX + userId;
Users user = (Users) redisTemplate.opsForValue().get(redisKey);
if (user == null) {
logger.warn("User not found in Redis. Will try to fetch from DB.");
} else {
- logger.info("User fetched successfully from Redis.");
+ logger.debug("User fetched successfully from Redis for userId: {}", userId);
}
return user; // Returns null if not found
}
private Users fetchUserFromDB(String userId) {
+ validateUserId(userId);
// This method will only be called if the user is not found in Redis.
- String redisKey = "user_" + userId; // Redis key format
+ String redisKey = REDIS_USER_KEY_PREFIX + userId;
// Fetch user from DB
- Users user = userLoginRepo.getUserByUserID(Long.parseLong(userId));
+ Long userIdLong;
+ try {
+ userIdLong = Long.parseLong(userId);
+ } catch (NumberFormatException e) {
+ logger.error("Invalid userId format: {}", userId);
+ return null;
+ }
+
+ Users user = userLoginRepo.getUserByUserID(userIdLong);
if (user != null) {
// Cache the user in Redis for future requests (cache for 30 minutes)
- redisTemplate.opsForValue().set(redisKey, user, 30, TimeUnit.MINUTES);
+ redisTemplate.opsForValue().set(redisKey, user, userCacheTTLMinutes, TimeUnit.MINUTES);
// Log that the user has been stored in Redis
- logger.info("User stored in Redis with key: " + redisKey);
+ logger.debug("User cached in Redis for userId: {}", userId);
} else {
- logger.warn("User not found for userId: " + userId);
+ logger.warn("User not found in database for userId: {}", userId);
}
return user;
}
+
+ private void validateUserId(String userId) {
+ if (userId == null || userId.trim().isEmpty()) {
+ throw new IllegalArgumentException("User ID cannot be null or empty");
+ }
+ if (!userId.matches("^\\d+$")) {
+ throw new IllegalArgumentException("User ID must be a valid number");
+ }
+ }📝 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.
| private Users getUserFromCache(String userId) { | |
| String redisKey = "user_" + userId; // The Redis key format | |
| Users user = (Users) redisTemplate.opsForValue().get(redisKey); | |
| if (user == null) { | |
| logger.warn("User not found in Redis. Will try to fetch from DB."); | |
| } else { | |
| logger.info("User fetched successfully from Redis."); | |
| } | |
| return user; // Returns null if not found | |
| } | |
| private Users fetchUserFromDB(String userId) { | |
| // This method will only be called if the user is not found in Redis. | |
| String redisKey = "user_" + userId; // Redis key format | |
| // Fetch user from DB | |
| Users user = userLoginRepo.getUserByUserID(Long.parseLong(userId)); | |
| if (user != null) { | |
| // Cache the user in Redis for future requests (cache for 30 minutes) | |
| redisTemplate.opsForValue().set(redisKey, user, 30, TimeUnit.MINUTES); | |
| // Log that the user has been stored in Redis | |
| logger.info("User stored in Redis with key: " + redisKey); | |
| } else { | |
| logger.warn("User not found for userId: " + userId); | |
| } | |
| return user; | |
| } | |
| private static final String REDIS_USER_KEY_PREFIX = "user_"; | |
| @Value("${redis.user.cache.ttl.minutes:30}") | |
| private int userCacheTTLMinutes; | |
| private Users getUserFromCache(String userId) { | |
| validateUserId(userId); | |
| String redisKey = REDIS_USER_KEY_PREFIX + userId; | |
| Users user = (Users) redisTemplate.opsForValue().get(redisKey); | |
| if (user == null) { | |
| logger.warn("User not found in Redis. Will try to fetch from DB."); | |
| } else { | |
| logger.debug("User fetched successfully from Redis for userId: {}", userId); | |
| } | |
| return user; // Returns null if not found | |
| } | |
| private Users fetchUserFromDB(String userId) { | |
| validateUserId(userId); | |
| // This method will only be called if the user is not found in Redis. | |
| String redisKey = REDIS_USER_KEY_PREFIX + userId; | |
| // Fetch user from DB | |
| Long userIdLong; | |
| try { | |
| userIdLong = Long.parseLong(userId); | |
| } catch (NumberFormatException e) { | |
| logger.error("Invalid userId format: {}", userId); | |
| return null; | |
| } | |
| Users user = userLoginRepo.getUserByUserID(userIdLong); | |
| if (user != null) { | |
| // Cache the user in Redis for future requests (cache for 30 minutes) | |
| redisTemplate.opsForValue().set(redisKey, user, userCacheTTLMinutes, TimeUnit.MINUTES); | |
| // Log that the user has been stored in Redis | |
| logger.debug("User cached in Redis for userId: {}", userId); | |
| } else { | |
| logger.warn("User not found in database for userId: {}", userId); | |
| } | |
| return user; | |
| } | |
| private void validateUserId(String userId) { | |
| if (userId == null || userId.trim().isEmpty()) { | |
| throw new IllegalArgumentException("User ID cannot be null or empty"); | |
| } | |
| if (!userId.matches("^\\d+$")) { | |
| throw new IllegalArgumentException("User ID must be a valid number"); | |
| } | |
| } |
|
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Configuration Updates
These updates improve the application's security and provide a robust framework for user authentication.