-
Notifications
You must be signed in to change notification settings - Fork 32
AMM-1456 : JwtToken skiped if it not present from mobile #76
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
Changes from all commits
7e5e941
f91def4
bc54859
77186bd
d2d44c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -65,32 +65,47 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo | |||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| // Retrieve JWT token from cookies | ||||||||||||||||||||||||||||||||||
| String jwtTokenFromCookie = getJwtTokenFromCookies(request); | ||||||||||||||||||||||||||||||||||
| logger.info("JWT token from cookie: "); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // 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"); | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| String jwtFromCookie = getJwtTokenFromCookies(request); | ||||||||||||||||||||||||||||||||||
| String jwtFromHeader = request.getHeader("JwtToken"); | ||||||||||||||||||||||||||||||||||
| String authHeader = request.getHeader("Authorization"); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (jwtFromCookie != null) { | ||||||||||||||||||||||||||||||||||
| logger.info("Validating JWT token from cookie"); | ||||||||||||||||||||||||||||||||||
| if (jwtAuthenticationUtil.validateUserIdAndJwtToken(jwtFromCookie)) { | ||||||||||||||||||||||||||||||||||
| filterChain.doFilter(servletRequest, servletResponse); | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Validate JWT token and userId | ||||||||||||||||||||||||||||||||||
| boolean isValid = jwtAuthenticationUtil.validateUserIdAndJwtToken(jwtToken); | ||||||||||||||||||||||||||||||||||
| if (jwtFromHeader != null) { | ||||||||||||||||||||||||||||||||||
| logger.info("Validating JWT token from header"); | ||||||||||||||||||||||||||||||||||
| if (jwtAuthenticationUtil.validateUserIdAndJwtToken(jwtFromHeader)) { | ||||||||||||||||||||||||||||||||||
| filterChain.doFilter(servletRequest, servletResponse); | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| String userAgent = request.getHeader("User-Agent"); | ||||||||||||||||||||||||||||||||||
| logger.info("User-Agent: " + userAgent); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (isValid) { | ||||||||||||||||||||||||||||||||||
| // If token is valid, allow the request to proceed | ||||||||||||||||||||||||||||||||||
| if (userAgent != null && isMobileClient(userAgent) && authHeader != null) { | ||||||||||||||||||||||||||||||||||
| filterChain.doFilter(servletRequest, servletResponse); | ||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid JWT token"); | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+87
to
93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π‘ Verification agent π§© Analysis chainSecurity concern with mobile client bypass. Allowing mobile clients to bypass JWT validation based solely on User-Agent and presence of an Authorization header creates a security vulnerability. The User-Agent can be easily spoofed. Consider implementing a more robust authentication mechanism for mobile clients. At minimum, verify that the Authorization header contains a valid token: π Script executed: #!/bin/bash
# Check if there's any validation for the Authorization header elsewhere in the codebase
rg -A 3 "Authorization.*header" --type javaLength of output: 45 π Script executed: #!/bin/bash
# Locate where the Authorization header is retrieved
rg -n 'getHeader.*"Authorization"' -A 3 --type java
# Show all occurrences of the local variable authHeader for context
rg -n 'authHeader' -A 5 --type javaLength of output: 4717 Ensure JWT validation for mobile clients, not just header presence The current logic in JwtUserIdValidationFilter.java (around lines 87β93) skips any real token check for mobile UAs: String userAgent = request.getHeader("User-Agent");
if (userAgent != null && isMobileClient(userAgent) && authHeader != null) {
filterChain.doFilter(servletRequest, servletResponse);
return;
}User-Agent values are trivial to spoofβallowing any non-null Authorization header effectively opens a backdoor. You should validate the token in the header just as you do for the cookie: β’ File: src/main/java/com/iemr/admin/utils/JwtUserIdValidationFilter.java Suggested change: - if (userAgent != null && isMobileClient(userAgent) && authHeader != null) {
- filterChain.doFilter(servletRequest, servletResponse);
- return;
- }
+ if (userAgent != null && isMobileClient(userAgent) && authHeader != null) {
+ logger.info("Validating JWT token from Authorization header for mobile client");
+ if (jwtAuthenticationUtil.validateUserIdAndJwtToken(authHeader)) {
+ filterChain.doFilter(servletRequest, servletResponse);
+ return;
+ }
+ logger.warn("Invalid JWT token in Authorization header for mobile client");
+ }This ensures mobile clients canβt bypass authentication by spoofing User-Agent. π€ Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| logger.warn("No valid authentication token found"); | ||||||||||||||||||||||||||||||||||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Unauthorized: Invalid or missing token"); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||||||||||||||||
| logger.error("Authorization error: ", e); | ||||||||||||||||||||||||||||||||||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Authorization error: "); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private boolean isMobileClient(String userAgent) { | ||||||||||||||||||||||||||||||||||
| if (userAgent == null) | ||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||
| userAgent = userAgent.toLowerCase(); | ||||||||||||||||||||||||||||||||||
| return userAgent.contains("okhttp"); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+103
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Mobile client detection is too simplistic. The mobile client detection relies solely on checking for "okhttp" in the User-Agent string. This is fragile and can be easily circumvented. Consider:
private boolean isMobileClient(String userAgent) {
if (userAgent == null)
return false;
userAgent = userAgent.toLowerCase();
- return userAgent.contains("okhttp");
+ // Check for common mobile client indicators
+ return userAgent.contains("okhttp") ||
+ userAgent.contains("android") ||
+ userAgent.contains("iphone") ||
+ userAgent.contains("mobile");
}π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| private String getJwtTokenFromCookies(HttpServletRequest request) { | ||||||||||||||||||||||||||||||||||
| Cookie[] cookies = request.getCookies(); | ||||||||||||||||||||||||||||||||||
| if (cookies != null) { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
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.
Inconsistent JWT token header naming.
There's an inconsistency between the JWT token header name used in initial logging (line 52: "Jwttoken") and the one used for actual validation (line 69: "JwtToken"). This case difference could cause the header check to fail if clients are using the case shown in the logging statement.
Also, consider using constants for header names to maintain consistency throughout the code.
π Committable suggestion
π€ Prompt for AI Agents