AMM-1456: JwtToken skiped if it not present from mobile #82
Conversation
WalkthroughThe JWT validation filter was refactored to sequentially validate tokens from cookies and headers, rather than choosing one source. If both validations fail, the filter checks for a mobile client and conditionally bypasses JWT validation. Logging was improved and a helper method for mobile client detection was introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JwtUserIdValidationFilter
participant Application
Client->>JwtUserIdValidationFilter: Send request (with cookies and/or headers)
JwtUserIdValidationFilter->>JwtUserIdValidationFilter: Validate JWT from cookie
alt Cookie token valid
JwtUserIdValidationFilter->>Application: Proceed with request
else Cookie token invalid
JwtUserIdValidationFilter->>JwtUserIdValidationFilter: Validate JWT from header
alt Header token valid
JwtUserIdValidationFilter->>Application: Proceed with request
else Header token invalid
JwtUserIdValidationFilter->>JwtUserIdValidationFilter: Check if mobile client and Authorization header present
alt Mobile client with Authorization
JwtUserIdValidationFilter->>Application: Proceed with request (bypass JWT validation)
else Not mobile or missing Authorization
JwtUserIdValidationFilter-->>Client: Respond 401 Unauthorized
end
end
end
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ 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: 1
🧹 Nitpick comments (2)
src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java (2)
103-108: Mobile client detection contains an inaccurate comment.The method for detecting mobile clients has an incorrect comment - "okhttp" is primarily an Android HTTP client library, not iOS as the comment suggests.
- return userAgent.contains("okhttp"); // iOS (custom clients) + return userAgent.contains("okhttp"); // Android HTTP client library
53-53: Incomplete log statement.This log statement mentions logging the JWT token from header but doesn't actually log the value (which might be intentional for security reasons). Either complete the log statement with a safe representation of the token or clarify the purpose of this log statement.
- logger.info("JWT token from header: "); + logger.info("JWT token from header: " + (jwtTokenFromHeader != null ? "[PRESENT]" : "[NOT PRESENT]"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (5)
src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java (5)
68-71: Organized token retrieval from multiple sources.The approach of extracting JWT tokens from different sources (cookie, header) and the authorization header separately improves code readability and maintainability.
72-78: Sequential validation flow and early return pattern.Good implementation of the sequential validation logic with early returns. This ensures that once a valid token is found from the cookie, the request proceeds immediately without unnecessary checks.
80-86: Consistent validation pattern for header token.The header-based JWT validation follows the same pattern as the cookie validation, maintaining consistency in the code structure.
87-89: Improved logging for debugging.Good addition of user agent logging which will help with debugging authentication issues.
95-97: Improved error messaging for authentication failures.The error message now clearly indicates the reason for the 401 response, which improves debugging and user experience.
| if (userAgent != null && isMobileClient(userAgent) && authHeader != null) { | ||
| filterChain.doFilter(servletRequest, servletResponse); | ||
| } else { | ||
| response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid JWT token"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security concern: Authentication bypass for mobile clients.
The code allows mobile clients with an Authorization header to bypass JWT validation entirely. While this matches the PR objective, it introduces a potential security risk, especially since User-Agent headers can be spoofed.
Please verify:
- Is the Authorization header properly validated elsewhere in the request pipeline?
- Is there a more robust way to identify legitimate mobile clients beyond the User-Agent?
- What is the security impact of this bypass?
🏁 Script executed:
#!/bin/bash
# Check if Authorization header is validated elsewhere
echo "Searching for Authorization header validation in the codebase..."
rg -A 5 "getHeader\(\"Authorization\"\)" --glob "*.java"Length of output: 2256
Authentication bypass for mobile clients must be addressed
The filter’s if (userAgent != null && isMobileClient(userAgent) && authHeader != null) block lets any request with a spoofable User-Agent and a non-empty Authorization header skip JWT validation entirely. We did not find any other point in the pipeline that verifies the header’s token.
• File to fix:
- src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java (around lines 90–93)
• Recommendations:
- Remove or tighten this bypass. Instead of simply checking
authHeader != null, parse and validate the JWT viajwtAuthenticationUtil.validateUserIdAndJwtToken(...). - If you must exempt certain mobile clients, identify them with a stronger mechanism—e.g., API-key or client certificate, device fingerprinting, or a dedicated mobile OAuth flow (PKCE)—not by User-Agent.
- Assess the impact: as written, any attacker can spoof the User-Agent header and supply a dummy Authorization value to gain unauthorized access.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java around lines
90 to 93, the current code bypasses JWT validation for mobile clients based
solely on the User-Agent and presence of an Authorization header, which is
insecure due to header spoofing. To fix this, remove this bypass or replace the
condition to parse and validate the JWT token using
jwtAuthenticationUtil.validateUserIdAndJwtToken(...) before allowing the request
to proceed. If any exemption for mobile clients is necessary, implement a
stronger client verification method such as API keys, client certificates, or a
dedicated OAuth flow instead of relying on User-Agent. This ensures proper
authentication and prevents unauthorized access.
2 participants
📋 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
Bug Fixes
Other Improvements