JwtToken,User-Agent validation changes #77
Conversation
|
Warning Rate limit exceeded@ravishanigarapu has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 23 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 (2)
WalkthroughThese changes introduce utility classes for centralized HTTP request construction and user agent management, refactor JWT token extraction to be static and safer, and update a filter to use a new request wrapper for Authorization headers. Manual header and cookie handling is replaced with reusable utilities, improving consistency and encapsulation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Filter as JwtUserIdValidationFilter
participant RequestWrapper as AuthorizationHeaderRequestWrapper
participant UserAgentCtx as UserAgentContext
participant Service as EmployeeMasterServiceImpl
participant RestUtil as RestTemplateUtil
Client->>Filter: Send HTTP request (with/without JWT)
Filter->>Filter: Check JWT in cookie/header
alt JWT found
Filter->>RequestWrapper: Wrap request with Authorization header
RequestWrapper->>Service: Pass wrapped request
else No JWT, but mobile client and Authorization present
Filter->>UserAgentCtx: Set user agent context
UserAgentCtx->>Service: Pass request with user agent context
Service->>UserAgentCtx: Clear user agent context
else No valid JWT or Authorization
Filter->>Client: Respond 401 Unauthorized
end
Service->>RestUtil: Create HTTP request entity (headers, JWT, user agent)
RestUtil->>Service: Return HttpEntity
Service->>...: Continue processing
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 5
🧹 Nitpick comments (6)
src/main/java/com/iemr/admin/utils/UserAgentContext.java (1)
3-18: Well-designed thread-local implementation for User-Agent contextThis is a clean implementation of the thread-local pattern for storing User-Agent information. The class provides simple methods for setting, getting, and clearing the context, and properly uses ThreadLocal.remove() to prevent memory leaks.
However, consider adding documentation to emphasize that users of this class must ensure
clear()is called after processing, ideally in a try-finally block, to prevent memory leaks with thread pools:public class UserAgentContext { private static final ThreadLocal<String> userAgentHolder = new ThreadLocal<>(); + /** + * Store the user agent for the current thread + * @param userAgent The user agent string to store + */ public static void setUserAgent(String userAgent) { userAgentHolder.set(userAgent); } + /** + * Get the user agent for the current thread + * @return The stored user agent string or null if not set + */ public static String getUserAgent() { return userAgentHolder.get(); } + /** + * Clear the user agent from the current thread. + * This must be called after processing is complete to prevent memory leaks, + * especially in application server environments with thread pools. + */ public static void clear() { userAgentHolder.remove(); } }src/main/java/com/iemr/admin/utils/http/AuthorizationHeaderRequestWrapper.java (3)
10-16: Follow Java naming conventions for member variablesThe member variable
Authorizationshould follow Java's camelCase naming convention for variables.- private final String Authorization; + private final String authorization;Remember to update references to this variable in your methods as well.
18-24: Consider adding null check for authorization valueThe method currently returns the
Authorizationfield even if it's null. Consider adding a null check to avoid returning null values.@Override public String getHeader(String name) { if ("Authorization".equalsIgnoreCase(name)) { - return Authorization; + return Authorization != null ? Authorization : ""; } return super.getHeader(name); }
1-42: Add JavaDoc documentation to improve code maintainabilityConsider adding JavaDoc comments to describe the purpose of this class and its methods. This would help other developers understand how and when to use this wrapper class.
package com.iemr.admin.utils.http; import java.util.Collections; import java.util.Enumeration; import java.util.List; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequestWrapper; +/** + * A wrapper for HttpServletRequest that overrides the Authorization header with a custom value. + * This is useful when you need to modify the Authorization header before passing the request along + * the filter chain. + */ public class AuthorizationHeaderRequestWrapper extends HttpServletRequestWrapper{ private final String Authorization; + /** + * Constructs a request wrapper that will override the Authorization header + * with the provided value. + * + * @param request The original HttpServletRequest + * @param authHeaderValue The value to use for the Authorization header + */ public AuthorizationHeaderRequestWrapper(HttpServletRequest request, String authHeaderValue) { super(request); this.Authorization = authHeaderValue; }src/main/java/com/iemr/admin/utils/RestTemplateUtil.java (2)
16-23: Extract request attribute retrieval logic to improve readabilityConsider extracting the logic to get the request attributes into a separate method to improve code readability and maintainability.
- ServletRequestAttributes servletRequestAttributes = ((ServletRequestAttributes) RequestContextHolder.getRequestAttributes()); - if (servletRequestAttributes == null) { + HttpServletRequest request = getRequestFromContext(); + if (request == null) { MultiValueMap<String, String> headers = new LinkedMultiValueMap<>(); headers.add(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + ";charset=utf-8"); headers.add(HttpHeaders.AUTHORIZATION, authorization); return new HttpEntity<>(body, headers); } - HttpServletRequest requestHeader = servletRequestAttributes.getRequest();Then add a private method:
private static HttpServletRequest getRequestFromContext() { ServletRequestAttributes servletRequestAttributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes(); return servletRequestAttributes != null ? servletRequestAttributes.getRequest() : null; }
13-41: Add JavaDoc documentation for the utility class and methodConsider adding JavaDoc comments to describe the purpose of this utility class and its method to improve code maintainability.
package com.iemr.admin.utils; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.context.request.RequestContextHolder; import org.springframework.web.context.request.ServletRequestAttributes; import jakarta.servlet.http.HttpServletRequest; +/** + * Utility class for creating HTTP request entities with consistent headers. + */ public class RestTemplateUtil { + /** + * Creates an HttpEntity with the provided body and authorization, + * including additional headers from the current request context if available. + * + * @param body The request body object + * @param authorization The authorization header value + * @return An HttpEntity with appropriate headers set + */ public static HttpEntity<Object> createRequestEntity(Object body, String authorization) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/admin/service/employeemaster/EmployeeMasterServiceImpl.java(2 hunks)src/main/java/com/iemr/admin/utils/CookieUtil.java(1 hunks)src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java(2 hunks)src/main/java/com/iemr/admin/utils/RestTemplateUtil.java(1 hunks)src/main/java/com/iemr/admin/utils/UserAgentContext.java(1 hunks)src/main/java/com/iemr/admin/utils/http/AuthorizationHeaderRequestWrapper.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java (2)
src/main/java/com/iemr/admin/utils/http/AuthorizationHeaderRequestWrapper.java (1)
AuthorizationHeaderRequestWrapper(10-42)src/main/java/com/iemr/admin/utils/UserAgentContext.java (1)
UserAgentContext(3-18)
src/main/java/com/iemr/admin/utils/RestTemplateUtil.java (1)
src/main/java/com/iemr/admin/utils/UserAgentContext.java (1)
UserAgentContext(3-18)
src/main/java/com/iemr/admin/service/employeemaster/EmployeeMasterServiceImpl.java (1)
src/main/java/com/iemr/admin/utils/RestTemplateUtil.java (1)
RestTemplateUtil(13-42)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (5)
src/main/java/com/iemr/admin/utils/CookieUtil.java (1)
26-35: Good improvement with null safety and static method conversionThe method has been appropriately converted to static and includes explicit null checking for cookies, which prevents potential
NullPointerExceptionthat could occur when callingArrays.streamon a null array. This is a robust defensive programming practice.src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java (3)
77-79: Improved security by clearing Authorization header with request wrapperUsing
AuthorizationHeaderRequestWrapperto set an empty Authorization header helps prevent potential security issues where both JWT token and Authorization header authentication might conflict or be misused.
82-89: Consistent handling for JWT token from headerThis change applies the same pattern as for cookie-based JWT tokens, providing consistency in how authentication is handled regardless of where the token comes from.
90-101: Good implementation of User-Agent context management for mobile clientsThe implementation correctly sets and clears the
UserAgentContextin a thread-safe manner using a try-finally block. This ensures that the thread-local context is always cleared, even if an exception occurs during filter chain processing.src/main/java/com/iemr/admin/utils/http/AuthorizationHeaderRequestWrapper.java (1)
34-41: LGTM - Header names handling is correctThe implementation correctly adds the "Authorization" header name if it's not already present in the list of header names.
| logger.warn("No valid authentication token found"); | ||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized: Invalid or missing token"); |
There was a problem hiding this comment.
Duplicate error message
There's a duplicate error message at lines 104-105 and 107-108. One of these blocks should be removed to avoid redundant logging and response.
- logger.warn("No valid authentication token found");
- response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized: Invalid or missing token");Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java around
lines 104 to 105, there is a duplicate error message block that repeats again at
lines 107 to 108. Remove one of these duplicate blocks to avoid redundant
logging and sending the same error response twice, ensuring only a single
warning log and error response is sent for invalid or missing authentication
tokens.
| HttpEntity<Object> request = RestTemplateUtil.createRequestEntity(campaignName, authToken); | ||
| String url = configProperties.getPropertyByName("common-url") + configProperties.getPropertyByName("create-feedback"); | ||
| HttpEntity<Object> request1 = new HttpEntity<Object>(campaignName, headers); | ||
| ResponseEntity<String> responseStr = restTemplate.exchange(url, HttpMethod.POST, request1, String.class); | ||
|
|
||
| ResponseEntity<String> responseStr = restTemplate.exchange(url, HttpMethod.POST, request, String.class); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good refactoring using centralized HTTP request entity creation
The refactoring simplifies the code by leveraging the new RestTemplateUtil.createRequestEntity method, which centralizes HTTP request construction and improves consistency by ensuring all necessary headers are included. This change removes duplicate code and makes the implementation more maintainable.
However, the URL being used appears to be for creating feedback but is used in a method for getting CTI campaign roles. Verify that this is the correct endpoint:
🏁 Script executed:
#!/bin/bash
# Verify the URL endpoint being used
echo "Method: getCTICampaignRoles"
echo "URL being used: ${configProperties.getPropertyByName('common-url')} + ${configProperties.getPropertyByName('create-feedback')}"
echo "Is this correct for retrieving campaign roles?"Length of output: 383
🏁 Script executed:
#!/bin/bash
# Show context around the getCTICampaignRoles method
rg -n -B3 -A10 "getCTICampaignRoles" --color=never src/main/java
# Search for the 'create-feedback' key in all .properties files
rg -n "create-feedback" -g "*.properties"
# Search for any 'campaign' keys (to find the roles endpoint) in all .properties files
rg -n "campaign" -g "*.properties"Length of output: 4265
Fix endpoint property key in getCTICampaignRoles
The code is using the undefined "create-feedback" property for fetching campaign roles. Use the "campaign-roles-url" key defined in application.properties instead.
- File: src/main/java/com/iemr/admin/service/employeemaster/EmployeeMasterServiceImpl.java, line 468
- String url = configProperties.getPropertyByName("common-url") + configProperties.getPropertyByName("create-feedback");
+ String url = configProperties.getPropertyByName("common-url") + configProperties.getPropertyByName("campaign-roles-url");📝 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.
| HttpEntity<Object> request = RestTemplateUtil.createRequestEntity(campaignName, authToken); | |
| String url = configProperties.getPropertyByName("common-url") + configProperties.getPropertyByName("create-feedback"); | |
| HttpEntity<Object> request1 = new HttpEntity<Object>(campaignName, headers); | |
| ResponseEntity<String> responseStr = restTemplate.exchange(url, HttpMethod.POST, request1, String.class); | |
| ResponseEntity<String> responseStr = restTemplate.exchange(url, HttpMethod.POST, request, String.class); | |
| HttpEntity<Object> request = RestTemplateUtil.createRequestEntity(campaignName, authToken); | |
| String url = configProperties.getPropertyByName("common-url") + configProperties.getPropertyByName("campaign-roles-url"); | |
| ResponseEntity<String> responseStr = restTemplate.exchange(url, HttpMethod.POST, request, String.class); |
🤖 Prompt for AI Agents
In
src/main/java/com/iemr/admin/service/employeemaster/EmployeeMasterServiceImpl.java
around lines 467 to 470, the URL for the REST call incorrectly uses the
"create-feedback" property key, which is meant for feedback creation, not for
fetching CTI campaign roles. Replace the property key "create-feedback" with the
correct "campaign-roles-url" key from application.properties to ensure the
endpoint matches the method's purpose.
| package com.iemr.admin.utils; | ||
|
|
||
| import org.springframework.http.HttpEntity; | ||
| import org.springframework.http.HttpHeaders; | ||
| import org.springframework.http.MediaType; | ||
| import org.springframework.util.LinkedMultiValueMap; | ||
| import org.springframework.util.MultiValueMap; | ||
| import org.springframework.web.context.request.RequestContextHolder; | ||
| import org.springframework.web.context.request.ServletRequestAttributes; | ||
|
|
||
| import jakarta.servlet.http.HttpServletRequest; | ||
|
|
||
| public class RestTemplateUtil { | ||
| public static HttpEntity<Object> createRequestEntity(Object body, String authorization) { | ||
|
|
||
| ServletRequestAttributes servletRequestAttributes = ((ServletRequestAttributes) RequestContextHolder.getRequestAttributes()); | ||
| if (servletRequestAttributes == null) { | ||
| MultiValueMap<String, String> headers = new LinkedMultiValueMap<>(); | ||
| headers.add(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + ";charset=utf-8"); | ||
| headers.add(HttpHeaders.AUTHORIZATION, authorization); | ||
| return new HttpEntity<>(body, headers); | ||
| } | ||
| HttpServletRequest requestHeader = servletRequestAttributes.getRequest(); | ||
| String jwtTokenFromCookie = null; | ||
| try { | ||
| jwtTokenFromCookie = CookieUtil.getJwtTokenFromCookie(requestHeader); | ||
|
|
||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| } | ||
|
|
||
| MultiValueMap<String, String> headers = new LinkedMultiValueMap<>(); | ||
| headers.add(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + ";charset=utf-8"); | ||
| headers.add(HttpHeaders.USER_AGENT, UserAgentContext.getUserAgent()); | ||
| headers.add(HttpHeaders.AUTHORIZATION, authorization); | ||
| headers.add("JwtToken",requestHeader.getHeader("JwtToken")); | ||
| headers.add(HttpHeaders.COOKIE, "Jwttoken=" + jwtTokenFromCookie); | ||
|
|
||
| return new HttpEntity<>(body, headers); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify JWT token naming consistency
I notice inconsistent naming of the JWT token:
- The method refers to "Jwttoken" in the cookie header
- The header is named "JwtToken"
Ensure this naming is consistent with other parts of the application.
🏁 Script executed:
#!/bin/bash
# Check for other occurrences of JWT token naming in the codebase
echo "Searching for JWT token references in the codebase..."
rg -i "jwt(token|token)" --type javaLength of output: 2642
🏁 Script executed:
#!/bin/bash
# Display CookieUtil.java to inspect token naming
sed -n '1,200p' src/main/java/com/iemr/admin/utils/CookieUtil.javaLength of output: 1082
Unify JWT token naming across headers and cookies
The codebase currently mixes two variants—Jwttoken and JwtToken—when reading from or writing to cookies and headers. This can lead to hard-to-trace bugs. Standardize on a single name (e.g. "JwtToken") by:
- Defining a constant, e.g.
public static final String JWT_TOKEN_KEY = "JwtToken";
in a shared class (JwtConstantsor similar). - Replacing all literal occurrences:
- In
CookieUtil.getJwtTokenFromCookie(...): filter onJWT_TOKEN_KEYinstead of"Jwttoken". - In
RestTemplateUtil.createRequestEntity(...):- Change
headers.add("JwtToken", ...)and
headers.add(HttpHeaders.COOKIE, "Jwttoken=" + ...)
to useJWT_TOKEN_KEYfor both header name and cookie name.
- Change
- In
JwtUserIdValidationFilter.java:- Use
JWT_TOKEN_KEYwhen callingrequest.getHeader(...). - Match the same constant when iterating cookies.
- Use
- In
This will ensure a single source of truth and avoid mismatches.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/admin/utils/RestTemplateUtil.java lines 1 to 42, the
JWT token naming is inconsistent between "Jwttoken" and "JwtToken" in headers
and cookies. To fix this, define a constant JWT_TOKEN_KEY (e.g., "JwtToken") in
a shared class and replace all literal occurrences in this file and related
classes like CookieUtil and JwtUserIdValidationFilter to use this constant for
both header and cookie names, ensuring consistent naming across the codebase.
| MultiValueMap<String, String> headers = new LinkedMultiValueMap<>(); | ||
| headers.add(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + ";charset=utf-8"); | ||
| headers.add(HttpHeaders.USER_AGENT, UserAgentContext.getUserAgent()); | ||
| headers.add(HttpHeaders.AUTHORIZATION, authorization); | ||
| headers.add("JwtToken",requestHeader.getHeader("JwtToken")); | ||
| headers.add(HttpHeaders.COOKIE, "Jwttoken=" + jwtTokenFromCookie); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null checks before adding headers
The code doesn't check if values like UserAgentContext.getUserAgent() or requestHeader.getHeader("JwtToken") are null before adding them to headers. This could lead to unexpected behavior.
MultiValueMap<String, String> headers = new LinkedMultiValueMap<>();
headers.add(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + ";charset=utf-8");
- headers.add(HttpHeaders.USER_AGENT, UserAgentContext.getUserAgent());
+ String userAgent = UserAgentContext.getUserAgent();
+ if (userAgent != null) {
+ headers.add(HttpHeaders.USER_AGENT, userAgent);
+ }
headers.add(HttpHeaders.AUTHORIZATION, authorization);
- headers.add("JwtToken", requestHeader.getHeader("JwtToken"));
+ String jwtToken = requestHeader.getHeader("JwtToken");
+ if (jwtToken != null) {
+ headers.add("JwtToken", jwtToken);
+ }
- headers.add(HttpHeaders.COOKIE, "Jwttoken=" + jwtTokenFromCookie);
+ if (jwtTokenFromCookie != null) {
+ headers.add(HttpHeaders.COOKIE, "Jwttoken=" + jwtTokenFromCookie);
+ }📝 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.
| MultiValueMap<String, String> headers = new LinkedMultiValueMap<>(); | |
| headers.add(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + ";charset=utf-8"); | |
| headers.add(HttpHeaders.USER_AGENT, UserAgentContext.getUserAgent()); | |
| headers.add(HttpHeaders.AUTHORIZATION, authorization); | |
| headers.add("JwtToken",requestHeader.getHeader("JwtToken")); | |
| headers.add(HttpHeaders.COOKIE, "Jwttoken=" + jwtTokenFromCookie); | |
| MultiValueMap<String, String> headers = new LinkedMultiValueMap<>(); | |
| headers.add(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + ";charset=utf-8"); | |
| String userAgent = UserAgentContext.getUserAgent(); | |
| if (userAgent != null) { | |
| headers.add(HttpHeaders.USER_AGENT, userAgent); | |
| } | |
| headers.add(HttpHeaders.AUTHORIZATION, authorization); | |
| String jwtToken = requestHeader.getHeader("JwtToken"); | |
| if (jwtToken != null) { | |
| headers.add("JwtToken", jwtToken); | |
| } | |
| if (jwtTokenFromCookie != null) { | |
| headers.add(HttpHeaders.COOKIE, "Jwttoken=" + jwtTokenFromCookie); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/admin/utils/RestTemplateUtil.java around lines 32 to
38, add null checks before adding headers such as
UserAgentContext.getUserAgent() and requestHeader.getHeader("JwtToken") to the
MultiValueMap. Only add these headers if their values are not null to prevent
potential null values causing unexpected behavior in the headers.
|
📋 Description
JIRA ID: AMM-1456
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor