Release 3.6.0 into main #123
Conversation
story: amm-1668 task - 1754
…tData. (#96) * Update version in pom.xml to 3.4.0 * story: amm-1668 task - 1754 * story: amm-1668 task - 1754 dto updated (#92) * story: amm-1668 task - 1754 dto updated (#93) * story: amm-1668 task - 1754 dto updated * story: amm-1668 task - 1754 * fix: amm-1879 doctor signature was not coming for ncdcare --------- Co-authored-by: Amoghavarsh <93114621+5Amogh@users.noreply.github.com> Co-authored-by: 5Amogh <amoghavarsh@navadhiti.com>
fix: aam-1896 prescribed quantity was not coming in the casesheet
3.4.0 to 3.4.1
fix: amm-1919 fix for update doctor data for higher refferal data
Fix the WASA Issue : IDOR Vulnerability
* fix: add @PreAuthorize to RBAC * fix: wasa RBAC implementation * fix: remove duplicate dependency * fix: coderabbit comments * fix: update role * fix: enable the request matcher
Sn/3.5.1
fix: amm-1963 frequncy is not calculating for single dose frequency
Sn/3.6.0
fix role issue in register api
attach logger
Debug/remove user agent validation
revert user agent
update RestTemplateUtil
add constant class
Implement Role-Based Access Control with JWT and Auth Integration
add role in register api
📝 WalkthroughWalkthroughThis PR implements comprehensive Spring Security integration across the codebase, adding role-based access control via Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Filter as RoleAuthenticationFilter
participant JWT as JwtUtil
participant Redis as RedisStorage
participant UserService as UserLoginRepo
participant Auth as SecurityContext
participant Controller as `@PreAuthorize`<br/>Protected Endpoint
participant Service as Business Logic
Client->>Filter: HTTP Request + JWT/Auth Token
Filter->>JWT: validateToken(token)
alt JWT Valid
JWT-->>Filter: userId extracted
else JWT Invalid
Filter->>Redis: getSession(authToken)
alt Redis Session Exists
Redis-->>Filter: userID from session JSON
else No Session
Filter->>Auth: clear SecurityContext
Filter->>Service: continue (unauthenticated)
Service-->>Client: 401 Unauthorized
Note over Client,Service: Request blocked
end
end
Filter->>Redis: getUserRoleFromCache(userId)
alt Cache Hit
Redis-->>Filter: [roles]
else Cache Miss
Filter->>UserService: getRoleNamebyUserId(userId)
UserService-->>Filter: [roles]
Filter->>Redis: cacheUserRoles(userId, roles)
end
Filter->>Auth: set UsernamePasswordAuthenticationToken<br/>with GrantedAuthorities
Auth-->>Filter: authenticated
Filter->>Controller: proceed with Authentication
Controller->>Controller: `@PreAuthorize`<br/>check roles
alt Role Match
Controller->>Service: invoke business logic
Service-->>Controller: response
Controller-->>Client: 200 OK + data
else Role Mismatch
Controller-->>Client: 403 Forbidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.3)src/main/java/com/iemr/tm/service/common/transaction/CommonNurseServiceImpl.javaThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| } | ||
| JSONObject obj = new JSONObject(comingRequest); | ||
| logger.info("getUserVanSpDetails request {}", comingRequest); |
Check notice
Code scanning / SonarCloud
Logging should not be vulnerable to injection attacks Low
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/iemr/tm/service/common/transaction/CommonNurseServiceImpl.java (1)
2883-3058: Fix logging typo and double-space inconsistency in frequency literalsTwo issues to address:
Logging bug (line 2886):
logger.info("Frequecy", frequency);Typo: "Frequecy" should be "Frequency". More importantly, without a
{}placeholder, thefrequencyparameter is silently ignored by SLF4J. Fix to:logger.info("Frequency: {}", frequency);Frequency string literals (line 3011):
frequency.equalsIgnoreCase("Single Dose Before Food") frequency.equalsIgnoreCase("Single Dose After Food")These contain double spaces before "Food", which will only match if upstream sends the exact same typo. Change to single spaces:
frequency.equalsIgnoreCase("Single Dose Before Food") frequency.equalsIgnoreCase("Single Dose After Food")src/main/java/com/iemr/tm/service/covid19/Covid19ServiceImpl.java (1)
970-987: FixdoctorSignatureFlagextraction insaveDoctorData(currently ignores actual JSON value)Right now:
boolean doctorSignature = requestOBJ.has("doctorSignatureFlag");doctorSignatureFlagis set todoctorSignature, i.e., it becomestruewhenever the field is present, even if the JSON value is explicitlyfalse.- The
.has(...)call happens before theif (requestOBJ != null)guard, so a nullrequestOBJwould NPE earlier than before.This means flow-table updates may incorrectly record a signed case whenever the client merely includes the field.
You can align this with the safer pattern used in
updateCovid19DoctorData:Proposed fix
- Long referSaveSuccessFlag = null; - boolean doctorSignature = requestOBJ.has("doctorSignatureFlag"); + Long referSaveSuccessFlag = null; @@ - Boolean doctorSignatureFlag = false; - if (doctorSignature) { - doctorSignatureFlag = doctorSignature; - } + Boolean doctorSignatureFlag = false; + if (requestOBJ != null + && requestOBJ.has("doctorSignatureFlag") + && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) { + doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); + } @@ - int i = commonDoctorServiceImpl.updateBenFlowtableAfterDocDataSave(commonUtilityClass, isTestPrescribed, - isMedicinePrescribed, tcRequestOBJ, doctorSignatureFlag); + int i = commonDoctorServiceImpl.updateBenFlowtableAfterDocDataSave( + commonUtilityClass, isTestPrescribed, isMedicinePrescribed, tcRequestOBJ, doctorSignatureFlag);Also applies to: 1093-1096
src/main/java/com/iemr/tm/repo/benFlowStatus/BeneficiaryFlowStatusRepo.java (1)
254-263: Same missing space issue in updateBenFlowStatusAfterDoctorActivityTCSpecialist.Line 258 has the same JPQL syntax error - missing space before
WHERE.🐛 Proposed fix
@Query("UPDATE BeneficiaryFlowStatus t set t.pharmacist_flag = :pharmaFlag, " + " t.oncologist_flag = :oncologistFlag, t.processed = 'U', t.specialist_flag = :tcSpecialistFlag, " - + "t.lab_technician_flag = :labTechnicianFlag, t.doctorSignatureFlag = :signatureFlag" + + "t.lab_technician_flag = :labTechnicianFlag, t.doctorSignatureFlag = :signatureFlag " + " WHERE t.benFlowID = :benFlowID AND t.beneficiaryRegID = :benRegID AND t.beneficiaryID = :benID ")
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/tm/utils/http/HttpUtils.java:
- Around line 63-73: The get method builds and populates localheaders
(HttpHeaders localheaders) and calls
RestTemplateUtil.getJwttokenFromHeaders(localheaders) but then constructs the
HttpEntity using the wrong variable (headers), so the JWT never gets sent;
change the HttpEntity construction in get(String uri) to use the populated
localheaders (i.e., new HttpEntity<String>(localheaders)) so the token is
included, and remove or reconcile the unused headers variable if necessary to
avoid confusion.
In @src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java:
- Around line 39-64: OPTIONS preflight requests that pass origin checks still
fall through to JWT validation causing 401s; also the full Authorization header
is being logged. In JwtUserIdValidationFilter, inside the OPTIONS branch after
the isOriginAllowed(origin) check add an immediate successful response and
return (e.g., set status SC_OK and return) so valid preflights do not continue
to JWT logic or call chain.doFilter; and replace any logging of the raw
authorization header (authorizationHeader) with a redacted/masked version (e.g.,
log only the scheme or a fixed placeholder) to avoid leaking tokens.
🟠 Major comments (21)
src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java-131-145 (1)
131-145: Log injection vulnerability and inconsistent error codes.
Log injection (line 132): Logging user-controlled
comingRequestdirectly can enable log injection attacks (e.g., inserting fake log entries or CRLF injection). Sanitize or avoid logging raw request data.Inconsistent error code (line 143): Uses
400here but5000is used ingetUserServicePointVanDetails(line 91) for similar errors. Standardize error codes for consistency.🔒 Suggested fixes
JSONObject obj = new JSONObject(comingRequest); - logger.info("getUserVanSpDetails request {}", comingRequest); + logger.info("getUserVanSpDetails request received for providerServiceMapID: {}", + obj.has("providerServiceMapID") ? obj.getInt("providerServiceMapID") : "missing"); if (obj.has("providerServiceMapID")) { String responseData = iemrMmuLoginServiceImpl.getUserVanSpDetails(userID, obj.getInt("providerServiceMapID")); response.setResponse(responseData); } else { response.setError(400, "Invalid request"); } } catch (Exception e) { - response.setError(400, "Error while getting van and service points data"); + response.setError(5000, "Error while getting van and service points data"); logger.error("getUserVanSpDetails failed", e); }src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java-78-87 (1)
78-87: Dead code and redundant null check.
JSONObject obj(line 81) is created but never used after the refactor.- The null check for
userID(lines 83-86) is redundant—Integer.valueOf()will throwNumberFormatExceptionif parsing fails, but won't return null on success.Consider catching
NumberFormatExceptionif the principal format is uncertain.♻️ Proposed fix
Integer userID = Integer.valueOf(authentication.getPrincipal().toString()); - JSONObject obj = new JSONObject(comingRequest); logger.info("getUserServicePointVanDetails request " + comingRequest); - if (userID == null) { - response.setError(403, "Unauthorized access: Missing or invalid token"); - return response.toString(); - } String responseData = iemrMmuLoginServiceImpl.getUserServicePointVanDetails(userID);src/main/java/com/iemr/tm/controller/common/main/WorklistController.java-727-728 (1)
727-728: Remove unreachable code.The condition
else if(userID == null)is unreachable because ifauthenticationis null or invalid, the method already returns at lines 715-718. After line 720,userIDcannot be null asInteger.valueOf()would throw an exception rather than return null.♻️ Proposed fix
if (providerServiceMapID != null && userID != null ) { String s = commonDoctorServiceImpl.getTCSpecialistWorkListNewForTM(providerServiceMapID, userID, serviceID); if (s != null) response.setResponse(s); - } else if(userID == null ) { - response.setError(403, "Unauthorized access!"); } else { logger.error("Invalid request"); response.setError(5000, "Invalid request");src/main/java/com/iemr/tm/controller/common/main/WorklistController.java-755-755 (1)
755-755: Add exception handling for user ID parsing.Converting the authentication principal to an Integer without validation could throw a
NumberFormatExceptionif the principal contains non-numeric data.🐛 Proposed fix
- Integer userID = Integer.valueOf(authentication.getPrincipal().toString()); + Integer userID; + try { + userID = Integer.valueOf(authentication.getPrincipal().toString()); + } catch (NumberFormatException e) { + logger.error("Invalid user ID in authentication principal: " + e); + response.setError(403, "Unauthorized access"); + return response.toString(); + }src/main/java/com/iemr/tm/controller/common/main/WorklistController.java-710-711 (1)
710-711: Add @PreAuthorize annotation for role-based access control.This endpoint is missing the
@PreAuthorizeannotation, which is inconsistent with the security hardening pattern applied throughout this PR. Based on the method name and similar endpoints (e.g., lines 108, 155, 176), this should be restricted to TC_SPECIALIST or TCSPECIALIST roles.♻️ Proposed fix
@Operation(summary = "Get teleconsultation specialist worklist") + @PreAuthorize("hasRole('TC_SPECIALIST') || hasRole('TCSPECIALIST')") @GetMapping(value = { "/getTCSpecialistWorklist/{providerServiceMapID}/{serviceID}" })Committable suggestion skipped: line range outside the PR's diff.
src/main/java/com/iemr/tm/controller/common/main/WorklistController.java-788-788 (1)
788-788: Add exception handling for user ID parsing.Converting the authentication principal to an Integer without validation could throw a
NumberFormatExceptionif the principal contains non-numeric data.🐛 Proposed fix
- Integer userID = Integer.valueOf(authentication.getPrincipal().toString()); + Integer userID; + try { + userID = Integer.valueOf(authentication.getPrincipal().toString()); + } catch (NumberFormatException e) { + logger.error("Invalid user ID in authentication principal: " + e); + response.setError(403, "Unauthorized access"); + return response.toString(); + }src/main/java/com/iemr/tm/controller/common/main/WorklistController.java-743-744 (1)
743-744: Add @PreAuthorize annotation for role-based access control.This endpoint is missing the
@PreAuthorizeannotation, which is inconsistent with the security hardening pattern applied throughout this PR. This should be restricted to TC_SPECIALIST or TCSPECIALIST roles.♻️ Proposed fix
@Operation(summary = "Get teleconsultation specialist worklist for patient app") + @PreAuthorize("hasRole('TC_SPECIALIST') || hasRole('TCSPECIALIST')") @GetMapping(value = {Committable suggestion skipped: line range outside the PR's diff.
src/main/java/com/iemr/tm/controller/common/main/WorklistController.java-775-776 (1)
775-776: Add @PreAuthorize annotation for role-based access control.This endpoint is missing the
@PreAuthorizeannotation, which is inconsistent with the security hardening pattern applied throughout this PR. This should be restricted to TC_SPECIALIST or TCSPECIALIST roles.♻️ Proposed fix
@Operation(summary = "Get teleconsultation specialist future scheduled") + @PreAuthorize("hasRole('TC_SPECIALIST') || hasRole('TCSPECIALIST')") @GetMapping(value = {Committable suggestion skipped: line range outside the PR's diff.
src/main/java/com/iemr/tm/controller/common/main/WorklistController.java-720-720 (1)
720-720: Add exception handling for user ID parsing.Converting the authentication principal to an Integer without validation could throw a
NumberFormatExceptionif the principal contains non-numeric data.🐛 Proposed fix
- Integer userID = Integer.valueOf(authentication.getPrincipal().toString()); + Integer userID; + try { + userID = Integer.valueOf(authentication.getPrincipal().toString()); + } catch (NumberFormatException e) { + logger.error("Invalid user ID in authentication principal: " + e); + response.setError(403, "Unauthorized access"); + return response.toString(); + }Committable suggestion skipped: line range outside the PR's diff.
src/main/java/com/iemr/tm/controller/foetalmonitor/FoetalMonitorController.java-31-31 (1)
31-31: Add @PreAuthorize annotations to FoetalMonitorController endpoints.The
PreAuthorizeimport was added but no security annotation is applied to the controller or its methods. All endpoints remain unprotected, which is inconsistent with the security controls applied to similar medical controllers (AntenatalCareController, PostnatalCareController, GeneralOPDController, NCDCareController, etc.).Apply method-level or class-level
@PreAuthorizeannotation to restrict access to appropriate roles (e.g.,hasRole('NURSE') || hasRole('DOCTOR')).src/main/java/com/iemr/tm/utils/JwtUtil.java-70-76 (1)
70-76: Document null return behavior and add validation.The method returns
nullwhen the token is invalid or theuserIdclaim is missing, but this isn't documented. Callers must handle null to avoid NPE.📝 Proposed improvement
+/** + * Extracts the userId from the JWT token. + * + * @param token the JWT token + * @return the userId as a String, or null if the token is invalid or userId claim is missing + */ public String getUserIdFromToken(String token) { Claims claims = validateToken(token); if (claims == null) { return null; } return claims.get("userId", String.class); }src/main/java/com/iemr/tm/service/ncdscreening/NCDSCreeningDoctorServiceImpl.java-81-82 (1)
81-82: Logic error: Checking key presence instead of value.The current code checks if the
doctorSignatureFlagkey exists (requestOBJ.has("doctorSignatureFlag")), not whether its value istrue. If the request contains{"doctorSignatureFlag": false},doctorSignaturewill still betrue.Additionally, lines 99-102 are redundant since
doctorSignatureFlagis assigneddoctorSignaturewhich is already a boolean.🐛 Proposed fix
- boolean doctorSignature = requestOBJ.has("doctorSignatureFlag"); - + boolean doctorSignatureFlag = false; + if (requestOBJ.has("doctorSignatureFlag") && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) { + doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); + } if (requestOBJ != null) { // ... existing code ... - Boolean doctorSignatureFlag = false; - if (doctorSignature) { - doctorSignatureFlag = doctorSignature; - } - // checking if test is prescribedAlso applies to: 99-102
src/main/java/com/iemr/tm/utils/exception/CustomAuthenticationEntryPoint.java-19-21 (1)
19-21: Potential JSON injection via unescaped exception message.The exception message is concatenated directly into the JSON string without escaping. If
authException.getMessage()contains quotes or special characters, it will produce malformed JSON or could allow injection.Consider using
ObjectMapperfor consistent and safe JSON serialization, as done inCustomAccessDeniedHandler.🔒 Proposed fix
package com.iemr.tm.utils.exception; import java.io.IOException; +import java.util.Map; +import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.security.core.AuthenticationException; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.stereotype.Component; @@ -12,10 +14,13 @@ @Component public class CustomAuthenticationEntryPoint implements AuthenticationEntryPoint { + private static final ObjectMapper mapper = new ObjectMapper(); + @Override public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException { response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); // 401 response.setContentType("application/json"); - response.getWriter().write("{\"error\": \"Unauthorized\", \"message\": \"" + authException.getMessage() + "\"}"); + Map<String, String> errorResponse = Map.of("error", "Unauthorized", "message", authException.getMessage()); + response.getWriter().write(mapper.writeValueAsString(errorResponse)); } }src/main/java/com/iemr/tm/repo/login/UserLoginRepo.java-18-19 (1)
18-19: Fix method naming convention and add deleted status filtering.
Method name
getRoleNamebyUserIdviolates Java camelCase convention; rename togetRoleNameByUserId.The query does not filter on
deleted = false. The codebase consistently filters deleted records across all repositories; apply the same pattern here to exclude deleted role mappings:Suggested fix
@Query(nativeQuery = true, value = "select rolename from m_role where roleid in (select roleid from m_userservicerolemapping where userid=:userID and deleted=false) and deleted=false") List<String> getRoleNameByUserId(@Param("userID") Long userID);src/main/java/com/iemr/tm/service/anc/ANCServiceImpl.java-351-356 (1)
351-356: Align ANC doctor save flowdoctorSignatureFlagwith JSON value
saveANCDoctorDatacurrently derives the flag as:
boolean doctorSignature = requestOBJ.has("doctorSignatureFlag");doctorSignatureFlagis then set todoctorSignature.So any request that merely includes
"doctorSignatureFlag"will be treated astrue, even if the value isfalse. Also,.has()is called before checkingrequestOBJfor null, which slightly weakens robustness.Since
updateANCDoctorDataalready reads the actual boolean value from JSON, the save path should mirror that behavior:Proposed fix
- Long referSaveSuccessFlag = null; - boolean doctorSignature = requestOBJ.has("doctorSignatureFlag"); - - Boolean doctorSignatureFlag = false; - if (doctorSignature) { - doctorSignatureFlag = doctorSignature; - } + Long referSaveSuccessFlag = null; + + Boolean doctorSignatureFlag = false; + if (requestOBJ != null + && requestOBJ.has("doctorSignatureFlag") + && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) { + doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); + } @@ - int i = commonDoctorServiceImpl.updateBenFlowtableAfterDocDataSave(commonUtilityClass, isTestPrescribed, - isMedicinePrescribed, tcRequestOBJ,doctorSignatureFlag); + int i = commonDoctorServiceImpl.updateBenFlowtableAfterDocDataSave( + commonUtilityClass, isTestPrescribed, isMedicinePrescribed, tcRequestOBJ, doctorSignatureFlag);This keeps the doctor-signature semantics consistent across ANC modules and prevents incorrectly marking encounters as signed.
Also applies to: 473-475
src/main/java/com/iemr/tm/service/cancerScreening/CSServiceImpl.java-813-819 (1)
813-819: CorrectdoctorSignatureFlaghandling in cancer screening doctor save flowIn
saveCancerScreeningDoctorDatathe flag is derived as:
boolean doctorSignature = requestOBJ.has("doctorSignatureFlag");doctorSignatureFlagis set todoctorSignature.This means:
- Any request that includes
"doctorSignatureFlag"in JSON will set the internal flag totrueeven if the payload value isfalse.- The
.has()call is performed before a null check onrequestOBJ, introducing a new NPE risk.Since this flag is persisted via
updateBenFlowAfterDocDataFromSpecialist/updateBenFlowAfterDocData, this can incorrectly mark records as signed.A safer, value-aware version:
Proposed fix
- Long docDataSuccessFlag = null; - Long tcRequestStatusFlag = null; - boolean doctorSignature = requestOBJ.has("doctorSignatureFlag"); - - Boolean doctorSignatureFlag = false; - if (doctorSignature) { - doctorSignatureFlag = doctorSignature; - } + Long docDataSuccessFlag = null; + Long tcRequestStatusFlag = null; + + Boolean doctorSignatureFlag = false; + if (requestOBJ != null + && requestOBJ.has("doctorSignatureFlag") + && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) { + doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); + } @@ - l1 = commonBenStatusFlowServiceImpl.updateBenFlowAfterDocDataFromSpecialist(tmpBenFlowID, - tmpbeneficiaryRegID, tmpBeneficiaryID, tmpBenVisitID, docFlag, pharmaFalg, oncologistFlag, - tcSpecialistFlag, (short) 0, doctorSignatureFlag); + l1 = commonBenStatusFlowServiceImpl.updateBenFlowAfterDocDataFromSpecialist( + tmpBenFlowID, tmpbeneficiaryRegID, tmpBeneficiaryID, tmpBenVisitID, + docFlag, pharmaFalg, oncologistFlag, tcSpecialistFlag, (short) 0, doctorSignatureFlag); @@ - l2 = commonBenStatusFlowServiceImpl.updateBenFlowAfterDocData(tmpBenFlowID, tmpbeneficiaryRegID, - tmpBeneficiaryID, tmpBenVisitID, docFlag, pharmaFalg, oncologistFlag, tcSpecialistFlag, - tcUserID, tcDate, (short) 0, doctorSignatureFlag); + l2 = commonBenStatusFlowServiceImpl.updateBenFlowAfterDocData( + tmpBenFlowID, tmpbeneficiaryRegID, tmpBeneficiaryID, tmpBenVisitID, + docFlag, pharmaFalg, oncologistFlag, tcSpecialistFlag, + tcUserID, tcDate, (short) 0, doctorSignatureFlag);Also applies to: 916-928
src/main/java/com/iemr/tm/service/quickConsultation/QuickConsultationServiceImpl.java-319-320 (1)
319-320: Inconsistent doctorSignatureFlag handling causes incorrect value preservation in save pathThe save method uses field presence to determine the flag:
boolean doctorSignature = quickConsultDoctorOBJ.has("doctorSignatureFlag"); ... Boolean doctorSignatureFlag = false; if (doctorSignature) { doctorSignatureFlag = doctorSignature; }This treats the flag as
truewhenever the field is present in the JSON, regardless of its actual boolean value. The update method correctly reads the actual boolean value:Boolean doctorSignatureFlag = false; if (quickConsultDoctorOBJ.has("doctorSignatureFlag") && !quickConsultDoctorOBJ.get("doctorSignatureFlag").isJsonNull()) { doctorSignatureFlag = quickConsultDoctorOBJ.get("doctorSignatureFlag").getAsBoolean(); }If a client sends
"doctorSignatureFlag": false, the save path will incorrectly set it totruewhile the update path will correctly set it tofalse. This causes inconsistent behavior between the two operations and silently corrupts data.Align the save path logic to match the update path by reading the actual boolean value instead of checking field presence.
src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java-132-137 (1)
132-137: Avoid logging raw Authorization/JWT header valuesLine 134 logs the full authorization header:
logger.info("TM-API_AUTH:" + authHeader);If
authHeadercontains JWTs or other credentials, this leaks secrets into logs, which is a security and compliance risk (log access, backups, etc.).Additionally, the same vulnerability exists in
HTTPRequestInterceptor.java(lines 70 and 127), where the Authorization header is logged unmasked in debug statements.Prefer logging only the fact that the header is present (or at most a short, non‑identifying prefix). Example fix for line 134:
Proposed fix
- logger.info("TM-API_AUTH:"+authHeader); + logger.info("TM-API_AUTH header present for mobile client");Update the debug logging in
HTTPRequestInterceptor.javato mask or omit the Authorization header value.src/main/java/com/iemr/tm/service/generalOPD/GeneralOPDServiceImpl.java-741-742 (1)
741-742: GeneralOPD doctorSignatureFlag: save path ignores actual JSON boolean valueIn
saveDoctorData(lines 741-763):boolean doctorSignature = requestOBJ.has("doctorSignatureFlag"); ... Boolean doctorSignatureFlag = false; if (doctorSignature) { doctorSignatureFlag = doctorSignature; }This uses only the presence of the field;
"doctorSignatureFlag": falsewill propagate astrue. The update method correctly reads the actual value (lines 1370-1373):Boolean doctorSignatureFlag = false; if (requestOBJ.has("doctorSignatureFlag") && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) { doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); }Align the save path with the update path to prevent behavioral discrepancies between create and update flows.
src/main/java/com/iemr/tm/service/ncdscreening/NCDScreeningServiceImpl.java-1011-1011 (1)
1011-1011: doctorSignatureFlag must read the actual JSON boolean value, not field presenceLine 1011 sets
doctorSignaturefromrequestOBJ.has("doctorSignatureFlag"), which only checks field presence. This means any request including the field (even with"doctorSignatureFlag": false) is treated astrue. Lines 1027–1031 then copy this incorrectly-determined boolean, and line 1128 passes it toupdateBenFlowtableAfterDocDataSave, misrepresenting the doctor's signature status.Other modules (QuickConsultation, PNC, NCDCare, GeneralOPD, Covid19) correctly handle this by reading the actual boolean value with
getAsBoolean()after checking both presence and null status. Apply the same pattern here:Proposed fix
- boolean doctorSignature = requestOBJ.has("doctorSignatureFlag"); + // Read the actual boolean value of doctorSignatureFlag from the request @@ - Boolean doctorSignatureFlag = false; - if (doctorSignature) { - doctorSignatureFlag = doctorSignature; - } + Boolean doctorSignatureFlag = false; + if (requestOBJ.has("doctorSignatureFlag") + && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) { + doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); + }Also applies to: 1027–1031, 1128–1129
src/main/java/com/iemr/tm/service/ncdCare/NCDCareServiceImpl.java-759-760 (1)
759-760: NCDCareServiceImpl saveDoctorData: align presence-based check with value-based logic in update pathThe save method checks only for field presence while the update method correctly reads the boolean value. When
"doctorSignatureFlag": falseis sent, the save path treats it astrue(field exists), but the update path correctly interprets it asfalse.In
saveDoctorData(line 759), replace:boolean doctorSignature = requestOBJ.has("doctorSignatureFlag"); // ... if (doctorSignature) { doctorSignatureFlag = doctorSignature; }With:
if (requestOBJ.has("doctorSignatureFlag") && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) { doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); }This matches the update path logic (line 1214–1217) and correctly handles both
trueandfalsevalues.
🟡 Minor comments (2)
src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java-135-135 (1)
135-135: BothTCSPECIALISTandTC_SPECIALISTrole checks are intentional; verify role data consistency in the database.The role transformation logic in
RoleAuthenticationFilternormalizes role names by converting spaces to underscores:"ROLE_" + r.toUpperCase().replace(" ", "_"). This means the system is designed to handle both role variants from the database.The dual checks across six endpoints (lines 135, 157, 179, 201, 223, 245) and other controllers indicate this is intentional to support inconsistent role naming—either from incomplete migration or inconsistent role assignment in the database.
Confirm:
- Whether both role variants actually exist in user role data or if one should be standardized
- If this is legacy support that can be cleaned up
src/main/java/com/iemr/tm/utils/JwtAuthenticationUtil.java-138-146 (1)
138-146: Fix exception handling and typo in error message.
The
catch (Exception e)block catches theIEMRExceptionthrown at line 141, causing it to be re-wrapped with a different message.Typo at line 145:
"Failed to retrieverole for usedId"should be"Failed to retrieve role for userId".🐛 Proposed fix
public List<String> getUserRoles(Long userId) throws IEMRException { if (null == userId || userId <= 0) { throw new IEMRException("Invalid User ID : " + userId); } - try { - List<String> role = userLoginRepo.getRoleNamebyUserId(userId); - if (null == role || role.isEmpty()) { - throw new IEMRException("No role found for userId : " + userId); - } - return role; - } catch (Exception e) { - throw new IEMRException("Failed to retrieverole for usedId : " + userId + " error : " + e.getMessage()); + List<String> role = userLoginRepo.getRoleNamebyUserId(userId); + if (null == role || role.isEmpty()) { + throw new IEMRException("No role found for userId : " + userId); } + return role; }Committable suggestion skipped: line range outside the PR's diff.
🧹 Nitpick comments (25)
src/main/java/com/iemr/tm/controller/location/LocationController.java (1)
45-45: UsehasAnyRole()for improved readability.The current annotation works but is verbose. Spring Security's
hasAnyRole()provides a cleaner alternative. The duplicate role naming (e.g.,LABTECHNICIAN/LAB_TECHNICIAN,TCSPECIALIST/TC_SPECIALIST,DATASYNC/DATA_SYNC) is used consistently throughout the codebase for backward compatibility with existing role assignments and should be preserved.♻️ Suggested refactor using hasAnyRole()
-@PreAuthorize("hasRole('NURSE') || hasRole('PHARMACIST') || hasRole('LABTECHNICIAN') || hasRole('REGISTRAR') || hasRole('DATASYNC') || hasRole('DATA_SYNC') || hasRole('DOCTOR') || hasRole('LAB_TECHNICIAN') || hasRole('TC_SPECIALIST') || hasRole('TCSPECIALIST') || hasRole('ONCOLOGIST') || hasRole('RADIOLOGIST')") +@PreAuthorize("hasAnyRole('NURSE', 'PHARMACIST', 'LABTECHNICIAN', 'REGISTRAR', 'DATASYNC', 'DATA_SYNC', 'DOCTOR', 'LAB_TECHNICIAN', 'TC_SPECIALIST', 'TCSPECIALIST', 'ONCOLOGIST', 'RADIOLOGIST')")src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java (2)
57-59: UnusedJwtUtildependency.
jwtUtilis autowired but never used in this controller. The user ID is extracted directly fromauthentication.getPrincipal().toString(). Remove this unused field to keep the code clean.🧹 Suggested removal
- - @Autowired - private JwtUtil jwtUtil; -
49-50: Standardize role naming conventions.The
@PreAuthorizeannotation includes bothLABTECHNICIANandLAB_TECHNICIAN, as well asDATASYNCandDATA_SYNC. This inconsistency extends across the codebase (e.g.,LocationController.java,RegistrarController.java,WorklistController.java) and also affectsTCSPECIALISTvsTC_SPECIALIST. Standardizing role names across the application will improve maintainability and reduce the risk of access control gaps.src/main/java/com/iemr/tm/controller/common/main/WorklistController.java (1)
62-64: Remove unused JwtUtil dependency.The
jwtUtilfield is autowired but never used in this controller. Based on the relevant code snippets, other controllers (e.g.,IemrMmuLoginController,VideoConsultationController) usejwtUtilto extract user information from JWT tokens, but this controller extracts the userID directly from theAuthenticationobject's principal.♻️ Proposed fix
- @Autowired - private JwtUtil jwtUtil; -src/main/java/com/iemr/tm/utils/http/HTTPRequestInterceptor.java (1)
149-163: Consider caching parsed origin patterns for performance.The method splits and converts
allowedOriginspatterns to regex on every call. Since this runs on each error response, consider pre-compiling and caching the patterns during initialization for better performance.♻️ Suggested optimization with pattern caching
Add a field to cache compiled patterns:
private List<Pattern> compiledOriginPatterns; @PostConstruct public void initOriginPatterns() { if (allowedOrigins != null && !allowedOrigins.trim().isEmpty()) { compiledOriginPatterns = Arrays.stream(allowedOrigins.split(",")) .map(String::trim) .map(pattern -> { String regex = pattern .replace(".", "\\.") .replace("*", ".*") .replace("http://localhost:.*", "http://localhost:\\d+"); return Pattern.compile(regex); }) .collect(Collectors.toList()); } } private boolean isOriginAllowed(String origin) { if (origin == null || compiledOriginPatterns == null || compiledOriginPatterns.isEmpty()) { return false; } return compiledOriginPatterns.stream() .anyMatch(pattern -> pattern.matcher(origin).matches()); }Don't forget to add imports:
import java.util.List; import java.util.regex.Pattern; import java.util.stream.Collectors; import jakarta.annotation.PostConstruct;src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java (1)
65-65: Authorization looks good with minor style note.The role-based access restrictions for NURSE and DOCTOR are appropriate for these medical data operations. Note that there are trailing spaces in the annotation strings (e.g.,
"hasRole('NURSE') || hasRole('DOCTOR') "), which don't affect functionality but could be cleaned up for consistency.Also applies to: 80-80, 95-95, 115-115
src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java (1)
50-50: Consider removing trailing space in the annotation.The
@PreAuthorizeannotation has a trailing space within the string. While this doesn't affect functionality, removing it improves consistency with string literals elsewhere in the codebase.♻️ Proposed fix
-@PreAuthorize("hasRole('LAB_TECHNICIAN') || hasRole('LABTECHNICIAN') ") +@PreAuthorize("hasRole('LAB_TECHNICIAN') || hasRole('LABTECHNICIAN')")src/main/java/com/iemr/tm/controller/nurse/vitals/AnthropometryVitalsController.java (1)
22-22: Consider removing trailing space in the annotation.The
@PreAuthorizeannotation contains a trailing space. While functionally harmless, removing it improves string consistency across the codebase.♻️ Proposed fix
-@PreAuthorize("hasRole('NURSE') ") +@PreAuthorize("hasRole('NURSE')")src/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.java (1)
73-73: Consider removing trailing spaces in annotations.Several
@PreAuthorizeannotations contain trailing spaces within their string values. While this doesn't affect functionality, removing them improves consistency.♻️ Proposed fix
-@PreAuthorize("hasRole('NURSE') ") +@PreAuthorize("hasRole('NURSE')") -@PreAuthorize("hasRole('DOCTOR') ") +@PreAuthorize("hasRole('DOCTOR')") -@PreAuthorize("hasRole('NURSE') || hasRole('DOCTOR') ") +@PreAuthorize("hasRole('NURSE') || hasRole('DOCTOR')")Also applies to: 110-110, 143-143, 175-175, 206-206, 239-239, 266-266, 304-304, 339-339
src/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.java (1)
28-28: Security annotations properly applied to quick consultation endpoints.The role-based access control correctly restricts nurse-only, doctor-only, and shared endpoints with appropriate
@PreAuthorizeannotations.Minor formatting inconsistency: some
@PreAuthorizeannotation strings contain trailing whitespace (particularly double spaces on lines 116, 155, 186, 219, and 246). While this doesn't affect functionality, cleaning these up would improve code consistency.♻️ Optional cleanup example
-@PreAuthorize("hasRole('DOCTOR') ") +@PreAuthorize("hasRole('DOCTOR')")Also applies to: 79-79, 116-116, 155-155, 186-186, 219-219, 246-246
src/main/java/com/iemr/tm/utils/redis/RedisStorage.java (1)
99-121: Role caching helpers are fine; consider null/empty handling and richer loggingThe cache/get helpers look correct for a best-effort cache, but two small improvements would make them more robust:
- Guard against
roles == nullorroles.isEmpty()beforerightPushAllto avoid surprising NPEs from the driver.- When catching
Exception, consider logging the exception object (logger.warn("Failed to cache role for user {}", userId, e);) so stack traces are available during production debugging.src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java (1)
53-75: Authentication check is good; principal→userID assumption is brittleUsing
Authenticationto gate/login/{userID}is a good step, but this line is risky:String userId = authentication.getPrincipal().toString();It assumes the principal’s
toString()is exactly the numericuserID. If your security setup ever returns aUserDetails/custom principal or a non‑numeric identifier, this comparison will silently fail (or break other places if you cast/parse).Consider:
- Using a strongly‑typed principal (e.g., custom
UserPrincipalexposinggetUserId()), or- Using your existing
JwtUtilto extract the user id from the token instead of relying ontoString().src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java (1)
49-52: Auth-based TC worklist is good; principal→Integer conversion is fragileMoving
getTCSpecialistWorkListNewto useAuthenticationinstead of a userId in the body is a solid improvement, and the class-level@PreAuthorize("hasRole('TCSPECIALIST') || hasRole('TC_SPECIALIST') ")matches that intent.One concern:
Integer userID = Integer.valueOf(authentication.getPrincipal().toString());This assumes the principal’s string form is always a numeric user id. If your security config ever switches to a custom principal or a username that isn’t numeric, this will throw
NumberFormatExceptionand drop into the generic error handler.Consider:
- Using a dedicated principal type with an explicit
getUserId(), or- Extracting the user id from the JWT via
JwtUtil/claims instead oftoString().Also, the subsequent
if (userID != null)is redundant, sinceInteger.valueOfnever returns null.Also applies to: 58-60, 148-178
src/main/java/com/iemr/tm/utils/CookieUtil.java (2)
12-13: Consider removing @service annotation.The class now contains only static utility methods. The
@Serviceannotation is unnecessary since no instance is needed for dependency injection. Consider converting to a utility class or removing the annotation.Also applies to: 15-15
34-34: Consider using Constants for cookie names.
Constants.javadefinesJWT_TOKEN = "Jwttoken", but this class uses hardcoded strings"Jwttoken"(line 34) and"Authorization"(line 44). Consider using the constants for consistency and maintainability.Also applies to: 44-44
src/main/java/com/iemr/tm/utils/Constants.java (1)
3-8: LGTM - Clean constants utility class.Good use of private constructor to prevent instantiation. Consider adding an
AUTHORIZATIONconstant for consistency withCookieUtil.getAuthTokenFromCookie()which uses the hardcoded"Authorization"string.src/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.java (1)
28-28: Role mappings look consistent with authority convention; optional readability tweakThe new
@PreAuthorizeguards align with theROLE_…authority convention fromRoleAuthenticationFilter(e.g.,hasRole('NURSE')vsROLE_NURSE), and the NURSE/DOCTOR combinations match the controller responsibilities. If you want to simplify expressions like"hasRole('NURSE') || hasRole('DOCTOR')"you could switch tohasAnyRole('NURSE','DOCTOR'), but that’s only a readability preference.Also applies to: 72-75, 109-112, 141-147, 174-178, 205-209, 237-241, 268-273, 296-300, 331-335, 366-370, 401-405, 435-439
src/main/java/com/iemr/tm/utils/mapper/RoleAuthenticationFilter.java (1)
29-145: Filter logic is sound; consider minor DRY/clarity tweaksThe authentication flow (JWT → legacy Redis session → role cache/DB →
SecurityContext) looks correct and defensive: on any failure you skip auth and continue the chain while clearing the context on hard errors. Two small, optional improvements:
- Cache the result of
CookieUtil.getJwtTokenFromCookie(request)in a local variable instead of calling it twice.- Consider logging at debug for high-frequency events like “UserId resolved from JWT/Redis” to keep info logs quieter in production.
These are non-blocking and won’t change behavior.
src/main/java/com/iemr/tm/utils/RestTemplateUtil.java (2)
53-63: Consolidate JWT cookie extraction to avoid duplication
extractJwttokenmirrors the try/catch already present increateRequestEntity. To reduce drift over time, consider reusingextractJwttokeninsidecreateRequestEntityas well, so any future change to how the JWT is read/logged is made in one place.
65-83: Clarify responsibilities ofgetJwttokenFromHeadersand avoid surprising header changes
getJwttokenFromHeaderscurrently:
- Adds
Content-TypeandUser-Agent, and- Adds JWT information (Cookie or header).
If callers only expect JWT enrichment, the implicit
Content-Typeaddition could lead to duplicate or conflicting values if they already set it. You might either:
- Rename the method to reflect that it standardizes multiple headers (not just JWT), or
- Restrict it to JWT/User-Agent concerns and leave
Content-Typeto the caller.Also, consider using
headers.set(...)instead ofadd(...)for these singleton headers to avoid duplicate entries.src/main/java/com/iemr/tm/service/pnc/PNCServiceImpl.java (1)
303-304: MakedoctorSignatureFlagextraction null-safe and consistentIn
savePNCDoctorData,boolean doctorSignature = requestOBJ.has("doctorSignatureFlag");is evaluated before theif (requestOBJ != null)guard. While callers likely never pass null, this introduces an unnecessary NPE risk and differs from the safer pattern you already use inupdatePNCDoctorData.You can simplify and harden the code by inlining the presence/value check:
- boolean doctorSignature = requestOBJ.has("doctorSignatureFlag"); @@ - Boolean doctorSignatureFlag = false; - if (doctorSignature) { - doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); - } + Boolean doctorSignatureFlag = false; + if (requestOBJ != null + && requestOBJ.has("doctorSignatureFlag") + && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) { + doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean(); + }The rest of the propagation to
updateBenFlowtableAfterDocDataSave/Updatelooks correct.Also applies to: 318-321, 423-425, 1389-1393, 1501-1503
src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java (1)
159-166: localhost origin wildcard replacement likely never matches due to replacement orderIn
isOriginAllowed:String regex = pattern .replace(".", "\\.") .replace("*", ".*") .replace("http://localhost:.*", "http://localhost:\\d+");Because
.is escaped first, any configured pattern likehttp://localhost:.*becomeshttp://localhost:\.*before the finalreplace("http://localhost:.*", ...)runs, so that last replacement probably never matches. This means any special handling you intended forhttp://localhost:<port>patterns won’t actually be applied.Consider:
- Applying the localhost replacement before escaping dots, or
- Using a more explicit normalization step (e.g., checking
pattern.startsWith("http://localhost:")and replacing just the port part), or- Treating localhost as a special case outside this generic
patterntransformation.Example adjustment
- return Arrays.stream(allowedOrigins.split(",")) - .map(String::trim) - .anyMatch(pattern -> { - String regex = pattern - .replace(".", "\\.") - .replace("*", ".*") - .replace("http://localhost:.*", "http://localhost:\\d+"); + return Arrays.stream(allowedOrigins.split(",")) + .map(String::trim) + .anyMatch(pattern -> { + if (pattern.startsWith("http://localhost:")) { + // Accept any numeric port on localhost + String regex = "http://localhost:\\\\d+"; + return origin.matches(regex); + } + String regex = pattern + .replace(".", "\\.") + .replace("*", ".*"); boolean matched = origin.matches(regex); return matched; });Please confirm the expected format of localhost entries in
allowedOriginsand adjust the normalization to match that contract.src/main/java/com/iemr/tm/utils/mapper/SecurityConfig.java (1)
33-52: Clarify CSRF configuration or remove unused CookieCsrfTokenRepository
CookieCsrfTokenRepositoryis instantiated and configured, but CSRF is then globally disabled:CookieCsrfTokenRepository csrfTokenRepository = new CookieCsrfTokenRepository(); csrfTokenRepository.setCookieHttpOnly(true); csrfTokenRepository.setCookiePath("/"); http .csrf(csrf -> csrf.disable())As written, the repository is never used. That’s harmless but confusing for future readers and might suggest CSRF protection is in place when it is not.
Either wire the repository into the CSRF config (if you want cookie-based CSRF tokens), or drop its creation entirely to reflect that the app is operating in a fully stateless, CSRF-disabled mode:
Example clean-up
- CookieCsrfTokenRepository csrfTokenRepository = new CookieCsrfTokenRepository(); - csrfTokenRepository.setCookieHttpOnly(true); - csrfTokenRepository.setCookiePath("/"); - http - .csrf(csrf -> csrf.disable()) + http + .csrf(csrf -> csrf.disable())Please confirm that all clients rely on stateless token auth only and that CSRF protection is intentionally disabled for non-browser flows.
src/main/java/com/iemr/tm/controller/covid19/CovidController.java (1)
47-47: Unusual use of Lettuce's@Paramannotation.The code imports
io.lettuce.core.dynamic.annotation.Paramwhich is from the Redis Lettuce client, but it's being used on controller method parameters. This annotation has no effect in Spring MVC controllers. If parameter binding is needed, use Spring's@RequestParamor rely on the existing@RequestBodyannotations already in place.♻️ Suggested fix
-import io.lettuce.core.dynamic.annotation.Param;Remove the
@Paramannotations from method parameters likegetBenVisitDetailsFrmNurseCovid19,getBenCovid19HistoryDetails, etc., as they serve no purpose in this context.src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java (1)
48-48: Same issue: Lettuce's@Paramannotation has no effect here.This is the same issue as in
CovidController.java. Theio.lettuce.core.dynamic.annotation.Paramimport is from the Redis Lettuce client and has no effect in Spring MVC controllers.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
pom.xmlsrc/main/java/com/iemr/tm/controller/anc/AntenatalCareController.javasrc/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.javasrc/main/java/com/iemr/tm/controller/common/main/WorklistController.javasrc/main/java/com/iemr/tm/controller/common/master/CommonMasterController.javasrc/main/java/com/iemr/tm/controller/covid19/CovidController.javasrc/main/java/com/iemr/tm/controller/dataSyncActivity/StartSyncActivity.javasrc/main/java/com/iemr/tm/controller/dataSyncLayerCentral/MMUDataSyncVanToServer.javasrc/main/java/com/iemr/tm/controller/foetalmonitor/FoetalMonitorController.javasrc/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.javasrc/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.javasrc/main/java/com/iemr/tm/controller/location/LocationController.javasrc/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.javasrc/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.javasrc/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.javasrc/main/java/com/iemr/tm/controller/nurse/vitals/AnthropometryVitalsController.javasrc/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.javasrc/main/java/com/iemr/tm/controller/pnc/PostnatalCareController.javasrc/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.javasrc/main/java/com/iemr/tm/controller/registrar/main/RegistrarController.javasrc/main/java/com/iemr/tm/controller/report/CRMReportController.javasrc/main/java/com/iemr/tm/controller/snomedct/SnomedController.javasrc/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.javasrc/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.javasrc/main/java/com/iemr/tm/data/benFlowStatus/BeneficiaryFlowStatus.javasrc/main/java/com/iemr/tm/data/ncdcare/NCDCareDiagnosis.javasrc/main/java/com/iemr/tm/repo/benFlowStatus/BeneficiaryFlowStatusRepo.javasrc/main/java/com/iemr/tm/repo/login/UserLoginRepo.javasrc/main/java/com/iemr/tm/repo/nurse/ncdcare/NCDCareDiagnosisRepo.javasrc/main/java/com/iemr/tm/service/anc/ANCServiceImpl.javasrc/main/java/com/iemr/tm/service/benFlowStatus/CommonBenStatusFlowServiceImpl.javasrc/main/java/com/iemr/tm/service/cancerScreening/CSServiceImpl.javasrc/main/java/com/iemr/tm/service/common/transaction/CommonDoctorServiceImpl.javasrc/main/java/com/iemr/tm/service/common/transaction/CommonNurseServiceImpl.javasrc/main/java/com/iemr/tm/service/covid19/Covid19ServiceImpl.javasrc/main/java/com/iemr/tm/service/generalOPD/GeneralOPDServiceImpl.javasrc/main/java/com/iemr/tm/service/ncdCare/NCDCareServiceImpl.javasrc/main/java/com/iemr/tm/service/ncdscreening/NCDSCreeningDoctorServiceImpl.javasrc/main/java/com/iemr/tm/service/ncdscreening/NCDScreeningServiceImpl.javasrc/main/java/com/iemr/tm/service/pnc/PNCServiceImpl.javasrc/main/java/com/iemr/tm/service/quickConsultation/QuickConsultationServiceImpl.javasrc/main/java/com/iemr/tm/utils/Constants.javasrc/main/java/com/iemr/tm/utils/CookieUtil.javasrc/main/java/com/iemr/tm/utils/JwtAuthenticationUtil.javasrc/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.javasrc/main/java/com/iemr/tm/utils/JwtUtil.javasrc/main/java/com/iemr/tm/utils/RestTemplateUtil.javasrc/main/java/com/iemr/tm/utils/exception/CustomAccessDeniedHandler.javasrc/main/java/com/iemr/tm/utils/exception/CustomAuthenticationEntryPoint.javasrc/main/java/com/iemr/tm/utils/http/HTTPRequestInterceptor.javasrc/main/java/com/iemr/tm/utils/http/HttpUtils.javasrc/main/java/com/iemr/tm/utils/mapper/RoleAuthenticationFilter.javasrc/main/java/com/iemr/tm/utils/mapper/SecurityConfig.javasrc/main/java/com/iemr/tm/utils/redis/RedisStorage.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-12-16T11:46:15.606Z
Learnt from: ravishanigarapu
Repo: PSMRI/TM-API PR: 58
File: src/main/java/com/iemr/tm/utils/http/HTTPRequestInterceptor.java:93-97
Timestamp: 2024-12-16T11:46:15.606Z
Learning: In the `HTTPRequestInterceptor` class (`src/main/java/com/iemr/tm/utils/http/HTTPRequestInterceptor.java`), the `validator` is mandatory and always initialized, so null checks are unnecessary.
Applied to files:
src/main/java/com/iemr/tm/utils/http/HTTPRequestInterceptor.java
📚 Learning: 2025-04-03T12:46:54.443Z
Learnt from: srishtigrp78
Repo: PSMRI/TM-API PR: 66
File: src/main/java/com/iemr/tm/service/pnc/PNCServiceImpl.java:200-208
Timestamp: 2025-04-03T12:46:54.443Z
Learning: The JSON field name "pNCDeatils" (instead of "pNCDetails") in PNCServiceImpl is intentionally preserved despite the typo to maintain backward compatibility with existing code.
Applied to files:
src/main/java/com/iemr/tm/service/pnc/PNCServiceImpl.javasrc/main/java/com/iemr/tm/data/ncdcare/NCDCareDiagnosis.javasrc/main/java/com/iemr/tm/repo/nurse/ncdcare/NCDCareDiagnosisRepo.java
📚 Learning: 2025-05-22T09:27:49.422Z
Learnt from: ravishanigarapu
Repo: PSMRI/TM-API PR: 79
File: src/main/java/com/iemr/tm/utils/UserAgentContext.java:3-18
Timestamp: 2025-05-22T09:27:49.422Z
Learning: JwtUserIdValidationFilter.java already includes a try-finally block to properly clear UserAgentContext's ThreadLocal resources after use to prevent memory leaks.
Applied to files:
src/main/java/com/iemr/tm/utils/mapper/RoleAuthenticationFilter.javasrc/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java
🧬 Code graph analysis (4)
src/main/java/com/iemr/tm/utils/http/HttpUtils.java (1)
src/main/java/com/iemr/tm/utils/RestTemplateUtil.java (1)
RestTemplateUtil(15-84)
src/main/java/com/iemr/tm/utils/exception/CustomAuthenticationEntryPoint.java (5)
src/main/java/com/iemr/tm/utils/mapper/RoleAuthenticationFilter.java (1)
Component(29-166)src/main/java/com/iemr/tm/utils/JwtAuthenticationUtil.java (1)
Component(22-148)src/main/java/com/iemr/tm/utils/JwtUtil.java (1)
Component(14-77)src/main/java/com/iemr/tm/utils/exception/CustomAccessDeniedHandler.java (1)
Component(14-28)src/main/java/com/iemr/tm/utils/redis/RedisStorage.java (1)
Component(37-122)
src/main/java/com/iemr/tm/utils/mapper/RoleAuthenticationFilter.java (3)
src/main/java/com/iemr/tm/utils/JwtAuthenticationUtil.java (1)
Component(22-148)src/main/java/com/iemr/tm/utils/JwtUtil.java (1)
Component(14-77)src/main/java/com/iemr/tm/utils/redis/RedisStorage.java (1)
Component(37-122)
src/main/java/com/iemr/tm/controller/common/main/WorklistController.java (1)
src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java (1)
RestController(47-167)
🪛 GitHub Check: SonarCloud
src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java
[notice] 132-132: Logging should not be vulnerable to injection attacks
Change this code to not log user-controlled data.See more on SonarQube Cloud
| public String get(String uri) { | ||
| String body; | ||
| HttpHeaders localheaders = new HttpHeaders(); | ||
| localheaders.add("Content-Type", "application/json"); | ||
| RestTemplateUtil.getJwttokenFromHeaders(localheaders); | ||
| HttpEntity<String> requestEntity = new HttpEntity<String>(headers); | ||
| ResponseEntity<String> responseEntity = rest.exchange(uri, HttpMethod.GET, requestEntity, String.class); | ||
| setStatus((HttpStatus) responseEntity.getStatusCode()); | ||
| body = responseEntity.getBody(); | ||
| return body; | ||
| } |
There was a problem hiding this comment.
Critical: JWT token not used in the request.
Lines 65-67 create and populate localheaders with JWT token information, but line 68 creates the HttpEntity using the original headers object instead. This means the JWT token will not be included in the HTTP request, breaking authentication token propagation.
🔧 Proposed fix
public String get(String uri) {
String body;
HttpHeaders localheaders = new HttpHeaders();
localheaders.add("Content-Type", "application/json");
RestTemplateUtil.getJwttokenFromHeaders(localheaders);
- HttpEntity<String> requestEntity = new HttpEntity<String>(headers);
+ HttpEntity<String> requestEntity = new HttpEntity<String>(localheaders);
ResponseEntity<String> responseEntity = rest.exchange(uri, HttpMethod.GET, requestEntity, String.class);
setStatus((HttpStatus) responseEntity.getStatusCode());
body = responseEntity.getBody();
return body;
}🤖 Prompt for AI Agents
In @src/main/java/com/iemr/tm/utils/http/HttpUtils.java around lines 63 - 73,
The get method builds and populates localheaders (HttpHeaders localheaders) and
calls RestTemplateUtil.getJwttokenFromHeaders(localheaders) but then constructs
the HttpEntity using the wrong variable (headers), so the JWT never gets sent;
change the HttpEntity construction in get(String uri) to use the populated
localheaders (i.e., new HttpEntity<String>(localheaders)) so the token is
included, and remove or reconcile the unused headers variable if necessary to
avoid confusion.
| String origin = request.getHeader("Origin"); | ||
| String method = request.getMethod(); | ||
| String uri = request.getRequestURI(); | ||
|
|
||
| logger.debug("Incoming Origin: {}", origin); | ||
| logger.debug("Allowed Origins Configured: {}", allowedOrigins); | ||
| if ("OPTIONS".equalsIgnoreCase(method)) { | ||
| if (origin == null) { | ||
| logger.warn("BLOCKED - OPTIONS request without Origin header | Method: {} | URI: {}", method, uri); | ||
| response.sendError(HttpServletResponse.SC_FORBIDDEN, "OPTIONS request requires Origin header"); | ||
| return; | ||
| } | ||
| if (!isOriginAllowed(origin)) { | ||
| logger.warn("BLOCKED - Unauthorized Origin | Origin: {} | Method: {} | URI: {}", origin, method, uri); | ||
| response.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed"); | ||
| return; | ||
| } | ||
| } else { | ||
| // For non-OPTIONS requests, validate origin if present | ||
| if (origin != null && !isOriginAllowed(origin)) { | ||
| logger.warn("BLOCKED - Unauthorized Origin | Origin: {} | Method: {} | URI: {}", origin, method, uri); | ||
| response.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed"); | ||
| return; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
OPTIONS preflight requests will fail with 401 due to JWT validation
The origin validation correctly blocks missing or disallowed Origin headers, but for allowed origins and OPTIONS requests, the code does not return early. After the origin check passes (lines 55–57), execution continues to line 67 where chain.doFilter(request, response) is called. After that returns, the JWT validation logic (lines 84–110) runs for all requests, including OPTIONS.
Browser preflight requests have no Authorization header, so they hit line 105 and return 401: "No valid authentication token found". Browsers treat a non–2xx preflight as a network error, blocking all CORS requests to protected endpoints.
Add an early return for valid OPTIONS requests before JWT validation:
Proposed fix
if ("OPTIONS".equalsIgnoreCase(method)) {
if (origin == null) {
logger.warn("BLOCKED - OPTIONS request without Origin header | Method: {} | URI: {}", method, uri);
response.sendError(HttpServletResponse.SC_FORBIDDEN, "OPTIONS request requires Origin header");
return;
}
if (!isOriginAllowed(origin)) {
logger.warn("BLOCKED - Unauthorized Origin | Origin: {} | Method: {} | URI: {}", origin, method, uri);
response.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed");
return;
}
+ // Valid preflight—send CORS headers and return 200
+ response.setHeader("Access-Control-Allow-Origin", origin);
+ response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS");
+ response.setHeader("Access-Control-Allow-Headers", "Authorization, Content-Type, Accept, Jwttoken, serverAuthorization, ServerAuthorization, serverauthorization, Serverauthorization");
+ response.setHeader("Access-Control-Allow-Credentials", "true");
+ response.setHeader("Access-Control-Max-Age", "3600");
+ response.setStatus(HttpServletResponse.SC_OK);
+ return;
} else {
// For non-OPTIONS requests, validate origin if present
if (origin != null && !isOriginAllowed(origin)) {
logger.warn("BLOCKED - Unauthorized Origin | Origin: {} | Method: {} | URI: {}", origin, method, uri);
response.sendError(HttpServletResponse.SC_FORBIDDEN, "Origin not allowed");
return;
}
}Additionally, line 134 logs the full authorization header value, which is a security and compliance risk—redact sensitive tokens from logs.
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java around lines
39 - 64, OPTIONS preflight requests that pass origin checks still fall through
to JWT validation causing 401s; also the full Authorization header is being
logged. In JwtUserIdValidationFilter, inside the OPTIONS branch after the
isOriginAllowed(origin) check add an immediate successful response and return
(e.g., set status SC_OK and return) so valid preflights do not continue to JWT
logic or call chain.doFilter; and replace any logging of the raw authorization
header (authorizationHeader) with a redacted/masked version (e.g., log only the
scheme or a fixed placeholder) to avoid leaking tokens.
Summary by CodeRabbit
Release Notes
New Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.