Skip to content

Conversation

@5Amogh
Copy link
Member

@5Amogh 5Amogh commented Jan 7, 2026

Summary by CodeRabbit

Release Notes - Version 3.6.0

  • New Features

    • Implemented role-based access control and JWT authentication across the application
    • Enhanced prescription responses to include prescribed drug identifiers
    • Added doctor signature tracking capability for clinical workflows
    • Improved data synchronization with detailed status reporting and error handling
  • Enhancements

    • Extended lab testing functionality with result entry improvements
    • Strengthened security with request origin validation and CORS configuration updates

✏️ Tip: You can customize this high-level summary in your review settings.

vanitha1822 and others added 30 commits July 16, 2025 15:45
…ning (#96)

* fix: change the return type to object to get the details

* fix: remove commented lines
* fix: Data Sync batch processing for large data

* fix: use parameterized query

* fix: revert the updated query

* fix: add token if it is missing while calling restTemplate

* fix: update the properties

* fix: sync group wise

* fix: enable logger in pom.xml

* fix: coderabbit comments

* fix: remove logger and replace the license

* fix: remove the logs

* fix: resolve code scanning alert

* fix: resolve code scanning alert

* fix: resolve code scanning alert

* fix: resolve code scanning alert

* fix: add comment for code violation

* fix: use syncuploaddataDigester class to load the deatils

* fix: add syncuploaddigestor in implementation file too

* fix: sonarcube comments
…f the beneficiary (#102)

* fix: amm-1754 changing the query to get the expected response similar to hwc

* fix: amm-1754 compilation error fix

* fix: amm-1754 argument issue fix

* fix: amm-1754 argument issue fix

* fix: amm-1754 argument issue fix
* fix: add file path in cancer gynecological examination

* fix: save the files uploaded from the doctor portal

* fix: get file names in the response of gynecological examination
* fix: resolve the conflicts

* fix: fix the issue in download masters table
* fix: add the schemas

* fix: remove logger

* fix: revert the old code for repository
* fix: datasync from local to central

* fix: fix the token
* fix: remove condition for i_beneficiarydetails

* fix: add logs

* fix: add logs

* fix: remove db_iemr

* fix: add log for show column names too

* fix: add date-format condition

* fix: change valid column name

* fix: change valid column name

* fix: change valid column name

* fix: change valid column name

* fix: update insert query

* fix: update cleaned column list

* fix: date conversion

* fix: conversion date-time

* fix: add date conversion

* fix: logs added

* fix: new logger

* fix: revert the date condition

* fix: revert insert code

* fix: revert insert code

* fix: date format issue

* fix: logs add

* fix: log for group and group lsit

* fix: clean the code

---------

Co-authored-by: vishwab1 <vishwanath@navadhiti.com>
* fix: update server authorization for bengen

* fix: update server authorization for bengen
* fix: add logs to check the identity-api

* fix: add logs

* fix: add logs
* Fix the issue in retrieving the Casesheet Print Data for Cancer Screening (#96)

* fix: change the return type to object to get the details

* fix: remove commented lines

* fix: add the column for NumberperWeek to store and fetch the data (#94)

* Update version in pom.xml to 3.4.0

* chore: add Lombok @DaTa to BenClinicalObservations (#97)

* fix: add file path in cancer gynecological examination (#98)

* Fix the data sync issue  (#93)

* fix: Data Sync batch processing for large data

* fix: use parameterized query

* fix: revert the updated query

* fix: add token if it is missing while calling restTemplate

* fix: update the properties

* fix: sync group wise

* fix: enable logger in pom.xml

* fix: coderabbit comments

* fix: remove logger and replace the license

* fix: remove the logs

* fix: resolve code scanning alert

* fix: resolve code scanning alert

* fix: resolve code scanning alert

* fix: resolve code scanning alert

* fix: add comment for code violation

* fix: use syncuploaddataDigester class to load the deatils

* fix: add syncuploaddigestor in implementation file too

* fix: sonarcube comments

* fix: add functionality to save the file ID's uploaded from doctor screen (#99)

* story: amm-1668 task - 1754

* story: amm-1754 updated response including father name and phone no of the beneficiary (#102)

* fix: amm-1754 changing the query to get the expected response similar to hwc

* fix: amm-1754 compilation error fix

* fix: amm-1754 argument issue fix

* fix: amm-1754 argument issue fix

* fix: amm-1754 argument issue fix

* Save the files uploaded from Doctor Screen (#100)

* fix: add file path in cancer gynecological examination

* fix: save the files uploaded from the doctor portal

* fix: get file names in the response of gynecological examination

* fix: cherry-pick the commits from develop

* fix: cherry-pick commits from develop

* Fix the Download Masters issue (#103)

* fix: resolve the conflicts

* fix: fix the issue in download masters table

* fix: remove the validation (#105)

* fix: replace the old working code (#106)

* Fix the datasync upload issue (#107)

* fix: add the schemas

* fix: remove logger

* fix: revert the old code for repository

* Fixing the datasync from local to central (#110)

* fix: datasync from local to central

* fix: fix the token

* fix: remove the token for server authorization (#111)

* Fix the datasync Demographics Issue (#112)

* fix: remove condition for i_beneficiarydetails

* fix: add logs

* fix: add logs

* fix: remove db_iemr

* fix: add log for show column names too

* fix: add date-format condition

* fix: change valid column name

* fix: change valid column name

* fix: change valid column name

* fix: change valid column name

* fix: update insert query

* fix: update cleaned column list

* fix: date conversion

* fix: conversion date-time

* fix: add date conversion

* fix: logs added

* fix: new logger

* fix: revert the date condition

* fix: revert insert code

* fix: revert insert code

* fix: date format issue

* fix: logs add

* fix: log for group and group lsit

* fix: clean the code

---------

Co-authored-by: vishwab1 <vishwanath@navadhiti.com>

* Fix the token issue for Ben-gen id generation (#114)

* fix: update server authorization for bengen

* fix: update server authorization for bengen

* fix: replace authorization for local api call (#116)

* fix: add logs (#117)

* Fix the BenGen ID Issue (#118)

* fix: add logs to check the identity-api

* fix: add logs

* fix: add logs

* fix: add logs

* fix: add logs for checking

* fix: update the prepare statement

* fix: add log

* fix: add log

* fix: add logs

* fix: add Sync Result

* fix: add logs

* fix: add syncResults

* fix: add syncResults

* fix: update processed flag

* fix: update the response

* fix: update the exception block

* fix: upload fix

* fix: address duplication issue

* fix: datasync issue

* fix: remove reason and add dataaccess exception

* fix: revert the server exception

* fix: remove unwanted code

* fix: exception message

* fix: fix the error message

* fix: update the version

---------

Co-authored-by: Amoghavarsh <93114621+5Amogh@users.noreply.github.com>
Co-authored-by: 5Amogh <amoghavarsh@navadhiti.com>
Co-authored-by: vishwab1 <vishwanath@navadhiti.com>
* fix: resolve the i_beneficiarymappig table issue

* fix: add logs

* fix: update function

* fix: update server functionality

* fix: update else block

* fix: update the catch block

* fix: corrected the table name

* fix: update the failure flag

* fix: remove unwanted code
* fix: date format issue

* fix: marriage date format

* fix: timeout issue - applicaiton.properties

* fix: timeout issue - applicaiton.properties

* fix: add logs

* fix: add logs

* fix: add logs

* fix: add more logs

* fix: add logs in server

* fix: update syncFacilityID

* fix: Reduce batch size to 10
vanitha1822 and others added 24 commits October 9, 2025 16:00
fix: aam-1896 prescribed quantity was not coming in the casesheet
* fix:casesheet signature

* fix: all visit code save data

* fix: all visit code save data

* fix: print data resposne doctor signature

* fix:removed comments
Send Signature flag for Update Doctor Data
fix: amm-1919 fix for update doctor data for higher refferal data
* fix: wasa-IDOR Vulnerability

* fix: coderabbit comments

* fix: remove userid from payload
fix: amm-1927 http methods shown as options issue fix
* fix: rebase to resolve the conflicts

* fix: coderabbit comments

* Fix the WASA Issue : IDOR Vulnerability (#137)

* fix: wasa-IDOR Vulnerability

* fix: coderabbit comments

* fix: remove userid from payload

* fix: resolve the conflicts

* fix: remove unwanted codes
@coderabbitai
Copy link

coderabbitai bot commented Jan 7, 2026

📝 Walkthrough

Walkthrough

This PR integrates Spring Security into the MMU API, bumping the version to 3.6.0. The changes add role-based access control (RBAC) via @PreAuthorize annotations across 20+ controllers, implement JWT-based user authentication through custom filters and utilities, modify service layer logic to propagate doctor signature flags and prescribed drug IDs, update repositories with new query methods, and introduce new security configuration classes for authentication entry points and access denial handlers.

Changes

Cohort / File(s) Summary
Build Configuration
pom.xml
Version bumped from 3.4.0 to 3.6.0; added spring-boot-starter-security dependency for Spring Security support.
Security Configuration & Utilities
src/main/java/com/iemr/mmu/utils/mapper/SecurityConfig.java, src/main/java/com/iemr/mmu/utils/mapper/RoleAuthenticationFilter.java, src/main/java/com/iemr/mmu/utils/exception/CustomAuthenticationEntryPoint.java, src/main/java/com/iemr/mmu/utils/exception/CustomAccessDeniedHandler.java
New Spring Security configuration class enabling stateless authentication, JWT-based role population filter, and custom exception handlers for 401/403 responses.
JWT & Authentication Utilities
src/main/java/com/iemr/mmu/utils/JwtUtil.java, src/main/java/com/iemr/mmu/utils/JwtAuthenticationUtil.java, src/main/java/com/iemr/mmu/utils/redis/RedisStorage.java
Exposed extractAllClaims, added getUserIdFromToken and getUserRoles methods; added Redis caching for user roles to optimize repeated role lookups.
CORS & Filter Utilities
src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java, src/main/java/com/iemr/mmu/utils/http/HttpInterceptor.java
Enhanced CORS origin validation with whitelist support; reorganized origin checks and CORS header setup; added preflight (OPTIONS) request handling with explicit origin validation.
Controllers — Security Annotations (RBAC)
src/main/java/com/iemr/mmu/controller/anc/ANCController.java, src/main/java/com/iemr/mmu/controller/cancerscreening/CancerScreeningController.java, src/main/java/com/iemr/mmu/controller/generalOPD/GeneralOPDController.java, src/main/java/com/iemr/mmu/controller/covid19/CovidController.java, src/main/java/com/iemr/mmu/controller/ncdCare/NCDCareController.java, src/main/java/com/iemr/mmu/controller/ncdscreening/NCDController.java, src/main/java/com/iemr/mmu/controller/pnc/PostnatalCareController.java, src/main/java/com/iemr/mmu/controller/quickconsult/QuickConsultController.java
Added method-level @PreAuthorize annotations restricting endpoints to NURSE, DOCTOR, or both; enhanced some success responses to include prescribedDrugIDs extracted from savedDrugIDs.
Controllers — Class-Level RBAC
src/main/java/com/iemr/mmu/controller/labtechnician/LabTechnicianController.java, src/main/java/com/iemr/mmu/controller/dataSyncActivity/StartSyncActivity.java, src/main/java/com/iemr/mmu/controller/dataSyncLayerCentral/MMUDataSyncVanToServer.java, src/main/java/com/iemr/mmu/controller/fileSync/FileSyncController.java, src/main/java/com/iemr/mmu/controller/snomedct/SnomedController.java, src/main/java/com/iemr/mmu/controller/nurse/vitals/AnthropometryVitalsController.java
Applied class-level @PreAuthorize to restrict controller access to specific roles (NURSE, DOCTOR, LAB_TECHNICIAN, DATASYNC, etc.).
Controllers — JWT Token Extraction
src/main/java/com/iemr/mmu/controller/common/main/CommonController.java, src/main/java/com/iemr/mmu/controller/location/LocationController.java, src/main/java/com/iemr/mmu/controller/login/IemrMmuLoginController.java, src/main/java/com/iemr/mmu/controller/teleconsultation/TeleConsultationController.java
Modified method signatures to accept HttpServletRequest, extract JWT from cookies, and derive userID for downstream use; updated TC specialist endpoints to remove path-level userID parameter; added 403 error responses for missing/invalid tokens.
Controllers — Registrar & Reports
src/main/java/com/iemr/mmu/controller/registrar/main/RegistrarController.java, src/main/java/com/iemr/mmu/controller/reports/ReportGateway.java, src/main/java//com/iemr/mmu/controller/common/master/CommonMasterController.java, src/main/java/com/iemr/mmu/controller/common/main/InsertCommonController.java
Added role-based access control via @PreAuthorize at method or class level for registrar workflows, reporting, and master data endpoints.
Data Models
src/main/java/com/iemr/mmu/data/benFlowStatus/BeneficiaryFlowStatus.java, src/main/java/com/iemr/mmu/data/ncdcare/NCDCareDiagnosis.java
Added doctorSignatureFlag field with JPA mappings to BeneficiaryFlowStatus; extended NCDCareDiagnosis constructor to include createdBy and createdDate parameters.
Repositories — New Methods
src/main/java/com/iemr/mmu/repo/benFlowStatus/BeneficiaryFlowStatusRepo.java, src/main/java/com/iemr/mmu/repo/labModule/LabResultEntryRepo.java, src/main/java/com/iemr/mmu/repo/nurse/BenVisitDetailRepo.java, src/main/java/com/iemr/mmu/repo/nurse/ncdcare/NCDCareDiagnosisRepo.java, src/main/java/com/iemr/mmu/repo/quickConsultation/BenChiefComplaintRepo.java, src/main/java/com/iemr/mmu/repo/quickConsultation/LabTestOrderDetailRepo.java, src/main/java/com/iemr/mmu/repo/quickConsultation/PrescriptionDetailRepo.java
Added transactional updateVanSerialNo methods and extended SELECT projections to include new fields (doctorSignatureFlag, createdBy, createdDate); new repository method getRoleNamebyUserId in UserLoginRepo.
Services — Doctor Signature & Drug IDs
src/main/java/com/iemr/mmu/service/anc/ANCServiceImpl.java, src/main/java/com/iemr/mmu/service/cancerScreening/CSServiceImpl.java, src/main/java/com/iemr/mmu/service/generalOPD/GeneralOPDServiceImpl.java, src/main/java/com/iemr/mmu/service/covid19/Covid19ServiceImpl.java, src/main/java/com/iemr/mmu/service/ncdCare/NCDCareServiceImpl.java, src/main/java/com/iemr/mmu/service/ncdscreening/NCDSCreeningDoctorServiceImpl.java, src/main/java/com/iemr/mmu/service/pnc/PNCServiceImpl.java, src/main/java/com/iemr/mmu/service/quickConsultation/QuickConsultationServiceImpl.java
Modified doctor data save/update flows to extract doctorSignatureFlag and propagate it through beneficiary flow updates; changed saveBenPrescribedDrugsList to return Map<String, Object> containing count and prescribedDrugIDs, then store IDs back into request as savedDrugIDs.
Services — Beneficiary Flow Status
src/main/java/com/iemr/mmu/service/benFlowStatus/CommonBenStatusFlowServiceImpl.java, src/main/java/com/iemr/mmu/service/common/transaction/CommonDoctorServiceImpl.java
Added signatureFlag parameter to multiple update methods; updated repository calls to propagate the new parameter; refined pharma flag handling via getPharmaFlag logic.
Services — Common & Data Sync
src/main/java/com/iemr/mmu/service/common/transaction/CommonNurseServiceImpl.java, src/main/java/com/iemr/mmu/service/anc/ANCNurseServiceImpl.java, src/main/java/com/iemr/mmu/service/labtechnician/LabTechnicianServiceImpl.java
Refactored saveBenPrescribedDrugsList return type; added updateVanSerialNo calls post-persistence; implemented/expanded ANC nurse data persistence and lab result entry logic; added getLast_3_ArchivedTestVisitList endpoint.
Data Sync Services
src/main/java/com/iemr/mmu/service/dataSyncActivity/DataSyncRepository.java, src/main/java/com/iemr/mmu/service/dataSyncActivity/DownloadDataFromServerImpl.java, src/main/java/com/iemr/mmu/service/dataSyncActivity/DownloadDataFromServerTransactionalImpl.java, src/main/java/com/iemr/mmu/service/dataSyncActivity/SyncResult.java, src/main/java/com/iemr/mmu/service/dataSyncActivity/UploadDataToServerImpl.java
Updated updateProcessedFlagInVan signature to accept List<String> and added status/reason parameters; refactored syncDataToServer to return structured Map with status/counts; introduced SyncResult DTO for granular sync tracking; extended logging and error handling.
Central Data Sync
src/main/java/com/iemr/mmu/service/dataSyncLayerCentral/DataSyncRepositoryCentral.java, src/main/java/com/iemr/mmu/service/dataSyncLayerCentral/GetDataFromVanAndSyncToDBImpl.java
Corrected table name from t_benchefcomplaint to t_benchiefcomplaint; introduced SyncResult collection tracking; refactored sync flows with per-table/per-record result aggregation and enhanced error reason extraction.
Service — Cancer Screening
src/main/java/com/iemr/mmu/service/cancerScreening/CSDoctorServiceImpl.java
Added vanSerialNo population post-persistence for CancerDiagnosis entities.
Service — NCD Screening
src/main/java/com/iemr/mmu/service/ncdscreening/NCDScreeningServiceImpl.java
Extended doctor data save to extract fileIDs and update visit details; refactored drug save to use new Map-based return type; propagated doctorSignatureFlag to flow updates.
Configuration & Properties
src/main/resources/application.properties
Updated timeout configurations: adjusted remove-abandoned-timeout and max-wait; added new async/REST/server timeout settings.
Test Updates
src/test/java/com/iemr/mmu/service/anc/ANCServiceImplTest.java, src/test/java/com/iemr/mmu/service/benFlowStatus/CommonBenStatusFlowServiceImplTest.java, src/test/java/com/iemr/mmu/service/generalOPD/GeneralOPDServiceImplTest.java, src/test/java/com/iemr/mmu/service/quickConsultation/QuickConsultationServiceImplTest.java
Updated mock stubs to reflect new method signatures (added signatureFlag parameter) and return types (Map<String, Object> for drug saves); added throws Exception to test method signatures.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Filter as RoleAuthenticationFilter
    participant JwtUtil
    participant RedisStorage
    participant JwtAuthUtil
    participant SecurityContext
    participant Controller

    Client->>Filter: HTTP Request with JWT in cookie/header
    Filter->>JwtUtil: Extract JWT & validate
    JwtUtil-->>Filter: Claims (valid or null)
    
    alt JWT Invalid
        Filter->>SecurityContext: Clear context
        Filter->>Controller: Continue (no auth)
    else JWT Valid
        Filter->>Filter: Extract userId from claims
        Filter->>RedisStorage: Get cached user roles
        
        alt Roles in Cache
            RedisStorage-->>Filter: List<String> roles
        else Roles Not Cached
            Filter->>JwtAuthUtil: Fetch roles from DB
            JwtAuthUtil-->>Filter: List<String> roles
            Filter->>RedisStorage: Cache roles
        end
        
        Filter->>Filter: Normalize roles to ROLE_*
        Filter->>SecurityContext: Set UsernamePasswordAuthenticationToken<br/>(userId, null, authorities)
        Filter->>Controller: Continue (authenticated & authorized)
        
        Controller->>Controller: `@PreAuthorize` checks roles
        alt Role Match
            Controller-->>Client: Success (200/201)
        else Role Mismatch
            Controller-->>Client: Forbidden (403)
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • drtechie

Poem

🐰 A hop and a skip through authentication's gate,
Spring Security standing proud and great,
JWT tokens dance in the Redis cache,
Roles bloom like carrots, in a RBAC bash!
Doctor signatures flag, and drug IDs cascade,
The MMU API's security upgrade is made! 🔐✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.36% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Release 3.6.0 into main' directly and clearly summarizes the primary objective of the pull request: releasing version 3.6.0 into the main branch. This matches the version bump in pom.xml and the comprehensive changes throughout the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 7, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
27.8% Duplication on New Code (required ≤ 3%)
B Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@5Amogh 5Amogh requested a review from drtechie January 7, 2026 08:31
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

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 (6)
src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java (1)

174-183: Bug: String.replace() performs literal replacement, not regex transformation.

Line 180 uses String.replace() which performs literal string replacement. The code .replace("http://localhost:.*", "http://localhost:\\d+") will only match if the pattern literally contains "http://localhost:.*", which is unlikely. This appears to be an attempt to handle localhost port matching but won't work as intended.

🐛 Proposed fix

If the intent is to allow http://localhost:* in configuration to match any port:

 		return Arrays.stream(allowedOrigins.split(","))
 				.map(String::trim)
 				.anyMatch(pattern -> {
-					String regex = pattern
-							.replace(".", "\\.")
-							.replace("*", ".*")
-						    .replace("http://localhost:.*", "http://localhost:\\d+");
+					String regex = pattern
+							.replace(".", "\\.")
+							.replace("*", ".*");
+					// Handle localhost port matching: http://localhost:* should match any port
+					if (regex.equals("http://localhost:.*")) {
+						regex = "http://localhost:\\d+";
+					}
 					boolean matched = origin.matches(regex);
 					return matched;
 				});
src/main/java/com/iemr/mmu/service/common/transaction/CommonNurseServiceImpl.java (1)

2785-2817: Clarify count semantics and verify ID accessor in saveBenPrescribedDrugsList

The new Map<String, Object> return with "count" and "prescribedDrugIDs" is useful, but two points to verify:

  • count is set to prescribedDrugDetailListRS.size() on non-empty input, but to 1 when the input list is empty. For a field named count, returning 1 when 0 records were persisted is counterintuitive and may confuse callers expecting an actual record count.
  • IDs are collected via savedDrug.getId(). Please confirm that PrescribedDrugDetail exposes its primary key via getId(); if the actual accessor is getID() or getPrescribedDrugID(), this will either not compile or always produce an empty prescribedDrugIDs list.

Consider:

  • Returning the true number of persisted rows (0 when list is empty), or clearly documenting that count is a success flag, not a record count.
  • Adjusting the ID accessor to match the actual entity API if needed.
src/main/java/com/iemr/mmu/service/common/transaction/CommonDoctorServiceImpl.java (2)

441-447: TMReferred flag reset inside loop can misrepresent multi‑service referrals

In both saveBenReferDetails and updateBenReferDetails, TMReferred is updated like this while iterating refrredToAdditionalServiceList:

if (sm.getServiceName().equalsIgnoreCase(tmReferCheckValue)) {
    TMReferred = 1;
} else {
    TMReferred = 0;
}

This means:

  • If a telemedicine service appears earlier in the list but a non‑TM service appears later, the last non‑TM entry will reset TMReferred back to 0, even though the visit was referred to TM at least once.

Given TMReferred drives later logic in updateBenFlowtableAfterDocDataSave/Update (including setting tcSpecialistFlag), this can incorrectly skip TM-related flow handling for multi-service referrals.

Suggested fix: only set TMReferred = 1 when a TM service is encountered and avoid resetting it to 0 in the else branch, e.g.:

if (sm.getServiceName().equalsIgnoreCase(tmReferCheckValue)) {
    TMReferred = 1;
}

and, if needed, reset TMReferred once per request (e.g., at the start of saveBenReferDetails/updateBenReferDetails), not per service in the loop.

Also applies to: 727-747


679-754: Flow‑table updates now carry doctorSignatureFlag; shared TMReferred state is risky

The extended signatures:

  • updateBenFlowtableAfterDocDataSave(..., TeleconsultationRequestOBJ tcRequestOBJ, Boolean doctorSignatureFlag)
  • updateBenFlowtableAfterDocDataUpdate(..., TeleconsultationRequestOBJ tcRequestOBJ, Boolean doctorSignatureFlag)

and the corresponding calls into CommonBenStatusFlowServiceImpl look consistent: all branches (specialist, TM referred, with/without further follow-up) propagate the new flag.

However:

  • Both methods read the instance field TMReferred, which is set in saveBenReferDetails/updateBenReferDetails. Since this service is a singleton Spring bean, TMReferred is effectively shared mutable state across requests. Concurrent requests can interleave and overwrite TMReferred, leading to incorrect tcSpecialistFlag handling for some patients.

If feasible, prefer passing TMReferred (or the inferred “TM referred” boolean) as a parameter based on the current request’s context instead of storing it as a field, or at least limiting its lifetime to a per-request structure.

Also applies to: 766-813, 827-895

src/main/java/com/iemr/mmu/controller/common/main/CommonController.java (1)

691-722: Guard JWT token retrieval before parsing userId

Both getTCSpecialistWorkListNew (lines 699–701) and getTCSpecialistWorklistFutureScheduled (lines 733–735) call getUserIdFromToken(jwtToken) and Integer.parseInt(userId) before validating that jwtToken and userId are non-null. When tokens are missing or invalid, this triggers an exception that falls into the generic 5000 error path instead of returning the intended 403 response.

Immediate fix:

  • Check if (jwtToken == null || jwtToken.isEmpty()) immediately after retrieving it; if true, set 403 and return.
  • Wrap getUserIdFromToken(jwtToken) and Integer.parseInt(userId) in a try-catch that maps token/parse errors to 403 rather than 5000.

Better approach (architectural):
The codebase already has RoleAuthenticationFilter, which properly validates the JWT once, extracts userId safely with null checks and exception handling, and populates SecurityContextHolder with a UsernamePasswordAuthenticationToken containing the userId as principal. Controllers should retrieve the user ID from the already-authenticated context via SecurityContextHolder.getContext().getAuthentication().getPrincipal() (or via @AuthenticationPrincipal) instead of re-reading and re-parsing the JWT in every method. This eliminates duplication, reduces error surface, and aligns with Spring Security best practices.

Also applies to: 725–756

src/main/java/com/iemr/mmu/service/dataSyncLayerCentral/GetDataFromVanAndSyncToDBImpl.java (1)

718-751: SQL injection risk in UPDATE and INSERT query builders — add whitelist validation.

Both getQueryToUpdateDataToServerDB() and getQueryToInsertDataToServerDB() directly concatenate schemaName and tableName without validation. Although DataSyncRepositoryCentral defines VALID_SCHEMAS and VALID_TABLES whitelists, GetDataFromVanAndSyncToDBImpl does not validate these parameters against them before constructing SQL. Additionally, serverColumns are split and trimmed but still concatenated directly, creating column injection risk.

Validate all three parameters against predefined whitelists before concatenating them into the query string, similar to the approach in DataSyncRepositoryCentral.

🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/mmu/controller/anc/ANCController.java:
- Around line 24-26: Remove the duplicate import declarations in
ANCController.java: keep a single set of imports for java.util.ArrayList,
java.util.HashMap, java.util.List, java.util.Map and com.google.gson.Gson and
delete the redundant repeated import block (the duplicates around lines 24-26
and 50-56). Ensure only one import statement exists for each symbol (ArrayList,
HashMap, List, Map, Gson) so the file compiles without duplicate-import errors.

In @src/main/java/com/iemr/mmu/service/anc/ANCServiceImpl.java:
- Around line 1628-1630: In ANCServiceImpl, the null check for variable r is
ordered incorrectly causing a potential NPE; update the condition that sets
prescriptionSuccessFlag so you check r for null first (e.g., change the if that
currently reads using r > 0 && r != null to check r != null before comparing) or
use Objects.nonNull(r) && r > 0, ensuring you reference the same local variable
r and assign to prescriptionSuccessFlag only when r is non-null and greater than
zero.
- Around line 400-402: In ANCServiceImpl the conditional uses `r > 0 && r !=
null`, which can NPE if r is null; change the check to test null first (e.g., `r
!= null && r > 0` or use `Objects.nonNull(r) && r > 0`) before assigning
`prescriptionSuccessFlag = r` so the null check guards the numeric comparison.

In @src/main/java/com/iemr/mmu/service/dataSyncActivity/DataSyncRepository.java:
- Around line 101-121: The updateProcessedFlagInVan method builds SQL by
concatenating vanSerialNos and autoIncreamentColumn causing SQL injection and
has duplicate branches; fix it by validating tableName and autoIncreamentColumn
against an allowlist (e.g., ALLOWED_TABLES and ALLOWED_COLUMNS) then use a
NamedParameterJdbcTemplate (or JdbcTemplate with positional bind markers) to
parameterize the IN clause for vanSerialNos (create a named list parameter like
:ids) and bind status, SyncedDate, user, reason as parameters; collapse the
identical if/else into a single query construction that uses the validated
table/column names and the parameterized IN clause, then execute the update with
the bound parameters.

In
@src/main/java/com/iemr/mmu/service/dataSyncActivity/DownloadDataFromServerTransactionalImpl.java:
- Line 112: Confirm and ensure the central server schema used in
updateProcessedFlagToCentral calls is correct: verify the central DB actually
exposes the "db_iemr" schema (not "db_iemr_sync") and that the processed-flag
update API endpoint accepts/operates on "db_iemr"; update configuration or
revert the schema string in all calls to updateProcessedFlagToCentral (used in
DownloadDataFromServerTransactionalImpl) to match the central server if
necessary, and run end-to-end synchronization tests (including the
downloadDataFromCentral flows referenced in DownloadDataFromServerImpl) to
validate there are no silent failures or data inconsistencies.

In
@src/main/java/com/iemr/mmu/service/labtechnician/LabTechnicianServiceImpl.java:
- Around line 501-517: There is a mismatched/extra closing brace causing a
compilation error around the block that saves lab results; in the method
containing the if (null != labResultsListNew && labResultsListNew.size() > 0) {
... } block remove the stray closing brace so the method's braces align
properly, ensure the return labResultSaveFlag; remains inside the method (not
inside an extra block), and verify the for-loop and outer if/else (which sets
labResultSaveFlag = 1) close correctly; specifically check and fix brace
placement around the labResultEntryRepo.saveAll(...) handling and the subsequent
updateVanSerialNo(result.getID()) loop so the method compiles.

In
@src/main/java/com/iemr/mmu/service/ncdscreening/NCDScreeningServiceImpl.java:
- Around line 1336-1346: The condition in NCDScreeningServiceImpl where you
check Integer r from commonNurseServiceImpl.saveBenPrescribedDrugsList(...) uses
"r > 0 && r != null" which will throw an NPE if r is null; change the check to
test nullity first (e.g., "r != null && r > 0" or use Objects.nonNull(r) && r >
0) before using r, and update any similar checks around the variable or method
saveBenPrescribedDrugsList to follow the same safe-evaluation ordering.
- Around line 271-284: The null-check order for the Integer r is wrong and can
cause a NullPointerException when evaluating "r > 0 && r != null"; change the
condition to check for null first and then compare the value (e.g., "r != null
&& r > 0") before assigning to prescriptionSuccessFlag; update the block around
the saveBenPrescribedDrugsList result handling (variables: drugSaveResult, r,
prescribedDrugIDs, requestOBJ, prescriptionSuccessFlag) to perform the null
check first and only set prescriptionSuccessFlag when r is non-null and
positive.

In @src/main/java/com/iemr/mmu/utils/JwtUtil.java:
- Around line 60-66: extractAllClaims currently exposes a way to parse JWTs
without performing the same integrity and denylist checks done in validateToken;
make extractAllClaims private and update all callers to call validateToken
instead, or if you must keep it public, add the same validation inside: verify
signature with getSigningKey, check expiration and the application's denylist
(the same logic used by validateToken), and throw a security exception for
invalid or denylisted tokens so no caller can bypass validateToken's checks.

In @src/main/java/com/iemr/mmu/utils/mapper/RoleAuthenticationFilter.java:
- Around line 56-57: The RoleAuthenticationFilter calls
filterChain.doFilter(request, response) in multiple early-return branches and
again in the finally block, causing duplicate chain invocation; remove the
duplicate calls by keeping a single invocation point: eliminate the early
filterChain.doFilter(...) + return statements (those inside the branches that
check conditions and currently call filterChain.doFilter) and let the single
call in the finally block handle continuing the chain, or alternatively move the
sole filterChain.doFilter(request, response) out of finally and use early
returns without calling the chain in the finally; ensure only one
filterChain.doFilter invocation remains (referencing
filterChain.doFilter(request, response) and the finally block in
RoleAuthenticationFilter).
🟠 Major comments (14)
src/main/java/com/iemr/mmu/utils/http/HttpInterceptor.java-157-171 (1)

157-171: Fragile localhost pattern logic and security risks need attention.

The isOriginAllowed method has several concerns:

  1. Localhost special-case logic is fragile: The third .replace() only works if the configuration uses http://localhost:* (no dot before asterisk). For http://localhost.*, the pattern would be transformed to http://localhost\\...* before the third replace runs, causing the search for http://localhost:.* to fail. This order-dependency is confusing and error-prone.

  2. Security risk - wildcard misconfiguration: If allowedOrigins is configured as *, it transforms to regex .* which matches ANY origin, completely bypassing CORS security. Consider adding validation to reject or warn about overly permissive patterns in production.

  3. Performance concern: The pattern splitting and regex transformation occur on every request. For high-traffic applications, consider pre-compiling patterns during initialization.

♻️ Suggested improvements

Improvement 1: Add validation against overly permissive wildcards

 private boolean isOriginAllowed(String origin) {
     if (origin == null || allowedOrigins == null || allowedOrigins.trim().isEmpty()) {
         return false;
     }
+    
+    // Warn about overly permissive wildcard configuration
+    if (allowedOrigins.trim().equals("*")) {
+        logger.warn("SECURITY WARNING: CORS allowedOrigins is configured as '*' - this allows ALL origins!");
+    }

     return Arrays.stream(allowedOrigins.split(","))

Improvement 2: Clarify localhost pattern handling with a comment

         .anyMatch(pattern -> {
             String regex = pattern
                     .replace(".", "\\.")
                     .replace("*", ".*")
+                   // Special case: transform http://localhost:* (no dot before *) to match any port number
                     .replace("http://localhost:.*", "http://localhost:\\d+");
             return origin.matches(regex);
         });
 }

Improvement 3: Consider pre-compiling patterns (optional, for performance)

You could initialize compiled patterns in a @PostConstruct method:

private List<Pattern> allowedOriginPatterns;

@PostConstruct
public void initializeOriginPatterns() {
    if (allowedOrigins != null && !allowedOrigins.trim().isEmpty()) {
        allowedOriginPatterns = 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 || allowedOriginPatterns == null || allowedOriginPatterns.isEmpty()) {
        return false;
    }
    return allowedOriginPatterns.stream()
        .anyMatch(pattern -> pattern.matcher(origin).matches());
}
src/main/java/com/iemr/mmu/service/dataSyncActivity/DownloadDataFromServerImpl.java-330-335 (1)

330-335: Add validation for required vanID parameter.

The code extracts vanID from the request but doesn't validate whether it exists or handle the null case. If vanID is missing, null will be added to localImportPayload (line 352), which could cause issues in downstream processing.

🛡️ Add validation
 		JSONObject originalRequest = new JSONObject(requestOBJ);
         BigInteger vanID = null;
         if (originalRequest.has("vanID")) {
             vanID = originalRequest.getBigInteger("vanID");
         }
+        if (vanID == null) {
+            throw new IllegalArgumentException("vanID is required in the request");
+        }
         logger.info("Extracted vanID from original request: " + vanID);
src/main/java/com/iemr/mmu/controller/anc/ANCController.java-116-132 (1)

116-132: Update API contract for two doctor data endpoints to include prescribed drug IDs.

The response structure for /save/doctorData and /update/doctorData endpoints has changed from a simple success message to a JSON object containing both a message and a prescribedDrugIDs array. This requires client applications to update their response parsing logic to access the nested structure.

Ensure all clients consuming these two endpoints are updated to handle:

{
  "data": {
    "message": "Data saved successfully",
    "prescribedDrugIDs": [...]
  },
  "statusCode": 200,
  ...
}

Note: Other similar endpoints in this controller (e.g., updateANCCareNurse, updateANCHistoryNurse) continue to return simple string responses, creating an inconsistency in API patterns.

src/main/java/com/iemr/mmu/utils/mapper/SecurityConfig.java-35-39 (1)

35-39: Dead code: CsrfTokenRepository is instantiated but never used.

Lines 35-37 create and configure a CookieCsrfTokenRepository, but line 39 disables CSRF entirely. Remove the unused code.

♻️ Proposed fix
 @Bean
 public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
-    CookieCsrfTokenRepository csrfTokenRepository = new CookieCsrfTokenRepository();
-    csrfTokenRepository.setCookieHttpOnly(true);
-    csrfTokenRepository.setCookiePath("/");
     http
         .csrf(csrf -> csrf.disable())
src/main/java/com/iemr/mmu/service/dataSyncActivity/UploadDataToServerImpl.java-443-607 (1)

443-607: Return failureReasons from syncDataToServer so callers can populate diagnostics

In syncDataToServer you correctly track:

  • successCount / failCount,
  • successVanSerialNos / failedVanSerialNos,
  • failureReasons (per failed record or per batch).

However, the result map only contains:

result.put("status", ...);
result.put("successCount", successCount);
result.put("failCount", failCount);

while startDataSync expects:

List<String> batchFailureReasons = (List<String>) syncResult.get("failureReasons");

Since "failureReasons" is never added to result, batchFailureReasons will always be null and no detailed reasons will be propagated into the group/table summaries.

Recommended fix:

result.put("successCount", successCount);
result.put("failCount", failCount);
result.put("failureReasons", failureReasons);

This will allow startDataSync to attach meaningful failure reasons per table and group, as implied by the new reporting structure.

src/main/java/com/iemr/mmu/service/dataSyncActivity/UploadDataToServerImpl.java-423-435 (1)

423-435: Avoid logging full sync payloads at INFO to reduce PII/PHI exposure

There are several log statements like:

logger.info("Fetched syncData for schema {} and table {}: {}",
    obj.getSchemaName(), obj.getTableName(),
    objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(syncData));

logger.info("Response from server: status={}, hasBody={}, getBody={}",
    response.getStatusCode(), response.hasBody(), response.getBody());

Given these payloads likely contain beneficiary identifiers and clinical data, logging them verbatim at INFO poses a significant privacy/compliance risk (logs are often retained long-term and accessed by more people than the DB).

Suggestions:

  • Downgrade detailed payload logs to DEBUG or TRACE and keep INFO logs high-level (counts, table names, statuses).
  • For error cases, log only minimal identifiers (e.g., table name, batch index, counts) plus a truncated or redacted error message from the server instead of full bodies.

This preserves observability while reducing the chance of sensitive data leaking into log files.

Also applies to: 471-488

src/main/java/com/iemr/mmu/controller/login/IemrMmuLoginController.java-49-61 (1)

49-61: Fix critical order-of-operations bug in getUserVanSpDetails and improve error handling in both JWT methods

getUserServicePointVanDetails (lines 68–93) and getUserVanSpDetails (lines 116–144) have JWT extraction and validation issues:

  1. getUserVanSpDetails has a critical null-dereference bug: Line 124 calls Integer.parseInt(userId) before the null check on line 126. If jwtToken is missing or invalid, userId will be null, causing a NullPointerException that the generic catch block maps to a 5000 error instead of 403.

  2. Both methods conflate JWT/auth errors with application errors: Any exception (including invalid JWT parsing) returns a 5000 error code. Reserved 401/403 status codes should be returned for missing or invalid tokens.

Suggested fixes:

  • In getUserVanSpDetails, null-check userId immediately after extraction (line 123) before attempting Integer.parseInt.
  • In getUserServicePointVanDetails, add a null-check on jwtToken before calling jwtUtil.getUserIdFromToken() to fail fast.
  • Wrap token parsing in a separate try-catch that explicitly returns 403 for JWT validation failures, keeping application errors (5000) separate.
src/main/java/com/iemr/mmu/controller/login/IemrMmuLoginController.java-114-137 (1)

114-137: Validate JWT token before parsing to prevent 500 errors on auth failures

The code at lines 123–124 parses userId before checking if it's null, causing NumberFormatException to be caught and returned as a generic 5000 error instead of the intended 403:

String userId = jwtUtil.getUserIdFromToken(jwtToken);
int userID = Integer.parseInt(userId);  // Executes even if userId is null
...
if (userId != null && obj.has("providerServiceMapID")) {
    ...
} else if (userId == null || jwtToken == null) {
    response.setError(403, "Unauthorized access: Missing or invalid token");  // Unreachable
}

Reorder to validate before parsing:

  1. Check if jwtToken is null/empty → return 403
  2. Derive userId with error handling; on failure → return 403
  3. Parse to int only after userId is confirmed valid

This same issue exists in the getUserServicePointVanDetails method at line 78 (and similar patterns in CommonController and TeleConsultationController). Consider extracting JWT validation into a shared utility method for consistency.

Note: While the class is already protected by @PreAuthorize, using Spring's SecurityContext to retrieve the authenticated user ID (via Authentication#getPrincipal) would eliminate the need to reparse the JWT in each method and provide a more robust, centralized approach.

src/main/java/com/iemr/mmu/service/benFlowStatus/CommonBenStatusFlowServiceImpl.java-75-75 (1)

75-75: VanSerialNo assigned but not persisted.

After saving, obj.setVanSerialNo((objRS.getBenFlowID())) is called but obj is never saved again, so this assignment has no effect on the database.

🐛 Proposed fix - save after assignment or use repository update
             if (beneficiaryRegID != null && beneficiaryID != null && beneficiaryRegID > 0 && beneficiaryID > 0) {
                 objRS = beneficiaryFlowStatusRepo.save(obj);
-                obj.setVanSerialNo((objRS.getBenFlowID()));
+                if (objRS != null) {
+                    objRS.setVanSerialNo(objRS.getBenFlowID());
+                    beneficiaryFlowStatusRepo.save(objRS);
+                }
                 if (objRS != null)
                     returnOBJ = 1;

Or use a dedicated repository update method.

Also applies to: 95-95

src/main/java/com/iemr/mmu/service/anc/ANCNurseServiceImpl.java-180-182 (1)

180-182: Same VanSerialNo persistence issue in saveAncImmunizationDetails.

The pattern ancWomenVaccineDetailRSList.get(0).setVanSerialNo(...) doesn't persist the change.

src/main/java/com/iemr/mmu/controller/generalOPD/GeneralOPDController.java-438-454 (1)

438-454: Same issue - extracting savedDrugIDs from request instead of result.

Identical pattern as saveBenGenOPDDoctorData. If the intent is to return IDs generated during the update, this should pull from the service result.

src/main/java/com/iemr/mmu/service/anc/ANCNurseServiceImpl.java-125-143 (1)

125-143: Duplicate save call - data saved twice unnecessarily.

The code saves LabTestOrderDetailList at line 126, then checks if it's not empty and saves it again at line 130. This is redundant and could cause issues.

🐛 Proposed fix - remove duplicate save
             ArrayList<LabTestOrderDetail> LabTestOrderDetailListRS = (ArrayList<LabTestOrderDetail>) labTestOrderDetailRepo
                     .saveAll(LabTestOrderDetailList);

             if (!LabTestOrderDetailListRS.isEmpty()) {
-                ArrayList<LabTestOrderDetail> labTestOrderDetailListRS
-                        = (ArrayList<LabTestOrderDetail>) labTestOrderDetailRepo.saveAll(LabTestOrderDetailList);
-
-                if (LabTestOrderDetailList.size() == labTestOrderDetailListRS.size()) {
-                    for (LabTestOrderDetail detail : labTestOrderDetailListRS) {
+                if (LabTestOrderDetailList.size() == LabTestOrderDetailListRS.size()) {
+                    for (LabTestOrderDetail detail : LabTestOrderDetailListRS) {
                         if (detail.getLabTestOrderID() != null) {
                             labTestOrderDetailRepo.updateVanSerialNo(detail.getLabTestOrderID());
                         }
                     }
                     r = 1;
                 }
src/main/java/com/iemr/mmu/service/dataSyncLayerCentral/GetDataFromVanAndSyncToDBImpl.java-692-716 (1)

692-716: SQL injection risk in INSERT query construction.

The serverColumns parameter is directly concatenated into the SQL query without validation. If serverColumns contains malicious input, it could lead to SQL injection.

🔒 Proposed fix - validate column names
 private String getQueryToInsertDataToServerDB(String schemaName, String tableName, String serverColumns) {
     String[] columnsArr = null;
-    if (serverColumns != null)
+    if (serverColumns != null) {
         columnsArr = serverColumns.split(",");
+        // Validate column names contain only allowed characters
+        for (String col : columnsArr) {
+            if (!col.trim().matches("^[a-zA-Z_][a-zA-Z0-9_]*$")) {
+                throw new IllegalArgumentException("Invalid column name: " + col);
+            }
+        }
+    }
src/main/java/com/iemr/mmu/service/anc/ANCNurseServiceImpl.java-167-168 (1)

167-168: VanSerialNo assigned but not persisted - same issue as CommonBenStatusFlowServiceImpl.

After saving, the code sets ancCareDetailsRS.setVanSerialNo(ancCareDetailsRS.getID()) but doesn't save again, so this value is lost.

🐛 Proposed fix
         ANCCareDetails ancCareDetailsRS = ancCareRepo.save(ancCareDetailsOBJ);
         if (ancCareDetailsRS != null) {
             ancCareDetailsRS.setVanSerialNo((ancCareDetailsRS.getID()));
+            ancCareRepo.save(ancCareDetailsRS); // Persist the VanSerialNo
             ancCareSuccessFlag = ancCareDetailsRS.getID();
         }

Or use a repository update method for efficiency.

🟡 Minor comments (18)
src/main/java/com/iemr/mmu/utils/JwtAuthenticationUtil.java-138-151 (1)

138-151: Fix typos in error message.

Line 149 contains typos: "retrieverole" should be "retrieve role" and "usedId" should be "userId".

📝 Proposed fix
-		throw new IEMRException("Failed to retrieverole for usedId : " + userId + " error : " + e.getMessage());
+		throw new IEMRException("Failed to retrieve role for userId : " + userId + " error : " + e.getMessage());
src/main/java/com/iemr/mmu/utils/JwtAuthenticationUtil.java-80-83 (1)

80-83: Remove unused roles variable or utilize it for authorization context.

The roles are retrieved but never used. This appears to be incomplete implementation—either remove this code or propagate the roles to Spring Security context or cache them for downstream authorization checks.

🔧 Recommended fix

If roles should be cached for later use:

 if(user != null) {
   // Store roles in security context or cache for authorization
   List<String> roles = getUserRoles(user.getUserID());
+  // TODO: Store roles in Spring Security context or RedisStorage
+  // Example: redisStorage.cacheUserRoles(user.getUserID(), roles);
 }

Or if this is premature code:

 if(user != null) {
-  // Store roles in security context or cache for authorization
-  List<String> roles = getUserRoles(user.getUserID());
 }
src/main/java/com/iemr/mmu/service/generalOPD/GeneralOPDServiceImpl.java-1571-1583 (1)

1571-1583: Same NPE risk in update path.

Line 1581 has the same null-check ordering issue.

🐛 Proposed fix
-				if (r > 0 && r != null) {
+				if (r != null && r > 0) {
src/main/java/com/iemr/mmu/service/generalOPD/GeneralOPDServiceImpl.java-918-931 (1)

918-931: Same NPE risk as in NCDCareServiceImpl.

The null check order issue exists here as well on line 929.

🐛 Proposed fix
 					Map<String, Object> drugSaveResult = commonNurseServiceImpl
 							.saveBenPrescribedDrugsList(prescribedDrugDetailList);
 					Integer r = (Integer) drugSaveResult.get("count");
-					List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");
+					@SuppressWarnings("unchecked")
+					List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");

 					// Store IDs in JsonObject
 					if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
 						Gson gson = new Gson();
 						requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
 					}

-					if (r > 0 && r != null) {
+					if (r != null && r > 0) {
 						prescriptionSuccessFlag = r;
 					}
src/main/java/com/iemr/mmu/service/ncdCare/NCDCareServiceImpl.java-1289-1299 (1)

1289-1299: Same issues as in save path - NPE risk and unchecked cast.

The condition on line 1299 has the same null-check ordering issue as in saveDoctorData.

🐛 Proposed fix
 				Map<String, Object> drugSaveResult = commonNurseServiceImpl
 						.saveBenPrescribedDrugsList(prescribedDrugDetailList);
 				Integer r = (Integer) drugSaveResult.get("count");
-				List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");
+				@SuppressWarnings("unchecked")
+				List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");

 				// Store IDs in JsonObject
 				if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
 					Gson gson = new Gson();
 					requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
 				}
-				if (r > 0 && r != null) {
+				if (r != null && r > 0) {
 					prescriptionSuccessFlag = r;
 				}
src/main/java/com/iemr/mmu/service/ncdCare/NCDCareServiceImpl.java-850-863 (1)

850-863: Potential NPE and unchecked cast.

Two issues in this segment:

  1. Line 853: Unchecked cast from Object to List<Long> may cause ClassCastException if the map contains an unexpected type.
  2. Line 861: The condition r > 0 && r != null evaluates r > 0 before checking for null, which would throw NPE if r is null.
🐛 Proposed fix
 				Map<String, Object> drugSaveResult = commonNurseServiceImpl
 						.saveBenPrescribedDrugsList(prescribedDrugDetailList);
 				Integer r = (Integer) drugSaveResult.get("count");
-				List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");
+				@SuppressWarnings("unchecked")
+				List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");

 				// Store IDs in JsonObject
 				if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
 					Gson gson = new Gson();
 					requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
 				}

-				if (r > 0 && r != null) {
+				if (r != null && r > 0) {
 					prescriptionSuccessFlag = r;
 				}
src/main/java/com/iemr/mmu/controller/teleconsultation/TeleConsultationController.java-28-35 (1)

28-35: Remove duplicate import.

JwtUtil is imported twice (lines 28 and 35).

🐛 Proposed fix
 import org.springframework.security.access.prepost.PreAuthorize;
-import com.iemr.mmu.utils.JwtUtil;

 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 import com.iemr.mmu.utils.JwtUtil;

Committable suggestion skipped: line range outside the PR's diff.

src/main/java/com/iemr/mmu/controller/reports/ReportGateway.java-45-46 (1)

45-46: Add a comment explaining the dual lab technician role names.

The codebase consistently includes both 'LABTECHNICIAN' and 'LAB_TECHNICIAN' across multiple controllers, indicating this is intentional backwards compatibility. Add a code comment above the annotation explaining why both roles are required (e.g., "/* Supporting both LABTECHNICIAN and LAB_TECHNICIAN role names for backwards compatibility */") to prevent future developers from attempting to remove the apparent duplicate.

src/main/java/com/iemr/mmu/service/quickConsultation/QuickConsultationServiceImpl.java-697-699 (1)

697-699: Same null check order issue in update path.

The condition r > 0 && r != null has the null check in the wrong order, risking NPE.

Fix the condition order
-			if (r > 0 && r != null) {
+			if (r != null && r > 0) {
src/main/java/com/iemr/mmu/service/covid19/Covid19ServiceImpl.java-1264-1266 (1)

1264-1266: Same null check order issue in saveDoctorData.

Fix the condition order
-				if (r > 0 && r != null) {
+				if (r != null && r > 0) {
src/main/java/com/iemr/mmu/service/covid19/Covid19ServiceImpl.java-956-958 (1)

956-958: Null check order is incorrect - potential NPE.

Same issue as in QuickConsultationServiceImpl: r > 0 && r != null will throw NPE if r is null.

Fix the condition order
-				if (r > 0 && r != null) {
+				if (r != null && r > 0) {
src/main/java/com/iemr/mmu/service/quickConsultation/QuickConsultationServiceImpl.java-460-462 (1)

460-462: Null check order is incorrect - potential NPE.

The condition r > 0 && r != null will throw a NullPointerException if r is null because the null check comes after the comparison.

Fix the condition order
-			if (r > 0 && r != null) {
+			if (r != null && r > 0) {
src/main/java/com/iemr/mmu/controller/location/LocationController.java-153-158 (1)

153-158: Integer.parseInt may throw NumberFormatException.

If userId from the token contains a non-numeric value, Integer.parseInt(userId) will throw an uncaught exception, resulting in a 500 error instead of the intended 403.

Add exception handling
 			if(userId != null) {
-				int userID = Integer.parseInt(userId);
+				int userID;
+				try {
+					userID = Integer.parseInt(userId);
+				} catch (NumberFormatException e) {
+					logger.error("Invalid userId format: " + userId);
+					response.setError(403, "Unauthorized access");
+					return response.toString();
+				}
 			if (obj != null && obj.has("spID") && obj.has("spPSMID") && obj.get("spID") != null
src/main/java/com/iemr/mmu/utils/mapper/RoleAuthenticationFilter.java-26-26 (1)

26-26: Wrong IOException import causes compilation issues.

The import io.jsonwebtoken.io.IOException is incorrect. This should be java.io.IOException. The method signature already uses java.io.IOException explicitly on line 48, indicating awareness of this conflict.

Fix the import
-import io.jsonwebtoken.io.IOException;
+import java.io.IOException;

Then simplify line 48:

-	throws ServletException, IOException, java.io.IOException {
+	throws ServletException, IOException {

Committable suggestion skipped: line range outside the PR's diff.

src/main/java/com/iemr/mmu/service/benFlowStatus/CommonBenStatusFlowServiceImpl.java-297-297 (1)

297-297: Inconsistent error messages - "ben flow creation" for update operations.

The error messages say "Error in ben flow creation" but these are update methods.

🐛 Proposed fix
         } catch (Exception e) {
-            logger.error("Error in ben flow creation = " + e);
+            logger.error("Error in ben flow update = " + e);
             throw new Exception(e);
         }

Also applies to: 319-319, 341-341

src/main/java/com/iemr/mmu/service/labtechnician/LabTechnicianServiceImpl.java-446-461 (1)

446-461: Unchecked cast warning for ecgAbnormalities list.

The cast (List<String>) comp.get("ecgAbnormalities") is unchecked and could fail at runtime if the actual type differs.

🐛 Safer alternative
                             if (comp.containsKey("ecgAbnormalities") && comp.get("ecgAbnormalities") != null) {
-                                List<String> ecgAbnormalitiesList = (List<String>) comp.get("ecgAbnormalities");
+                                Object ecgObj = comp.get("ecgAbnormalities");
+                                List<?> ecgAbnormalitiesList = ecgObj instanceof List ? (List<?>) ecgObj : null;

-                                if (ecgAbnormalitiesList != null && ecgAbnormalitiesList.size() > 0) {
+                                if (ecgAbnormalitiesList != null && !ecgAbnormalitiesList.isEmpty()) {
                                     StringBuilder sb = new StringBuilder();
-                                    for (String abnormility : ecgAbnormalitiesList) {
-                                        sb.append(abnormility).append("||");
+                                    for (Object abnormality : ecgAbnormalitiesList) {
+                                        sb.append(String.valueOf(abnormality)).append("||");
                                     }
src/main/java/com/iemr/mmu/service/dataSyncLayerCentral/GetDataFromVanAndSyncToDBImpl.java-483-483 (1)

483-483: Incorrect logger format string - missing placeholder.

The logger statement uses a comma instead of concatenation but the format string has no placeholder, so updateParams.toArray() won't be logged.

🐛 Proposed fix
-                logger.info("Update Params=",updateParams.toArray());
+                logger.info("Update Params={}", Arrays.toString(updateParams.toArray()));
src/main/java/com/iemr/mmu/service/dataSyncLayerCentral/GetDataFromVanAndSyncToDBImpl.java-617-625 (1)

617-625: Off-by-one error in substring extraction.

The condition keyStart > 8 should be keyStart > 9 since "for key '" is 9 characters, and indexOf returns the start position. If indexOf returns 0 (found at start), keyStart would be 9, which is valid.

🐛 Proposed fix
-        if (message.contains("Duplicate entry") && message.contains("for key")) {
-            // Extract: "Duplicate entry 'value' for key 'constraint_name'"
-            int keyStart = message.indexOf("for key '") + 9;
-            int keyEnd = message.indexOf("'", keyStart);
-            if (keyStart > 8 && keyEnd > keyStart) {
+        if (message.contains("Duplicate entry") && message.contains("for key")) {
+            int keyStart = message.indexOf("for key '") + 9;
+            int keyEnd = message.indexOf("'", keyStart);
+            if (keyStart >= 9 && keyEnd > keyStart) {

Comment on lines +24 to +26
import java.util.ArrayList;
import java.util.HashMap;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Remove duplicate imports.

Lines 24-26 and 50-56 contain duplicate import statements for ArrayList, HashMap, List, Map, and Gson. This will cause compilation errors.

🐛 Proposed fix
-import java.util.ArrayList;
-import java.util.HashMap;
-
 import org.json.JSONObject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.repository.query.Param;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.transaction.annotation.Transactional;
 
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 
 import com.google.gson.Gson;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParser;
 import com.iemr.mmu.service.anc.ANCService;
 import com.iemr.mmu.utils.response.OutputResponse;
 
 import io.swagger.v3.oas.annotations.Operation;
 
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import com.google.gson.Gson;
 import com.google.gson.JsonArray;

Also applies to: 50-56

🤖 Prompt for AI Agents
In @src/main/java/com/iemr/mmu/controller/anc/ANCController.java around lines 24
- 26, Remove the duplicate import declarations in ANCController.java: keep a
single set of imports for java.util.ArrayList, java.util.HashMap,
java.util.List, java.util.Map and com.google.gson.Gson and delete the redundant
repeated import block (the duplicates around lines 24-26 and 50-56). Ensure only
one import statement exists for each symbol (ArrayList, HashMap, List, Map,
Gson) so the file compiles without duplicate-import errors.

Comment on lines 400 to 402
if (r > 0 && r != null) {
prescriptionSuccessFlag = r;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Null check order causes potential NPE.

The condition r > 0 && r != null will throw a NullPointerException if r is null, because r > 0 is evaluated first (Java's short-circuit evaluation is left-to-right). The null check should come first.

🐛 Proposed fix
-if (r > 0 && r != null) {
+if (r != null && r > 0) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (r > 0 && r != null) {
prescriptionSuccessFlag = r;
}
if (r != null && r > 0) {
prescriptionSuccessFlag = r;
}
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/mmu/service/anc/ANCServiceImpl.java around lines 400
- 402, In ANCServiceImpl the conditional uses `r > 0 && r != null`, which can
NPE if r is null; change the check to test null first (e.g., `r != null && r >
0` or use `Objects.nonNull(r) && r > 0`) before assigning
`prescriptionSuccessFlag = r` so the null check guards the numeric comparison.

Comment on lines 1628 to 1630
if (r > 0 && r != null) {
prescriptionSuccessFlag = r;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Same null check order bug - potential NPE.

This has the identical issue as Line 400. The condition r > 0 && r != null will NPE if r is null.

🐛 Proposed fix
-if (r > 0 && r != null) {
+if (r != null && r > 0) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (r > 0 && r != null) {
prescriptionSuccessFlag = r;
}
if (r != null && r > 0) {
prescriptionSuccessFlag = r;
}
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/mmu/service/anc/ANCServiceImpl.java around lines 1628
- 1630, In ANCServiceImpl, the null check for variable r is ordered incorrectly
causing a potential NPE; update the condition that sets prescriptionSuccessFlag
so you check r for null first (e.g., change the if that currently reads using r
> 0 && r != null to check r != null before comparing) or use Objects.nonNull(r)
&& r > 0, ensuring you reference the same local variable r and assign to
prescriptionSuccessFlag only when r is non-null and greater than zero.

Comment on lines +101 to 121
public int updateProcessedFlagInVan(String schemaName, String tableName, List<String> vanSerialNos,
String autoIncreamentColumn, String user, String status, String reason) throws Exception {
jdbcTemplate = getJdbcTemplate();
String query = "";

logger.info("Updating processed flag in table: " + tableName + " for vanSerialNos: " + vanSerialNos);

if (tableName != null && tableName.toLowerCase().equals("i_ben_flow_outreach")) {
query = "UPDATE " + schemaName + "." + tableName
+ " SET created_date = ? , processed = 'P', SyncedDate = ?, Syncedby = ? "
+ "WHERE " + autoIncreamentColumn + " IN (" + vanSerialNos + ")";
+ " SET processed = ?, SyncedDate = ?, Syncedby = ? , SyncFailureReason = ? "
+ "WHERE " + autoIncreamentColumn + " IN (" + String.join(",", vanSerialNos) + ")";
} else {
query = "UPDATE " + schemaName + "." + tableName
+ " SET CreatedDate = ? , processed = 'P', SyncedDate = ?, Syncedby = ? "
+ "WHERE " + autoIncreamentColumn + " IN (" + vanSerialNos + ")";
+ " SET processed = ?, SyncedDate = ?, Syncedby = ? , SyncFailureReason = ? "
+ "WHERE " + autoIncreamentColumn + " IN (" + String.join(",", vanSerialNos) + ")";
}

Timestamp syncedDate = new Timestamp(System.currentTimeMillis());
int updatedRows = jdbcTemplate.update(query, syncedDate, syncedDate, user);
int updatedRows = jdbcTemplate.update(query, status, syncedDate, user, reason);
return updatedRows;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: SQL Injection vulnerability in query construction.

The vanSerialNos list values and autoIncreamentColumn are concatenated directly into the SQL query without parameterization. If any of these values come from untrusted sources, this creates a SQL injection vulnerability.

Additionally, lines 108-116 contain duplicate code - both if and else branches construct identical queries.

🔒 Recommended fix using parameterized queries

Consider using parameterized queries with NamedParameterJdbcTemplate for the IN clause:

+import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;

-	public int updateProcessedFlagInVan(String schemaName, String tableName, List<String> vanSerialNos,
-			String autoIncreamentColumn, String user, String status, String reason) throws Exception {
-		jdbcTemplate = getJdbcTemplate();
-		String query = "";
-
-		logger.info("Updating processed flag in table: " + tableName + " for vanSerialNos: " + vanSerialNos);
-	
-		if (tableName != null && tableName.toLowerCase().equals("i_ben_flow_outreach")) {
-			query = "UPDATE " + schemaName + "." + tableName
-					+ " SET processed = ?, SyncedDate = ?, Syncedby = ? , SyncFailureReason = ? "
-					+ "WHERE " + autoIncreamentColumn + " IN (" + String.join(",", vanSerialNos) + ")";
-		} else {
-			query = "UPDATE " + schemaName + "." + tableName
-					+ " SET processed = ?, SyncedDate = ?, Syncedby = ? , SyncFailureReason = ? "
-					+ "WHERE " + autoIncreamentColumn + " IN (" + String.join(",", vanSerialNos) + ")";
-		}
-
-		Timestamp syncedDate = new Timestamp(System.currentTimeMillis());
-		int updatedRows = jdbcTemplate.update(query, status, syncedDate, user, reason);
-		return updatedRows;
+	public int updateProcessedFlagInVan(String schemaName, String tableName, List<String> vanSerialNos,
+			String autoIncreamentColumn, String user, String status, String reason) throws Exception {
+		// Validate table and column names against allowlist to prevent SQL injection
+		if (!isValidTableName(tableName) || !isValidColumnName(autoIncreamentColumn)) {
+			throw new IllegalArgumentException("Invalid table or column name");
+		}
+		
+		NamedParameterJdbcTemplate namedJdbcTemplate = new NamedParameterJdbcTemplate(dataSource);
+		
+		logger.info("Updating processed flag in table: {} for vanSerialNos count: {}", tableName, vanSerialNos.size());
+		
+		String query = "UPDATE " + schemaName + "." + tableName
+				+ " SET processed = :status, SyncedDate = :syncedDate, Syncedby = :user, SyncFailureReason = :reason "
+				+ "WHERE " + autoIncreamentColumn + " IN (:vanSerialNos)";
+		
+		MapSqlParameterSource params = new MapSqlParameterSource();
+		params.addValue("status", status);
+		params.addValue("syncedDate", new Timestamp(System.currentTimeMillis()));
+		params.addValue("user", user);
+		params.addValue("reason", reason);
+		params.addValue("vanSerialNos", vanSerialNos);
+		
+		return namedJdbcTemplate.update(query, params);

Also add allowlist validation for table/column names:

private static final Set<String> ALLOWED_TABLES = Set.of("i_ben_flow_outreach", /* other tables */);
private static final Set<String> ALLOWED_COLUMNS = Set.of("van_serial_no", /* other columns */);

private boolean isValidTableName(String tableName) {
    return tableName != null && ALLOWED_TABLES.contains(tableName.toLowerCase());
}

private boolean isValidColumnName(String columnName) {
    return columnName != null && ALLOWED_COLUMNS.contains(columnName.toLowerCase());
}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In @src/main/java/com/iemr/mmu/service/dataSyncActivity/DataSyncRepository.java
around lines 101 - 121, The updateProcessedFlagInVan method builds SQL by
concatenating vanSerialNos and autoIncreamentColumn causing SQL injection and
has duplicate branches; fix it by validating tableName and autoIncreamentColumn
against an allowlist (e.g., ALLOWED_TABLES and ALLOWED_COLUMNS) then use a
NamedParameterJdbcTemplate (or JdbcTemplate with positional bind markers) to
parameterize the IN clause for vanSerialNos (create a named list parameter like
:ids) and bind status, SyncedDate, user, reason as parameters; collapse the
identical if/else into a single query construction that uses the validated
table/column names and the parameterized IN clause, then execute the update with
the bound parameters.

indentRepo.saveAll(indentList);

int updateFlag = updateProcessedFlagToCentral("db_iemr_sync", "t_indent", ids, ServerAuthorization, token);
int updateFlag = updateProcessedFlagToCentral("db_iemr", "t_indent", ids, ServerAuthorization, token);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for db_iemr_sync and db_iemr references
echo "=== Searching for db_iemr_sync references ==="
rg -n "db_iemr_sync" --type java

echo -e "\n=== Searching for db_iemr schema references in Java ==="
rg -n '"db_iemr"' --type java -C 1

echo -e "\n=== Checking configuration files ==="
fd -E node_modules -E .git \( -name "*.properties" -o -name "*.yml" -o -name "*.yaml" -o -name "application*" \) -type f | head -20

echo -e "\n=== Searching for schema definitions in config files ==="
rg -n "db_iemr" --type properties --type yaml --type xml 2>/dev/null | head -30

Repository: PSMRI/MMU-API

Length of output: 5814


🏁 Script executed:

#!/bin/bash
# Check the file to see all updateProcessedFlagToCentral calls with context
echo "=== All updateProcessedFlagToCentral calls in the file ==="
rg -n "updateProcessedFlagToCentral" src/main/java/com/iemr/mmu/service/dataSyncActivity/DownloadDataFromServerTransactionalImpl.java -A 1

echo -e "\n=== Method definition of updateProcessedFlagToCentral ==="
rg -n "def.*updateProcessedFlagToCentral|void updateProcessedFlagToCentral|int updateProcessedFlagToCentral" src/main/java/com/iemr/mmu/service/dataSyncActivity/ -B 2 -A 10

Repository: PSMRI/MMU-API

Length of output: 2974


🏁 Script executed:

#!/bin/bash
# Search for other potential references to these schemas in other service files
echo "=== Search for db_iemr references in sync/dataSyncActivity directory ==="
rg -n '"db_iemr' src/main/java/com/iemr/mmu/service/dataSyncActivity/ -C 1

echo -e "\n=== Check for SyncUploadDataDigester usage ==="
rg -n "SyncUploadDataDigester" --type java -A 3

Repository: PSMRI/MMU-API

Length of output: 35015


Verify central server is correctly configured for "db_iemr" schema.

Schema name has been changed from "db_iemr_sync" to "db_iemr" across all five updateProcessedFlagToCentral calls (lines 112, 139, 167, 194-195, 222-223), aligning them with the downloadDataFromCentral calls which already use "db_iemr". This change is systematic and appears intentional (commented code in DownloadDataFromServerImpl.java line 243 indicates migration from "db_iemr_sync").

Confirm:

  1. The central server's database is correctly configured with "db_iemr" schema
  2. The processed flag update API endpoint targets the correct schema
  3. Synchronization testing passes without silent failures or data inconsistencies
🤖 Prompt for AI Agents
In
@src/main/java/com/iemr/mmu/service/dataSyncActivity/DownloadDataFromServerTransactionalImpl.java
at line 112, Confirm and ensure the central server schema used in
updateProcessedFlagToCentral calls is correct: verify the central DB actually
exposes the "db_iemr" schema (not "db_iemr_sync") and that the processed-flag
update API endpoint accepts/operates on "db_iemr"; update configuration or
revert the schema string in all calls to updateProcessedFlagToCentral (used in
DownloadDataFromServerTransactionalImpl) to match the central server if
necessary, and run end-to-end synchronization tests (including the
downloadDataFromCentral flows referenced in DownloadDataFromServerImpl) to
validate there are no silent failures or data inconsistencies.

Comment on lines +501 to +517
if (null != labResultsListNew && labResultsListNew.size() > 0) {
List<LabResultEntry> labResultEntryRes = (List<LabResultEntry>) labResultEntryRepo.saveAll(labResultsListNew);

if (labResultEntryRes != null && labResultsListNew.size() == labResultEntryRes.size()) {
for (LabResultEntry result : labResultEntryRes) {
if (result.getID() != null) {
labResultEntryRepo.updateVanSerialNo(result.getID());
}
}
labResultSaveFlag = 1;
}
} else {
labResultSaveFlag = 1;
}
}
return labResultSaveFlag;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Syntax error - mismatched braces causing compilation failure.

The closing braces and return statement are incorrectly placed, likely causing the pipeline failure. The if block at line 501 has an extra closing brace at line 515 that doesn't match the method structure.

🐛 Proposed fix
             if (null != labResultsListNew && labResultsListNew.size() > 0) {
                 List<LabResultEntry> labResultEntryRes = (List<LabResultEntry>) labResultEntryRepo.saveAll(labResultsListNew);

                 if (labResultEntryRes != null && labResultsListNew.size() == labResultEntryRes.size()) {
                     for (LabResultEntry result : labResultEntryRes) {
                         if (result.getID() != null) {
                             labResultEntryRepo.updateVanSerialNo(result.getID());
                         }
                     }
                     labResultSaveFlag = 1;
                 }
             } else {
                 labResultSaveFlag = 1;
             }
-			}
-            return labResultSaveFlag;
-        }
+        }
+        return labResultSaveFlag;
+    }
🤖 Prompt for AI Agents
In
@src/main/java/com/iemr/mmu/service/labtechnician/LabTechnicianServiceImpl.java
around lines 501 - 517, There is a mismatched/extra closing brace causing a
compilation error around the block that saves lab results; in the method
containing the if (null != labResultsListNew && labResultsListNew.size() > 0) {
... } block remove the stray closing brace so the method's braces align
properly, ensure the return labResultSaveFlag; remains inside the method (not
inside an extra block), and verify the for-loop and outer if/else (which sets
labResultSaveFlag = 1) close correctly; specifically check and fix brace
placement around the labResultEntryRepo.saveAll(...) handling and the subsequent
updateVanSerialNo(result.getID()) loop so the method compiles.

Comment on lines +271 to +284
Map<String, Object> drugSaveResult = commonNurseServiceImpl
.saveBenPrescribedDrugsList(prescribedDrugDetailList);
Integer r = (Integer) drugSaveResult.get("count");
List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");

// Store IDs in JsonObject
if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
Gson gson = new Gson();
requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
}

if (r > 0 && r != null) {
prescriptionSuccessFlag = r;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Null check order causes potential NPE.

Line 282 has r > 0 && r != null which will throw NPE if r is null. The null check must come first.

🐛 Proposed fix
-if (r > 0 && r != null) {
+if (r != null && r > 0) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Map<String, Object> drugSaveResult = commonNurseServiceImpl
.saveBenPrescribedDrugsList(prescribedDrugDetailList);
Integer r = (Integer) drugSaveResult.get("count");
List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");
// Store IDs in JsonObject
if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
Gson gson = new Gson();
requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
}
if (r > 0 && r != null) {
prescriptionSuccessFlag = r;
}
Map<String, Object> drugSaveResult = commonNurseServiceImpl
.saveBenPrescribedDrugsList(prescribedDrugDetailList);
Integer r = (Integer) drugSaveResult.get("count");
List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");
// Store IDs in JsonObject
if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
Gson gson = new Gson();
requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
}
if (r != null && r > 0) {
prescriptionSuccessFlag = r;
}
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/mmu/service/ncdscreening/NCDScreeningServiceImpl.java
around lines 271 - 284, The null-check order for the Integer r is wrong and can
cause a NullPointerException when evaluating "r > 0 && r != null"; change the
condition to check for null first and then compare the value (e.g., "r != null
&& r > 0") before assigning to prescriptionSuccessFlag; update the block around
the saveBenPrescribedDrugsList result handling (variables: drugSaveResult, r,
prescribedDrugIDs, requestOBJ, prescriptionSuccessFlag) to perform the null
check first and only set prescriptionSuccessFlag when r is non-null and
positive.

Comment on lines +1336 to 1346
Map<String, Object> drugSaveResult = commonNurseServiceImpl
.saveBenPrescribedDrugsList(prescribedDrugDetailList);
Integer r = (Integer) drugSaveResult.get("count");
List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");

// Store IDs in JsonObject
if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
Gson gson = new Gson();
requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
}
if (r > 0 && r != null) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Same null check order bug at Line 1346.

The condition r > 0 && r != null will NPE when r is null.

🐛 Proposed fix
-if (r > 0 && r != null) {
+if (r != null && r > 0) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Map<String, Object> drugSaveResult = commonNurseServiceImpl
.saveBenPrescribedDrugsList(prescribedDrugDetailList);
Integer r = (Integer) drugSaveResult.get("count");
List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");
// Store IDs in JsonObject
if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
Gson gson = new Gson();
requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
}
if (r > 0 && r != null) {
Map<String, Object> drugSaveResult = commonNurseServiceImpl
.saveBenPrescribedDrugsList(prescribedDrugDetailList);
Integer r = (Integer) drugSaveResult.get("count");
List<Long> prescribedDrugIDs = (List<Long>) drugSaveResult.get("prescribedDrugIDs");
// Store IDs in JsonObject
if (prescribedDrugIDs != null && !prescribedDrugIDs.isEmpty()) {
Gson gson = new Gson();
requestOBJ.add("savedDrugIDs", gson.toJsonTree(prescribedDrugIDs));
}
if (r != null && r > 0) {
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/mmu/service/ncdscreening/NCDScreeningServiceImpl.java
around lines 1336 - 1346, The condition in NCDScreeningServiceImpl where you
check Integer r from commonNurseServiceImpl.saveBenPrescribedDrugsList(...) uses
"r > 0 && r != null" which will throw an NPE if r is null; change the check to
test nullity first (e.g., "r != null && r > 0" or use Objects.nonNull(r) && r >
0) before using r, and update any similar checks around the variable or method
saveBenPrescribedDrugsList to follow the same safe-evaluation ordering.

Comment on lines +60 to 66
public Claims extractAllClaims(String token) {
return Jwts.parser()
.verifyWith(getSigningKey())
.build()
.parseSignedClaims(token)
.getPayload();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Security risk: Public extractAllClaims bypasses token validation.

Making extractAllClaims public exposes an API that parses JWT tokens without verifying their integrity or checking the denylist. This bypasses the security checks in validateToken (lines 30-49), potentially allowing denylisted or invalid tokens to be used.

🔒 Recommended fix

Option 1 (Preferred): Keep extractAllClaims private and update callers to use validateToken:

-	public Claims extractAllClaims(String token) {
+	private Claims extractAllClaims(String token) {
		return Jwts.parser()
			.verifyWith(getSigningKey())
			.build()
			.parseSignedClaims(token)
			.getPayload();
	}

Option 2: If public access is required, add validation:

	public Claims extractAllClaims(String token) {
+		// Validate token and check denylist first
+		Claims validatedClaims = validateToken(token);
+		if (validatedClaims == null) {
+			throw new SecurityException("Invalid or denylisted token");
+		}
+		return validatedClaims;
-		return Jwts.parser()
-			.verifyWith(getSigningKey())
-			.build()
-			.parseSignedClaims(token)
-			.getPayload();
	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public Claims extractAllClaims(String token) {
return Jwts.parser()
.verifyWith(getSigningKey())
.build()
.parseSignedClaims(token)
.getPayload();
}
private Claims extractAllClaims(String token) {
return Jwts.parser()
.verifyWith(getSigningKey())
.build()
.parseSignedClaims(token)
.getPayload();
}
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/mmu/utils/JwtUtil.java around lines 60 - 66,
extractAllClaims currently exposes a way to parse JWTs without performing the
same integrity and denylist checks done in validateToken; make extractAllClaims
private and update all callers to call validateToken instead, or if you must
keep it public, add the same validation inside: verify signature with
getSigningKey, check expiration and the application's denylist (the same logic
used by validateToken), and throw a security exception for invalid or denylisted
tokens so no caller can bypass validateToken's checks.

Comment on lines +56 to +57
filterChain.doFilter(request, response);
return;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Filter chain invoked twice - duplicate request processing.

The filter calls filterChain.doFilter() in early return paths (lines 56, 61, 67, 75) AND in the finally block (line 99). This causes the filter chain to be invoked twice when returning early, leading to duplicate request processing.

🐛 Fix by removing early filterChain calls or restructuring
 	@Override
 	protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
 			throws ServletException, IOException, java.io.IOException {
 		List<String> authRoles = null;
 		try {
 			String jwtFromCookie = CookieUtil.getJwtTokenFromCookie(request);
 			String jwtFromHeader = request.getHeader("Jwttoken");

 			String jwtToken = jwtFromCookie != null ? jwtFromCookie : jwtFromHeader;
 			if(null == jwtToken || jwtToken.trim().isEmpty()) {
-				filterChain.doFilter(request, response);
 				return;
 			}
 			Claims extractAllClaims = jwtUtil.extractAllClaims(jwtToken);
 			if(null == extractAllClaims) {
-				filterChain.doFilter(request, response);
 				return;
 			}
 			Object userIdObj = extractAllClaims.get("userId");
 			String userId = userIdObj != null ? userIdObj.toString() : null;
 			if (null == userId || userId.trim().isEmpty()) {
-				filterChain.doFilter(request, response);
 				return;
 			}
 			Long userIdLong;
 			try {
 				userIdLong=Long.valueOf(userId);
 			}catch (NumberFormatException ex) {
 				logger.warn("Invalid userId format: {}",userId);
-				filterChain.doFilter(request, response);
 				return;
 			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
filterChain.doFilter(request, response);
return;
return;
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/mmu/utils/mapper/RoleAuthenticationFilter.java around
lines 56 - 57, The RoleAuthenticationFilter calls filterChain.doFilter(request,
response) in multiple early-return branches and again in the finally block,
causing duplicate chain invocation; remove the duplicate calls by keeping a
single invocation point: eliminate the early filterChain.doFilter(...) + return
statements (those inside the branches that check conditions and currently call
filterChain.doFilter) and let the single call in the finally block handle
continuing the chain, or alternatively move the sole
filterChain.doFilter(request, response) out of finally and use early returns
without calling the chain in the finally; ensure only one filterChain.doFilter
invocation remains (referencing filterChain.doFilter(request, response) and the
finally block in RoleAuthenticationFilter).

@5Amogh 5Amogh merged commit 5862f19 into main Jan 7, 2026
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants