-
Notifications
You must be signed in to change notification settings - Fork 45
jwtToken and user-agent validation #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes refactor HTTP request entity creation in service classes to use a new utility method, introduce utility classes for managing user agent context and authorization header wrapping, enhance JWT token extraction and validation, and add conditional request handling in an HTTP interceptor to bypass validation when authorization is missing. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Filter as JwtUserIdValidationFilter
participant Wrapper as AuthorizationHeaderRequestWrapper
participant UserAgentCtx as UserAgentContext
participant Chain as FilterChain
Client->>Filter: HTTP Request
alt JWT in Cookie or Header
Filter->>Wrapper: Wrap request with AuthorizationHeaderRequestWrapper
Wrapper->>Chain: doFilter (wrapped request)
else Mobile Client (User-Agent contains "okhttp" and Authorization header present)
Filter->>UserAgentCtx: setUserAgent
Filter->>Chain: doFilter
UserAgentCtx->>UserAgentCtx: clear
else No valid token
Filter-->>Client: 401 Unauthorized response
end
sequenceDiagram
participant Service as ServiceImpl
participant Util as RestTemplateUtil
participant CookieUtil
participant UserAgentCtx as UserAgentContext
participant RestTemplate
Service->>Util: createRequestEntity(data, Authorization)
Util->>CookieUtil: getJwtTokenFromCookie(request)
Util->>UserAgentCtx: getUserAgent()
Util->>RestTemplate: HttpEntity with headers and body
Service->>RestTemplate: exchange HTTP request
Suggested reviewers
Poem
π Recent review detailsConfiguration used: CodeRabbit UI π Files selected for processing (1)
π§ Files skipped from review as they are similar to previous changes (1)
β° Context from checks skipped due to timeout of 90000ms (3)
β¨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
π§Ή Nitpick comments (1)
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (1)
104-104
: Minor formatting fix in catch block.The catch block's opening brace has been moved to the end of the previous line for consistent code style.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (8)
src/main/java/com/iemr/common/service/everwell/EverwellRegistrationServiceImpl.java
(2 hunks)src/main/java/com/iemr/common/service/uptsu/UptsuServiceImpl.java
(2 hunks)src/main/java/com/iemr/common/utils/CookieUtil.java
(1 hunks)src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java
(2 hunks)src/main/java/com/iemr/common/utils/RestTemplateUtil.java
(1 hunks)src/main/java/com/iemr/common/utils/UserAgentContext.java
(1 hunks)src/main/java/com/iemr/common/utils/http/AuthorizationHeaderRequestWrapper.java
(1 hunks)src/main/java/com/iemr/common/utils/http/HTTPRequestInterceptor.java
(1 hunks)
π§° Additional context used
𧬠Code Graph Analysis (4)
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (2)
src/main/java/com/iemr/common/utils/http/AuthorizationHeaderRequestWrapper.java (1)
AuthorizationHeaderRequestWrapper
(10-42)src/main/java/com/iemr/common/utils/UserAgentContext.java (1)
UserAgentContext
(3-18)
src/main/java/com/iemr/common/service/everwell/EverwellRegistrationServiceImpl.java (1)
src/main/java/com/iemr/common/utils/RestTemplateUtil.java (1)
RestTemplateUtil
(13-42)
src/main/java/com/iemr/common/service/uptsu/UptsuServiceImpl.java (1)
src/main/java/com/iemr/common/utils/RestTemplateUtil.java (1)
RestTemplateUtil
(13-42)
src/main/java/com/iemr/common/utils/RestTemplateUtil.java (1)
src/main/java/com/iemr/common/utils/UserAgentContext.java (1)
UserAgentContext
(3-18)
β° Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Build
- GitHub Check: Analyze (java)
π Additional comments (14)
src/main/java/com/iemr/common/utils/UserAgentContext.java (1)
3-18
: Well-designed thread-safe user agent context implementation.This utility class effectively uses ThreadLocal to store user agent information in a thread-safe manner, which is essential for maintaining proper isolation in a multi-threaded environment like a web application. The API is clean and focused with clear method names.
src/main/java/com/iemr/common/utils/http/HTTPRequestInterceptor.java (1)
72-76
:β Verification inconclusive
Ensure this early return doesn't compromise security.
The change allows requests without authorization headers to bypass validation completely. While this may be necessary for certain public endpoints, it's important to verify that all secured endpoints are still properly protected through other mechanisms.
π Script executed:
#!/bin/bash # Search for all controller endpoints to identify which ones might now be accessible without authorization # Find all controller classes echo "Searching for controller classes..." rg -l "@(Rest)?Controller" --type java # Find endpoints that might rely on this interceptor for security echo "Searching for endpoints without explicit security annotations..." rg -l "@RequestMapping|@GetMapping|@PostMapping|@PutMapping|@DeleteMapping" --type java | xargs rg -L "@PreAuthorize|@Secured"Length of output: 3819
Confirm Authorization Coverage in Interceptor
The early return in HTTPRequestInterceptor (src/main/java/com/iemr/common/utils/http/HTTPRequestInterceptor.java:72β76) bypasses validation when the
Authorization
header is missing:if (authorization == null || authorization.isEmpty()) { logger.info("Authorization header is null or empty. Skipping HTTPRequestInterceptor."); return true; // Allow the request to proceed without validation }
Before merging, please verify that:
- All secured endpoints are protected by Spring Security (e.g., via a
SecurityFilterChain
/WebSecurityConfigurerAdapter
or method-level annotations like@PreAuthorize
/@Secured
), not just by this interceptor.- Any public endpoints are explicitly configured to allow anonymous access (for example, with
permitAll()
in your security configuration).If any controller methods depend solely on this interceptor for authentication, they may now be reachable without a valid token. Review your security setup to ensure no protected routes are inadvertently exposed.
src/main/java/com/iemr/common/utils/CookieUtil.java (1)
68-68
: Good refactoring to static method.Making this method static is appropriate since it doesn't rely on instance state, allowing it to be called without instantiating the CookieUtil class. This change supports the new centralized request entity creation in RestTemplateUtil.
src/main/java/com/iemr/common/service/everwell/EverwellRegistrationServiceImpl.java (2)
60-60
: Good addition of RestTemplateUtil import.This import supports the refactoring to use the centralized HTTP entity creation utility.
381-384
: Good refactoring to use centralized HTTP entity creation.Replacing the manual HTTP entity construction with RestTemplateUtil.createRequestEntity improves code maintainability and ensures consistent header handling across service calls, including proper propagation of JWT tokens and user agent information.
src/main/java/com/iemr/common/service/uptsu/UptsuServiceImpl.java (2)
61-61
: Added utility import for HTTP request handling.The import of
RestTemplateUtil
aligns with the refactoring of the HTTP entity creation in therestTemplate
method.
266-269
: Improved HTTP entity creation with centralized utility.The refactoring replaces manual construction of HTTP headers and entities with a call to
RestTemplateUtil.createRequestEntity
. This change improves code maintainability by centralizing header management, including authorization and JWT token propagation.public String restTemplate(String requestOBJ, String url, String Authorization) { - HttpHeaders headers = new HttpHeaders(); - headers.setContentType(MediaType.APPLICATION_JSON); - headers.add("Authorization", Authorization); - HttpEntity<Object> request = new HttpEntity<>(requestOBJ, headers); + HttpEntity<Object> request = RestTemplateUtil.createRequestEntity(requestOBJ, Authorization); RestTemplate restTemplate = new RestTemplate(); return restTemplate.exchange(url, HttpMethod.POST, request, String.class).getBody(); }src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (5)
9-10
: Added import for new request wrapper.The import of
AuthorizationHeaderRequestWrapper
supports the enhanced JWT token handling implemented in this filter.
72-74
: Enhanced JWT validation flow with authorization header management.When a valid JWT token is found in cookies, the request is now wrapped in an
AuthorizationHeaderRequestWrapper
with an empty Authorization header before continuing the filter chain. This approach standardizes authorization handling across the application.
77-84
: Added consistent handling for JWT tokens from headers.Similar to the cookie-based JWT handling, when a valid JWT token is found in headers, the request is wrapped with an empty Authorization header. This ensures consistent authorization behavior regardless of where the JWT token is located.
85-96
: Added mobile client detection and User-Agent context management.The filter now detects mobile clients (identified by "okhttp" in the User-Agent) and manages the User-Agent context for the duration of the request processing. This ensures proper propagation of User-Agent information to downstream services while maintaining thread safety.
99-100
: Improved error handling for unauthorized requests.The filter now explicitly returns a 401 Unauthorized response with a clear error message when no valid authentication token is found, improving API error reporting.
src/main/java/com/iemr/common/utils/http/AuthorizationHeaderRequestWrapper.java (1)
1-42
: New wrapper for controlling Authorization headers.This new class extends
HttpServletRequestWrapper
to provide a clean way to override the Authorization header in HTTP requests. It ensures that:
- When
getHeader("Authorization")
is called, the custom value is returned- When
getHeaders("Authorization")
is called, an enumeration with the custom value is returned- The "Authorization" header name is included in the list of header names
This approach allows transparent modification of the Authorization header without changing the original request, which is a clean design pattern for servlet filters.
src/main/java/com/iemr/common/utils/RestTemplateUtil.java (1)
1-42
: New utility for standardized HTTP request entity creation.This utility class centralizes the creation of HTTP request entities with proper headers for REST template calls. Key benefits:
- Standardizes header creation across the application
- Automatically propagates JWT tokens from the current request
- Includes User-Agent information from the thread-local context
- Provides a fallback for contexts without an active HTTP request
The implementation correctly handles both the presence and absence of a request context, ensuring it can be used in various scenarios.
try { | ||
jwtTokenFromCookie = CookieUtil.getJwtTokenFromCookie(requestHeader); | ||
|
||
} catch (Exception e) { | ||
e.printStackTrace(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π οΈ Refactor suggestion
Improve exception handling for JWT token extraction.
The current implementation catches and prints exceptions when extracting the JWT token from cookies, but continues execution with a null token. Consider using a logging framework instead of printStackTrace()
and handling the null token case more explicitly.
try {
jwtTokenFromCookie = CookieUtil.getJwtTokenFromCookie(requestHeader);
} catch (Exception e) {
- e.printStackTrace();
+ // Use a logger instead of printStackTrace
+ // Logger logger = LoggerFactory.getLogger(RestTemplateUtil.class);
+ // logger.error("Error extracting JWT token from cookie", e);
}
π€ Prompt for AI Agents
In src/main/java/com/iemr/common/utils/RestTemplateUtil.java around lines 25 to
30, replace the use of e.printStackTrace() with proper logging using the
project's logging framework. Additionally, after catching the exception, handle
the case where jwtTokenFromCookie is null explicitly, such as by logging a
warning or throwing a custom exception to prevent continuing execution with a
null token.
|
jwtToken and user-agent validation (PSMRI#196)
π 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
Chores