Add Security Check to Prevent User Enumeration Vb/enumeration #241
Conversation
WalkthroughThis change updates error handling and logging in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IEMRAdminController
participant IEMRAdminUserServiceImpl
User->>IEMRAdminController: Request (e.g., login, password reset)
IEMRAdminController->>IEMRAdminUserServiceImpl: Service call (authenticate, validate, etc.)
IEMRAdminUserServiceImpl-->>IEMRAdminController: Response (success/failure, with standardized messages)
IEMRAdminController-->>User: Response (generic error message or success)
Note over IEMRAdminController: Logs warnings/errors with details internally
Possibly related PRs
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(20 hunks)src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java(14 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:53:24.380Z
Learning: The user vishwab1 confirmed that their CAPTCHA implementation handles null token checking at the controller level before calling the service, which mitigates the direct security vulnerability of the service returning true for null tokens.
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:21:53.819Z
Learning: The user vishwab1 expects CAPTCHA tokens to be optionally sent in request bodies during implementation, suggesting a gradual rollout approach where clients may or may not include CAPTCHA tokens initially.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:41-45
Timestamp: 2024-11-20T07:23:22.514Z
Learning: In the `Common-API` codebase, exception handling within controller classes like `AbdmFacilityController.java` is managed using the `OutputResponse` class, and it is not required to catch specific exceptions separately. General exception handling is sufficient in this context.
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (7)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:53:24.380Z
Learning: The user vishwab1 confirmed that their CAPTCHA implementation handles null token checking at the controller level before calling the service, which mitigates the direct security vulnerability of the service returning true for null tokens.
Learnt from: vishwab1
PR: PSMRI/Common-API#214
File: src/main/java/com/iemr/common/service/recaptcha/CaptchaValidationService.java:28-31
Timestamp: 2025-06-10T10:21:53.819Z
Learning: The user vishwab1 expects CAPTCHA tokens to be optionally sent in request bodies during implementation, suggesting a gradual rollout approach where clients may or may not include CAPTCHA tokens initially.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:52-66
Timestamp: 2025-02-21T07:43:03.828Z
Learning: In the Common-API project's CookieUtil class, the current implementation of addJwtTokenToCookie using both response.addCookie() and manual Set-Cookie header has been tested and confirmed to work as required for their use case. URL encoding of the JWT token is not necessary.
Learnt from: sandipkarmakar3
PR: PSMRI/Common-API#162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:40-47
Timestamp: 2025-02-21T07:42:36.497Z
Learning: In the Common-API project's CookieUtil class, JWT cookies are configured with SameSite=None to support cross-origin requests, which is required for the project's CORS functionality.
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (2)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Learnt from: indraniBan
PR: PSMRI/Common-API#138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
🧬 Code Graph Analysis (1)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
src/main/java/com/iemr/common/constant/Constants.java (1)
Constants(3-16)
🔇 Additional comments (10)
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (5)
227-227: Excellent security improvement for preventing user enumeration.The standardized error message "Invalid username or password" prevents attackers from determining whether a username exists in the system, which is a critical security enhancement.
266-278: Good logging implementation for security monitoring.The warning logs provide detailed information for security monitoring while maintaining generic user-facing error messages. This strikes the right balance between security and auditability.
621-623: Proper error message generalization for user validation.The generic error message prevents information disclosure about user existence while the warning log provides necessary details for administrators.
1131-1136: Enhanced security question validation with proper logging.The implementation correctly prevents user enumeration by providing generic error messages while logging specific details for security monitoring.
968-971: Simplified error handling for force logout operations.The generic error messages prevent information leakage during force logout operations, maintaining security consistency across the application.
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (5)
256-258: Standardized authentication error responses.The generic "Unauthorized." messages across token validation scenarios prevent information leakage while maintaining consistent API responses. This is an excellent security practice.
Also applies to: 264-266, 272-274
332-337: Improved concurrent session logout error handling.The generic error messages and proper logging prevent user enumeration while providing administrators with necessary diagnostic information.
Also applies to: 345-347
625-631: Enhanced password reset security.The implementation now provides the same generic response regardless of whether the username exists, effectively preventing user enumeration attacks during password reset flows.
Also applies to: 667-672
947-985: Comprehensive force logout implementation with proper JWT handling.The rewritten method properly validates JWT tokens, handles token revocation, and maintains security best practices. The implementation correctly extracts tokens from cookies and adds them to the denylist.
987-997: Well-implemented JWT token extraction utility.The helper method correctly extracts JWT tokens from cookies using the proper constant reference and handles null cases appropriately.
| logger.warn("Authentication failed: no token found in header or cookies."); | ||
| throw new IEMRException("Authentication failed. Please log in again."); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify token extraction logic consistency.
The getLoginResponse method extracts JWT tokens from both headers and cookies, but ensure this logic is consistent with the security requirements and doesn't introduce vulnerabilities.
Also applies to: 592-595
🏁 Script executed:
#!/bin/bash
# Description: Check for consistent JWT token extraction patterns across the codebase
# Expected: Consistent token extraction logic without security gaps
echo "Searching for JWT token extraction patterns..."
rg -A 3 -B 3 "getHeader.*[Jj]wt"
echo -e "\nSearching for cookie-based token extraction..."
rg -A 5 -B 2 "getCookies.*JWT|Jwttoken"
echo -e "\nChecking for token validation consistency..."
rg -A 3 "validateToken.*token"Length of output: 13178
🏁 Script executed:
#!/bin/bash
# Extract getLoginResponse method and surrounding lines for context
FILE="src/main/java/com/iemr/common/controller/users/IEMRAdminController.java"
# Find the start line of the method
start=$(rg -n "getLoginResponse" -n $FILE | head -n1 | cut -d: -f1)
# Print 40 lines before and 60 lines after the method signature
sed -n "$((start-40)),$((start+60))p" $FILELength of output: 4171
Unify JWT extraction and strengthen null/blank checks
The code currently mixes header names (“Jwttoken” vs “JwtToken”) and falls back to cookies, which risks missing tokens due to inconsistent casing and null-pointer errors (e.g. calling isEmpty() on a null header). Let’s harmonize on a single constant and ensure safe checks:
• In IEMRAdminController.getLoginResponse (lines ~568–584):
– Replace request.getHeader("Jwttoken") with request.getHeader(Constants.JWT_TOKEN)
– Change if (authHeader.isEmpty()) to if (authHeader == null || authHeader.isBlank())
• In JwtUserIdValidationFilter.java:
– Use Constants.JWT_TOKEN for header lookups in both places (request.getHeader("Jwttoken") and request.getHeader("JwtToken"))
– Leverage your existing getJwtTokenFromCookies(request) helper instead of manual loops
• In RestTemplateUtil.java and CookieUtil.java:
– Reference Constants.JWT_TOKEN rather than hard-coding “JwtToken” or “Jwttoken”
This ensures consistent token names, avoids NPEs, and makes future updates (e.g. renaming the header) a single-line change.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
around lines 568 to 584, unify JWT token header usage by replacing all hardcoded
header names like "Jwttoken" or "JwtToken" with the constant
Constants.JWT_TOKEN. Update null or empty checks on the header to safely check
for null or blank values using (authHeader == null || authHeader.isBlank())
instead of isEmpty() alone. Similarly, in JwtUserIdValidationFilter.java,
replace all header lookups with Constants.JWT_TOKEN and use the existing
getJwtTokenFromCookies(request) helper method instead of manual cookie
iteration. Also, in RestTemplateUtil.java and CookieUtil.java, replace all
hardcoded JWT token header strings with Constants.JWT_TOKEN to ensure consistent
naming and prevent null pointer exceptions.
📋 Description
JIRA ID: 1249
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
Style
Documentation