feat(cors):Cors configuration for hwc api service #133
Conversation
WalkthroughThis update centralizes and standardizes CORS (Cross-Origin Resource Sharing) handling by introducing global configuration classes and filters. All Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DynamicCorsFilter
participant JwtUserIdValidationFilter
participant SpringController
Client->>DynamicCorsFilter: Sends HTTP request (with Origin header)
DynamicCorsFilter->>DynamicCorsFilter: Check if Origin is allowed
alt Origin allowed
DynamicCorsFilter->>Client: Set Access-Control-Allow-Origin header
end
alt OPTIONS request
DynamicCorsFilter->>Client: Respond 200 OK (preflight)
else Other HTTP methods
DynamicCorsFilter->>JwtUserIdValidationFilter: Pass request
JwtUserIdValidationFilter->>JwtUserIdValidationFilter: Validate JWT & set CORS headers
JwtUserIdValidationFilter->>SpringController: Pass request
SpringController-->>JwtUserIdValidationFilter: Handle business logic
JwtUserIdValidationFilter-->>DynamicCorsFilter: Pass response
DynamicCorsFilter-->>Client: Return response with CORS headers
end
Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/main/java/com/iemr/hwc/controller/uptsu/UPTechnicalSupportUnitController.java (1)
57-91: 💡 Verification agent🧩 Analysis chain
Verify centralized CORS configuration handles all endpoints.
The @crossorigin annotations have been properly removed from all methods. Ensure that the new centralized CORS configuration (
CorsConfig,DynamicCorsFilter) properly handles all the endpoint patterns and HTTP methods for this controller.
🏁 Script executed:
#!/bin/bash # Description: Verify that centralized CORS configuration exists and covers the endpoint patterns # Check if CorsConfig class exists and examine its configuration echo "=== Checking CorsConfig implementation ===" ast-grep --pattern 'class CorsConfig { $$$ }' echo "=== Checking for CORS-related filter implementations ===" ast-grep --pattern 'class DynamicCorsFilter { $$$ }' echo "=== Checking cors.allowed-origins property configuration ===" fd -t f -e properties | xargs rg -l "cors.allowed-origins" echo "=== Verifying endpoint patterns in CORS config ===" ast-grep --pattern 'addMapping($$$)'Length of output: 753
Implement centralized CORS configuration for all endpoints
It looks like all @crossorigin annotations were removed, but I couldn’t find any central CORS setup in the codebase:
- No
CorsConfigorDynamicCorsFilterclass definitions- No calls to
CorsRegistry.addMapping(...)- Only
cors.allowed-originsproperties exist in environment filesPlease add a global CORS configuration (for example, a
@Configurationclass implementingWebMvcConfigureror a filter) that:
- Registers mappings covering
/get/action-master,/getWorklistByVanID/**, and any other controller routes- Allows the required HTTP methods and origins (wired to
cors.allowed-origins)Once that’s in place and tested, the removal of per-endpoint @crossorigin annotations will be safe.
🧹 Nitpick comments (8)
src/main/environment/common_example.properties (1)
99-99: Configure wildcard localhost origins for development
This setscors.allowed-origins=http://localhost:*for local testing. Note: if you need to support wildcard ports with credentials, consider usingallowedOriginPatternsinCorsConfiginstead ofallowedOrigins. Otherwise, verify thathttp://localhost:*is supported by your Spring Boot version.src/main/java/com/iemr/hwc/controller/snomedct/SnomedController.java (1)
28-28: Remove unused CrossOrigin import
Since all@CrossOriginannotations have been stripped, the importorg.springframework.web.bind.annotation.CrossOriginis no longer used. Removing it will clean up the imports.src/main/java/com/iemr/hwc/controller/spoke/VanSpokeController.java (1)
28-28: Remove unused CrossOrigin import
With all controller-level@CrossOriginannotations removed, the importorg.springframework.web.bind.annotation.CrossOriginis now unused. Please delete it to keep the code clean.src/main/java/com/iemr/hwc/controller/videoconsultation/VideoConsultationController.java (1)
27-27: Remove unused CrossOrigin import
The importorg.springframework.web.bind.annotation.CrossOriginremains even though all annotations have been removed. Cleaning this up will prevent unused dependencies.src/main/java/com/iemr/hwc/controller/registrar/master/RegistrarMasterController.java (1)
29-29: Remove unused @crossorigin import.Since all @crossorigin annotations have been removed, the import statement is no longer needed and should be cleaned up.
-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/hwc/controller/foetalmonitor/FoetalMonitorController.java (1)
26-33: UnusedCrossOriginimport should be removed
Theimport org.springframework.web.bind.annotation.CrossOrigin;remains but no CORS annotations are used. Please remove this unused import to clean up the code.src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java (2)
43-55: Verify CORS header configuration and OPTIONS handling.The CORS implementation looks generally correct, but there are a few considerations:
- The
Access-Control-Allow-Headersshould include common headers likeX-Requested-With- Consider making CORS headers configurable rather than hardcoded
Consider this improvement:
if (origin != null && isOriginAllowed(origin)) { response.setHeader("Access-Control-Allow-Origin", origin); response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS"); - response.setHeader("Access-Control-Allow-Headers", "Authorization, Content-Type, Accept, Jwttoken"); + response.setHeader("Access-Control-Allow-Headers", "Authorization, Content-Type, Accept, Jwttoken, X-Requested-With"); response.setHeader("Access-Control-Allow-Credentials", "true"); + response.setHeader("Access-Control-Max-Age", "3600"); }
25-25: Consider separating CORS and JWT concerns.Mixing CORS handling with JWT validation in the same filter violates the Single Responsibility Principle and makes the code harder to maintain and test.
Consider creating a separate
CorsFilterthat runs before the JWT validation filter in the filter chain. This would:
- Improve separation of concerns
- Make CORS configuration more reusable
- Simplify testing
- Allow CORS to work independently of JWT validation
The filter chain should be:
CorsFilter→JwtUserIdValidationFilter→ ControllersAlso applies to: 43-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/hwc/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.java(3 hunks)src/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.java(7 hunks)src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.java(1 hunks)src/main/java/com/iemr/hwc/controller/choApp/CHOAppSyncController.java(1 hunks)src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java(20 hunks)src/main/java/com/iemr/hwc/controller/common/master/CommonMasterController.java(1 hunks)src/main/java/com/iemr/hwc/controller/covid19/CovidController.java(5 hunks)src/main/java/com/iemr/hwc/controller/dataSyncActivity/StartSyncActivity.java(6 hunks)src/main/java/com/iemr/hwc/controller/dataSyncLayerCentral/MMUDataSyncController.java(3 hunks)src/main/java/com/iemr/hwc/controller/diabetesAndHypertensionOutcome/DiabetesHypertensionScreeningController.java(3 hunks)src/main/java/com/iemr/hwc/controller/familyPlanning/FamilyPlanningController.java(4 hunks)src/main/java/com/iemr/hwc/controller/foetalmonitor/FoetalMonitorController.java(6 hunks)src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java(9 hunks)src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java(6 hunks)src/main/java/com/iemr/hwc/controller/location/LocationController.java(1 hunks)src/main/java/com/iemr/hwc/controller/masterVillage/MasterVillageController.java(1 hunks)src/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.java(5 hunks)src/main/java/com/iemr/hwc/controller/ncdscreening/NCDScreeningController.java(7 hunks)src/main/java/com/iemr/hwc/controller/neonatal/NeonatalController.java(6 hunks)src/main/java/com/iemr/hwc/controller/patientApp/master/PatientAppCommonMasterController.java(0 hunks)src/main/java/com/iemr/hwc/controller/pnc/PostnatalCareController.java(2 hunks)src/main/java/com/iemr/hwc/controller/quickBlox/QuickbloxController.java(2 hunks)src/main/java/com/iemr/hwc/controller/quickconsult/QuickConsultController.java(3 hunks)src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java(11 hunks)src/main/java/com/iemr/hwc/controller/registrar/master/RegistrarMasterController.java(2 hunks)src/main/java/com/iemr/hwc/controller/report/CRMReportController.java(5 hunks)src/main/java/com/iemr/hwc/controller/snomedct/SnomedController.java(2 hunks)src/main/java/com/iemr/hwc/controller/spoke/VanSpokeController.java(4 hunks)src/main/java/com/iemr/hwc/controller/teleconsultation/TeleConsultationController.java(1 hunks)src/main/java/com/iemr/hwc/controller/uptsu/UPTechnicalSupportUnitController.java(3 hunks)src/main/java/com/iemr/hwc/controller/version/VersionController.java(1 hunks)src/main/java/com/iemr/hwc/controller/videoconsultation/VideoConsultationController.java(6 hunks)src/main/java/com/iemr/hwc/controller/wo/LocationControllerWo.java(2 hunks)src/main/java/com/iemr/hwc/controller/wo/UserBiomectrics.java(2 hunks)src/main/java/com/iemr/hwc/utils/DynamicCorsFilter.java(1 hunks)src/main/java/com/iemr/hwc/utils/FilterConfig.java(1 hunks)src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java(4 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/iemr/hwc/controller/patientApp/master/PatientAppCommonMasterController.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/iemr/hwc/utils/FilterConfig.java (1)
src/main/java/com/iemr/hwc/config/CorsConfig.java (1)
Configuration(8-24)
src/main/java/com/iemr/hwc/config/CorsConfig.java (1)
src/main/java/com/iemr/hwc/utils/FilterConfig.java (1)
Configuration(8-26)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (54)
src/main/java/com/iemr/hwc/controller/quickBlox/QuickbloxController.java (2)
39-39: Ensure global CORS applies after removal of class‐level annotation
The class‐level@CrossOriginannotation was removed here. Confirm that your newCorsConfigandDynamicCorsFilterare correctly registered and cover requests to/quickblox/**, so that no endpoints become inaccessible due to missing CORS headers at the class level.
48-48: Validate per‐endpoint CORS handling after removal of method‐level annotation
The method‐level@CrossOriginongetquickbloxIdswas removed. Verify that preflight OPTIONS and actual POST calls are handled properly by the centralized CORS filter, especially since controllers are mapped withheaders="Authorization". Ensure the CORS filter intercepts OPTIONS requests before that header check.src/main/environment/common_ci.properties (1)
100-100: Introduce centralized CORS origins property
The newcors.allowed-origins=@CORS_ALLOWED_ORIGINS@entry externalizes allowed origins to your CI environment. This aligns with the centralized CORS setup inCorsConfig. Ensure the placeholder@CORS_ALLOWED_ORIGINS@correctly expands to a comma-separated list of valid origins and that the application fails fast with clear logging if it’s unset or malformed.src/main/java/com/iemr/hwc/controller/version/VersionController.java (1)
48-48: Remove method‐level CORS annotation – verify coverage
You’ve removed the@CrossOriginannotation beforeversionInformation(). Confirm that the global CORS configuration applies to the/versionendpoint and that no preflight or actual GET requests are blocked. Consider adding integration tests for CORS responses on this endpoint.src/main/java/com/iemr/hwc/controller/common/master/CommonMasterController.java (1)
42-42: Removed class‐level CORS annotation – check preflight handling
The@CrossOriginannotation was removed from the controller. Given the class‐level mapping includesheaders="Authorization", ensure that CORS preflight (OPTIONS) requests bypass this header requirement and are handled by your global CORS filter.src/main/java/com/iemr/hwc/controller/masterVillage/MasterVillageController.java (1)
37-37: Removal of per-controller CORS annotation
The explicit@CrossOriginannotation was removed to rely on the centralized CORS configuration. Ensure that your newCorsConfigandDynamicCorsFilterare correctly applied to all/masterVillage/**endpoints so that browsers receive the expected CORS headers.src/main/java/com/iemr/hwc/controller/snomedct/SnomedController.java (2)
58-58: Removal of per-method CORS annotation
The manual@CrossOriginongetSnomedCTRecordwas removed in favor of the global CORS filter. Please verify that preflightOPTIONSrequests for this endpoint are handled correctly by your centralized configuration.
84-84: Removal of per-method CORS annotation
Similarly, the@CrossOriginongetSnomedCTRecordListhas been removed. Confirm that yourCorsConfigapplies to/snomed/getSnomedCTRecordList.src/main/java/com/iemr/hwc/controller/spoke/VanSpokeController.java (3)
56-56: Removal of per-method CORS annotation
@CrossOriginwas removed fromgetUserServicePointVanDetails. Ensure your global CORS setup covers this POST endpoint and responds with appropriate CORS headers.
77-77: Removal of per-method CORS annotation
@CrossOriginongetServicepointVillageshas been removed. Verify that/user/getServicepointVillagesis included in the allowed origins list.
99-99: Removal of per-method CORS annotation
The@CrossOriginongetUserSpokeDetailswas dropped; confirm that the global filter is registered before any security filters (e.g., JWT) so that preflight OPTIONS aren’t inadvertently blocked.src/main/java/com/iemr/hwc/controller/videoconsultation/VideoConsultationController.java (6)
47-47: Removal of per-method CORS annotation
The@CrossOriginon theloginendpoint was removed. Please test that/videoConsultation/login/{userID}returns the correctAccess-Control-Allow-Originheader via the new CORS configuration.
69-69: Removal of per-method CORS annotation
CORS annotation removed from thecallendpoint. Confirm that your global CORS policy is applied for/videoConsultation/call/{fromUserID}/{toUserID}.
91-91: Removal of per-method CORS annotation
Removed@CrossOriginfromcallSwymedAndJitsi. Verify that both variants of this GET mapping are covered by the centralized CORS settings.
119-119: Removal of per-method CORS annotation
The annotation oncallvanwas dropped. Ensure global config includes the/videoConsultation/callvan/**path.
141-141: Removal of per-method CORS annotation
Removed fromcallVanSwymedAndJitsi. Double-check your CORS filter handles the{type}variant too.
171-171: Removal of per-method CORS annotation
@CrossOriginremoved fromlogout. Validate that logout calls still return the proper CORS headers globally.src/main/java/com/iemr/hwc/controller/choApp/CHOAppSyncController.java (1)
39-39: Removal of class-level CORS annotation
The class-level@CrossOriginis removed to take advantage of the new centralized CORS setup. Please verify that all/sync/**endpoints are included in yourCorsConfigmappings.src/main/java/com/iemr/hwc/controller/uptsu/UPTechnicalSupportUnitController.java (1)
50-50: LGTM: Clean removal of @crossorigin annotation from class level.The removal aligns with the centralized CORS configuration approach, which is an architectural improvement over scattered annotations.
src/main/java/com/iemr/hwc/controller/familyPlanning/FamilyPlanningController.java (1)
218-367: LGTM: Consistent removal of @crossorigin annotations from all methods.The removal of @crossorigin annotations from all endpoints (
getBenVitalDetailsFrmNurseFamilyPlanning,getBenFPDetailsFrmNurseFamilyPlanning,getBenCaseRecordFromDoctor,updateFamilyPlanningDoctorData) is properly executed and aligns with the centralized CORS configuration strategy.src/main/java/com/iemr/hwc/controller/registrar/master/RegistrarMasterController.java (1)
44-58: LGTM: Proper removal of @crossorigin annotations.The class-level and method-level @crossorigin annotations have been correctly removed as part of the centralized CORS configuration refactoring.
src/main/java/com/iemr/hwc/controller/location/LocationController.java (1)
180-183:Details
✅ Verification successful
LGTM: @crossorigin annotation properly removed.
The removal aligns with the centralized CORS configuration approach. This controller contains both GET and POST endpoints, so ensure the centralized configuration handles all HTTP methods used by this controller.
🏁 Script executed:
#!/bin/bash # Description: Verify that the centralized CORS config handles all HTTP methods used in LocationController echo "=== Checking HTTP methods in LocationController ===" rg -A 2 "@(GetMapping|PostMapping|RequestMapping)" src/main/java/com/iemr/hwc/controller/location/LocationController.java echo "=== Verifying CORS config allows these HTTP methods ===" ast-grep --pattern 'allowedMethods($$$)'Length of output: 2304
I want to locate the centralized CORS configuration to confirm it covers all HTTP methods used. Let’s search for WebMvcConfigurer implementations, CorsFilter beans, and addCorsMappings calls:
🏁 Script executed:
#!/bin/bash echo "=== Searching for CORS config class implementing WebMvcConfigurer ===" rg -n "implements.*WebMvcConfigurer" -C3 . echo "=== Searching for addCorsMappings usage ===" rg -n "addCorsMappings" -C3 . echo "=== Searching for CorsFilter bean definitions ===" rg -n "CorsFilter" -C3 . echo "=== Searching for CorsConfigurationSource usage ===" rg -n "CorsConfigurationSource" -C3 .Length of output: 3088
Confirmed centralized CORS config covers GET and POST endpoints
CorsConfig.addCorsMappings registers allowedMethods("GET", "POST", "PUT", "DELETE", "OPTIONS"), which includes all HTTP methods used by LocationController. No further changes needed.
src/main/java/com/iemr/hwc/controller/diabetesAndHypertensionOutcome/DiabetesHypertensionScreeningController.java (1)
42-42:Details
✅ Verification successful
Centralize CORS handling by removing per-controller annotations.
The deletion of
@CrossOriginat both the class and method levels is correct—CORS is now managed via the new globalCorsConfig. Please verify that your global configuration includes the/screeningOutcome/**path and allows OPTIONS (preflight) along with the required HTTP methods and headers.Run this to confirm:
Also applies to: 51-51, 69-69
🏁 Script executed:
#!/bin/bash # Verify that CorsConfig registers mappings for all endpoints (including /screeningOutcome/**) rg -n "registry\\.addMapping" -A2 src/main/java/com/iemr/hwc/config/CorsConfig.javaLength of output: 257
Global CORS config covers screeningOutcome endpoints
TheCorsConfigregisters a mapping for"/**"(line 16) with.allowedMethods("GET", "POST", "PUT", "DELETE", "OPTIONS")(lines 17–18), so it includes/screeningOutcome/**and supports preflight OPTIONS. No further changes needed—approving these updates.src/main/java/com/iemr/hwc/controller/dataSyncActivity/StartSyncActivity.java (1)
51-51:Details
✅ Verification successful
Unified CORS configuration applied globally: per-method annotations removed.
The removed
@CrossOriginentries simplify the controller. Ensure your global CORS settings cover/dataSyncActivity/**(including preflightOPTIONS) with the correct allowed origins, methods, headers, and credentials.Also applies to: 89-89, 111-111, 145-145, 159-159
🏁 Script executed:
#!/bin/bash # Check CorsConfig mappings rg -n "registry\\.addMapping" -A2 src/main/java/com/iemr/hwc/config/CorsConfig.javaLength of output: 257
Continuing verification of CORS settings in CorsConfig.java:
🏁 Script executed:
#!/bin/bash # Inspect full CORS configuration for headers and credentials rg -n "addMapping" -A5 src/main/java/com/iemr/hwc/config/CorsConfig.java rg -n "allowedHeaders" -A2 src/main/java/com/iemr/hwc/config/CorsConfig.java rg -n "allowCredentials" -A2 src/main/java/com/iemr/hwc/config/CorsConfig.javaLength of output: 859
Global CORS configuration covers
/dataSyncActivity/**The
CorsConfig.javamapping
- registry.addMapping("/**")
- .allowedOriginPatterns(allowedOrigins.split(","))
- .allowedMethods("GET","POST","PUT","DELETE","OPTIONS")
- .allowedHeaders("*")
- .exposedHeaders("Authorization","Jwttoken")
- .allowCredentials(true)
- .maxAge(3600)
satisfies preflight
OPTIONS, headers, methods, and credentials for all endpoints (including/dataSyncActivity/**). No further changes needed.src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.java (1)
62-62:Details
✅ Verification successful
Removed individual CORS annotations in favor of centralized config.
Good removal of the method-level
@CrossOriginannotations; CORS is now handled byCorsConfig. Confirm that/child-adolescent-care/**is included in the global mappings.Also applies to: 128-128, 295-295
🏁 Script executed:
#!/bin/bash # Ensure CorsConfig covers child-adolescent-care endpoints rg -n "registry\\.addMapping" -A2 src/main/java/com/iemr/hwc/config/CorsConfig.javaLength of output: 257
CORS mapping confirmed for all endpoints
The globalCorsConfigusesregistry.addMapping("/**")(line 16), which covers
/child-adolescent-care/**. No further changes needed.src/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.java (1)
107-107:Details
✅ Verification successful
Switching to global CORS config: per-method
@CrossOriginremoved.The cleanup of
@CrossOriginannotations is on point. Verify that the global configuration inCorsConfigcorrectly exposes/ANC/**to the allowed origins and handles preflight requests properly.Also applies to: 176-176, 213-213, 246-246, 279-279, 539-539
🏁 Script executed:
#!/bin/bash # Validate global CORS mappings rg -n "registry\\.addMapping" -A2 src/main/java/com/iemr/hwc/config/CorsConfig.javaLength of output: 257
Global CORS config correctly covers
/ANC/**The
CorsConfigregisters a global mapping for"/**", uses your configuredallowedOriginspatterns, and explicitly allows"OPTIONS"alongside standard methods—so preflight requests and all/ANC/**endpoints are handled as expected. No further changes needed here.src/main/java/com/iemr/hwc/controller/report/CRMReportController.java (1)
54-54:Details
✅ Verification successful
Removed isolated CORS annotations; global config now applies.
Nice removal of scattered
@CrossOriginannotations. Please double-check that/TMReport/**endpoints are covered by your centralized CORS policy and that preflight requests succeed.Also applies to: 75-75, 96-96, 118-118, 139-139
🏁 Script executed:
#!/bin/bash # Confirm CorsConfig includes TMReport mappings rg -n "registry\\.addMapping" -A2 src/main/java/com/iemr/hwc/config/CorsConfig.javaLength of output: 257
Global CORS policy covers
/TMReport/**endpoints
TheCorsConfigregisters a mapping for"/**"(lines 16–18), includingOPTIONS, so all/TMReport/**routes are covered and preflight requests will succeed. No further changes needed.src/main/java/com/iemr/hwc/controller/wo/LocationControllerWo.java (2)
32-38: Explicit import cleanup — good practice
Replacing wildcard imports with explicit imports forGetMapping,PostMapping,PathVariable,RequestBody,RequestMapping, andRestControllerimproves readability and prevents accidental inclusion of unused classes.
142-144: Removed@CrossOriginannotation — verify global CORS coverage
Controller-level CORS annotations have been removed in favor of the new centralized configuration. Please ensure that theCorsConfigandDynamicCorsFilterproperly apply CORS headers (including preflight OPTIONS) for the/wo/location/**endpoints.src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java (1)
27-35: Removed@CrossOriginimports and annotations
All per-controller and method-level@CrossOriginannotations have been stripped to centralize CORS handling. Confirm that the/registrar/**routes are fully covered by the globalCorsConfigand that OPTIONS preflight requests are handled byDynamicCorsFilter.src/main/java/com/iemr/hwc/controller/pnc/PostnatalCareController.java (1)
28-34: Removed per-controller@CrossOriginannotations
CORS is now configured globally; controller-level annotations have been removed. Verify that/PNC/**endpoints correctly return the required CORS headers (including allowed origins and credentials) under the newCorsConfig.src/main/java/com/iemr/hwc/controller/foetalmonitor/FoetalMonitorController.java (1)
49-53: Removed controller-level@CrossOrigin— verify centralized handling
All explicit@CrossOriginannotations have been removed. Ensure that the/foetalMonitor/**endpoints are correctly covered by the global filter (DynamicCorsFilter) and configuration (CorsConfig) for both simple and preflight requests.src/main/java/com/iemr/hwc/controller/teleconsultation/TeleConsultationController.java (1)
25-32: Removed@CrossOriginfrom TeleConsultationController
CORS concerns are now handled centrally; verify that/tc/**mappings correctly emit CORS headers (includingAccess-Control-Allow-Credentialsif needed) and that OPTIONS preflight requests are not blocked by the JWT filter.src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.java (1)
52-52: LGTM! Good architectural improvement.The removal of
@CrossOriginannotations from this controller aligns with the centralized CORS configuration approach. This eliminates code duplication and provides better maintainability.src/main/java/com/iemr/hwc/controller/covid19/CovidController.java (1)
51-51: LGTM! Consistent with CORS centralization.The removal of
@CrossOriginannotations from this controller is consistent with the architectural improvement to centralize CORS configuration.src/main/java/com/iemr/hwc/controller/ncdscreening/NCDScreeningController.java (1)
54-54: LGTM! Part of beneficial CORS centralization.The removal of
@CrossOriginannotations from this controller maintains consistency with the overall architectural improvement to centralize CORS configuration.src/main/java/com/iemr/hwc/config/CorsConfig.java (1)
1-24:Details
❓ Verification inconclusive
Excellent centralized CORS configuration with security considerations.
This new configuration class effectively centralizes CORS policy management. The implementation is well-structured and comprehensive.
Key strengths:
- Property-based configuration allows environment-specific settings
- Uses
allowedOriginPatternsfor flexible origin matching- Comprehensive method and header support
- Proper preflight caching with
maxAgeSecurity considerations to verify:
allowCredentials(true)combined with pattern-based origins requires careful configuration of thecors.allowed-originsproperty to avoid overly permissive access- Ensure the property contains only trusted origins/patterns
Please verify that the
cors.allowed-originsproperty is properly configured with specific, trusted origins rather than overly broad patterns like*whenallowCredentials(true)is used.
🏁 Script executed:
#!/bin/bash # Description: Check CORS configuration properties across environment files # Expected: Find cors.allowed-origins property with specific origins, not wildcard patterns echo "Searching for CORS configuration in environment files..." fd -t f -e properties . --exec grep -l "cors.allowed-origins" {} \; echo -e "\nCORS property values found:" fd -t f -e properties . --exec grep "cors.allowed-origins" {} \; echo -e "\nChecking for potentially unsafe wildcard configurations..." fd -t f -e properties . --exec grep -i "cors.allowed-origins.*\*" {} \;Length of output: 759
Excellent centralized CORS configuration with security considerations.
This new
CorsConfigclass effectively centralizes CORS policy management. The implementation is clear, environment‐driven, and covers all necessary facets (methods, headers, credentials, caching).Key strengths:
- Property-based configuration for environment-specific settings
allowedOriginPatternsenables flexible yet controlled origin matching- Comprehensive support for HTTP methods and headers
- Proper preflight caching via
.maxAge(3600)Security considerations (please verify):
allowCredentials(true)plus broad patterns can inadvertently expose endpoints to untrusted origins.- In
src/main/environment/common_example.properties:– acceptable for local dev only; ensure this never propagates to production.cors.allowed-origins=http://localhost:*- In
src/main/environment/common_ci.properties:– verify that the CI/CD–injected value is a comma-separated list of explicit, trusted origins (no wildcards) when deployed.cors.allowed-origins=@CORS_ALLOWED_ORIGINS@Action items:
- Confirm your production properties override any wildcards and list only specific, trusted origins.
- Consider updating the example file to a more concrete development origin (e.g.,
http://localhost:3000) to avoid confusion.src/main/java/com/iemr/hwc/utils/FilterConfig.java (2)
14-26: LGTM! Well-structured filter registration.The filter configuration follows Spring Boot best practices:
- Proper use of
@Beanannotation- Correct dependency injection via method parameter
- Appropriate URL pattern mapping to intercept all requests
- Clear separation of concerns by externalizing CORS origins configuration
11-12:Details
✅ Verification successful
Verify the property injection and usage.
Ensure that the
cors.allowed-originsproperty is properly defined in your configuration files and that theJwtUserIdValidationFilterconstructor accepts theallowedOriginsparameter as shown in line 20.
🏁 Script executed:
#!/bin/bash # Description: Verify the JwtUserIdValidationFilter constructor signature and property configuration # Check if JwtUserIdValidationFilter has the expected constructor ast-grep --pattern $'class JwtUserIdValidationFilter { $$$ JwtUserIdValidationFilter($_, $_) { $$$ } $$$ }' # Check for cors.allowed-origins property in configuration files rg -A 2 -B 2 "cors\.allowed-origins"Length of output: 2504
✅ Property Injection & Constructor Signature Verified
The
cors.allowed-originsproperty is defined in your environment configs (common_example.properties,common_ci.properties), and theJwtUserIdValidationFilterconstructor signature (JwtAuthenticationUtil, String allowedOrigins) matches how you inject it inFilterConfig. No missing imports or dependency issues detected.src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java (1)
46-46: LGTM! Clean formatting improvements.The formatting changes improve code readability by adding consistent spacing around method groups and sections. The removal of
@CrossOriginannotations (as mentioned in the summary) correctly supports the centralization of CORS handling through the new global configuration.Also applies to: 95-95, 122-122, 143-143, 163-163, 207-207, 227-227, 247-247, 267-267, 305-305, 655-655, 676-676, 701-701, 727-727, 754-754, 853-853, 905-905, 930-930, 954-954, 983-983
src/main/java/com/iemr/hwc/controller/dataSyncLayerCentral/MMUDataSyncController.java (1)
49-49: LGTM! Consistent with CORS centralization strategy.The formatting improvements and removal of
@CrossOriginannotations align with the application-wide refactoring to centralize CORS handling through global configuration and filters.Also applies to: 60-60, 79-79
src/main/java/com/iemr/hwc/controller/wo/UserBiomectrics.java (1)
24-32: LGTM: Well-organized imports and annotations.The Spring annotations and import organization look clean and follow best practices.
src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java (1)
27-31: LGTM: Proper dependency injection for CORS configuration.The constructor properly injects the allowed origins configuration using
@Valueannotation.src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java (3)
47-47: LGTM: Removal of @crossorigin annotation aligns with centralized CORS strategy.The removal of the
@CrossOriginannotation is consistent with the PR's objective to centralize CORS configuration through filters and global configuration.
65-65: LGTM: Consistent removal of @crossorigin annotations from methods.All method-level
@CrossOriginannotations have been properly removed, which aligns with the centralized CORS approach implemented in this PR.Also applies to: 101-101, 132-132, 159-159
172-173: LGTM: Minor formatting improvement.The method call formatting has been improved for better readability without changing the logic.
src/main/java/com/iemr/hwc/controller/quickconsult/QuickConsultController.java (2)
51-51: LGTM: Removal of class-level @crossorigin annotation.The removal of the class-level
@CrossOriginannotation is consistent with the centralized CORS configuration strategy implemented in this PR.
73-73: LGTM: Consistent removal of method-level @crossorigin annotations.All method-level
@CrossOriginannotations have been properly removed from the quick consultation endpoints, which aligns with the centralized CORS approach.Also applies to: 155-155, 187-187, 219-219, 255-255
src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java (3)
66-105: LGTM: Method-level CORS annotations properly removed.The removal of
@CrossOriginannotations from individual methods is consistent with the centralized CORS approach. This eliminates scattered CORS configurations and improves maintainability.
142-418: LGTM: Consistent CORS annotation removal across all endpoints.All remaining method-level
@CrossOriginannotations have been systematically removed, ensuring consistent application of the centralized CORS configuration across all General OPD endpoints.Verify that the endpoints still function correctly with the new CORS setup by testing a representative sample of the API endpoints for proper CORS headers in the response.
51-51:Details
✅ Verification successful
LGTM: CORS annotation removal aligns with centralization strategy.
The removal of
@CrossOriginannotations from the controller class supports the migration to centralized CORS configuration, which is a better architectural approach.Please verify that the centralized CORS configuration properly handles all endpoints:
🏁 Script executed:
#!/bin/bash # Description: Verify that centralized CORS configuration exists and covers all endpoints # Search for the new CorsConfig class mentioned in AI summary ast-grep --pattern 'class CorsConfig implements WebMvcConfigurer { $$$ }' # Search for DynamicCorsFilter mentioned in AI summary ast-grep --pattern 'class DynamicCorsFilter { $$$ }' # Check for CORS configuration properties rg -A 5 "cors\.allowed-origins" # Verify no remaining @CrossOrigin annotations in controllers rg "@CrossOrigin" --type javaLength of output: 2675
Centralized CORS configuration verified and complete
- Confirmed src/main/java/com/iemr/hwc/config/CorsConfig.java implements WebMvcConfigurer and applies
registry.addMapping("/**").- DynamicCorsFilter is present in src/main/java/com/iemr/hwc/utils/DynamicCorsFilter.java and wired via FilterConfig.
- CORS origins are driven by
cors.allowed-originsin all environment property files.- No remaining
@CrossOriginannotations found in any controllers.All endpoints are now covered by the centralized CORS setup. LGTM.
src/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.java (2)
51-51: LGTM: Consistent CORS centralization pattern.The removal of class-level
@CrossOriginannotation follows the same centralization pattern as other controllers, maintaining consistency across the codebase.
66-347: LGTM: Complete removal of method-level CORS annotations.All method-level
@CrossOriginannotations have been systematically removed from NCD care endpoints, ensuring consistent application of centralized CORS configuration across all methods in this controller.src/main/java/com/iemr/hwc/controller/neonatal/NeonatalController.java (1)
63-264:Details
❓ Verification inconclusive
LGTM: Completes consistent CORS centralization across controllers.
The systematic removal of all
@CrossOriginannotations from the neonatal controller endpoints completes the consistent pattern observed across all reviewed controllers. This centralization approach will improve maintainability and provide better control over CORS policies.Since this appears to be part of a comprehensive CORS refactoring, please verify that:
- All controllers in the application have had their
@CrossOriginannotations removed- The centralized CORS configuration handles all the specific CORS requirements that were previously defined in individual annotations
- End-to-end testing confirms that cross-origin requests still work as expected
🏁 Script executed:
#!/bin/bash # Description: Final verification of CORS migration completeness # Check if any @CrossOrigin annotations remain in the entire codebase echo "Checking for remaining @CrossOrigin annotations..." rg "@CrossOrigin" --type java --context 2 # Verify the centralized CORS configuration files exist echo "Checking for centralized CORS configuration..." fd -e java | xargs rg -l "CorsConfig\|DynamicCorsFilter" # Check for CORS-related properties echo "Checking CORS configuration properties..." fd -e properties | xargs rg "cors\."Length of output: 619
Missing Centralized CORS Configuration
The search for a global CORS config class (
CorsConfigorDynamicCorsFilter) returned no Java files, even though the CORS properties exist undersrc/main/environment. Please verify that you have a central CORS configuration in place to replace all removed@CrossOriginannotations. For example, a@Configurationclass registeringaddCorsMappings(...)or a customCorsFilter.Checklist for final validation:
- Confirm a global CORS config class (e.g.,
WebConfig,CorsConfig, etc.) insrc/main/javathat readscors.allowed-originsand applies the policy.- Ensure this config covers all endpoints previously annotated with
@CrossOrigin.- Run end-to-end tests to validate that cross-origin requests still succeed.
| String origin = request.getHeader("Origin"); | ||
| if (origin != null && Arrays.asList(allowedOrigins).contains(origin)) { | ||
| response.setHeader("Access-Control-Allow-Origin", origin); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider redundancy with existing CORS configuration.
This filter duplicates CORS functionality that's already handled by the CorsConfig class. Having both may cause conflicts or unexpected behavior.
Consider whether this filter is necessary given that CorsConfig already provides comprehensive CORS configuration. If you need filter-based CORS handling, ensure it doesn't conflict with the Spring MVC CORS configuration, or disable one of them.
+ // Consider adding @ConditionalOnProperty or similar to avoid conflicts
+ // with the existing CorsConfigCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/hwc/utils/DynamicCorsFilter.java around lines 26 to
29, the CORS handling duplicates functionality already provided by the
CorsConfig class, which can cause conflicts. Review the overall CORS setup and
decide whether to remove this DynamicCorsFilter entirely or disable the Spring
MVC CORS configuration in CorsConfig to avoid overlapping. Ensure only one CORS
mechanism is active to prevent unexpected behavior.
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| } else { | ||
| filterChain.doFilter(request, response); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing CORS headers for preflight requests.
The OPTIONS handling is incomplete. Preflight requests typically require additional CORS headers to be functional.
if ("OPTIONS".equalsIgnoreCase(request.getMethod())) {
+ response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
+ response.setHeader("Access-Control-Allow-Headers", "*");
+ response.setHeader("Access-Control-Max-Age", "3600");
response.setStatus(HttpServletResponse.SC_OK);
+ return; // Don't call filterChain.doFilter() for OPTIONS
} else {
filterChain.doFilter(request, response);
}📝 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.
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | |
| response.setStatus(HttpServletResponse.SC_OK); | |
| } else { | |
| filterChain.doFilter(request, response); | |
| } | |
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | |
| response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS"); | |
| response.setHeader("Access-Control-Allow-Headers", "*"); | |
| response.setHeader("Access-Control-Max-Age", "3600"); | |
| response.setStatus(HttpServletResponse.SC_OK); | |
| return; // Don't call filterChain.doFilter() for OPTIONS | |
| } else { | |
| filterChain.doFilter(request, response); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/hwc/utils/DynamicCorsFilter.java around lines 31 to
35, the handling of OPTIONS preflight requests is incomplete because it only
sets the status to OK without adding necessary CORS headers. To fix this, add
the required CORS headers such as Access-Control-Allow-Origin,
Access-Control-Allow-Methods, Access-Control-Allow-Headers, and
Access-Control-Max-Age to the response before setting the status. This ensures
that preflight requests are properly handled and accepted by the browser.
| throws ServletException, IOException { | ||
|
|
||
| String origin = request.getHeader("Origin"); | ||
| if (origin != null && Arrays.asList(allowedOrigins).contains(origin)) { |
There was a problem hiding this comment.
Update the origin comparison logic.
The current implementation needs to be updated to use the corrected property injection approach.
- if (origin != null && Arrays.asList(allowedOrigins).contains(origin)) {
+ String[] origins = getAllowedOrigins(); // or use List<String> if using SpEL
+ if (origin != null && Arrays.asList(origins).contains(origin)) {📝 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.
| if (origin != null && Arrays.asList(allowedOrigins).contains(origin)) { | |
| String[] origins = getAllowedOrigins(); // or use List<String> if using SpEL | |
| if (origin != null && Arrays.asList(origins).contains(origin)) { |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/hwc/utils/DynamicCorsFilter.java at line 27, update
the origin comparison logic to use the corrected property injection approach.
Replace the current direct use of allowedOrigins array with the properly
injected property value, ensuring that the origin check uses the updated list of
allowed origins obtained via the correct injection method.
| @Value("${cors.allowed-origins}") | ||
| private String[] allowedOrigins; |
There was a problem hiding this comment.
Fix the property injection for array type.
The @Value annotation for array injection requires a SpEL expression. The current syntax may not properly parse the comma-separated values.
- @Value("${cors.allowed-origins}")
- private String[] allowedOrigins;
+ @Value("${cors.allowed-origins:}")
+ private String allowedOriginsStr;
+
+ private String[] getAllowedOrigins() {
+ return allowedOriginsStr.isEmpty() ? new String[0] : allowedOriginsStr.split(",");
+ }Or use SpEL expression:
- @Value("${cors.allowed-origins}")
- private String[] allowedOrigins;
+ @Value("#{T(java.util.Arrays).asList('${cors.allowed-origins}'.split(','))}")
+ private List<String> allowedOrigins;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/hwc/utils/DynamicCorsFilter.java at lines 17 to 18,
the @Value annotation for injecting the allowedOrigins array is incorrect
because it does not use a SpEL expression to parse the comma-separated string
into an array. Fix this by changing the annotation to use a SpEL expression like
@Value("#{'${cors.allowed-origins}'.split(',')}") to properly convert the
property string into a String array.
| public String addFingerPrints(@RequestBody FingerPrintDTO comingRequest) { | ||
| OutputResponse response = new OutputResponse(); | ||
| try { | ||
| if (comingRequest != null && comingRequest.getUserName() != null) { | ||
| String resp = registrarService.saveFingerprints(comingRequest); | ||
| if (resp != null && resp.equals("ok")) { | ||
| response.setResponse(resp); | ||
| } else if (resp != null && resp.equals("ko")) { | ||
| response.setError(500, "Error adding fingerprints"); | ||
| } | ||
| } else { | ||
| response.setError(400, "Invalid request"); | ||
| } | ||
| } catch (Exception e) { | ||
| logger.error(e.getMessage()); | ||
| response.setError(500, "Error while adding fingerprints data"); | ||
| } | ||
| return response.toString(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and input validation.
The error handling could be more robust and consistent:
- The method checks for
userNamebut the response handling logic is confusing with "ok"/"ko" string comparisons - Consider using proper HTTP status codes instead of generic 500 errors
- Add input validation for the DTO
Consider this improved approach:
@PostMapping(value = "add/fingerprint/wo", produces = { "application/json" })
public String addFingerPrints(@RequestBody FingerPrintDTO comingRequest) {
OutputResponse response = new OutputResponse();
try {
- if (comingRequest != null && comingRequest.getUserName() != null) {
+ if (comingRequest != null && comingRequest.getUserName() != null && !comingRequest.getUserName().trim().isEmpty()) {
String resp = registrarService.saveFingerprints(comingRequest);
- if (resp != null && resp.equals("ok")) {
+ if ("ok".equals(resp)) {
response.setResponse(resp);
- } else if (resp != null && resp.equals("ko")) {
- response.setError(500, "Error adding fingerprints");
+ } else {
+ response.setError(422, "Failed to save fingerprints");
}
} else {
response.setError(400, "Invalid request");
}
} catch (Exception e) {
logger.error(e.getMessage());
- response.setError(500, "Error while adding fingerprints data");
+ response.setError(500, "Internal server error while adding fingerprints");
}
return response.toString();
}📝 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.
| public String addFingerPrints(@RequestBody FingerPrintDTO comingRequest) { | |
| OutputResponse response = new OutputResponse(); | |
| try { | |
| if (comingRequest != null && comingRequest.getUserName() != null) { | |
| String resp = registrarService.saveFingerprints(comingRequest); | |
| if (resp != null && resp.equals("ok")) { | |
| response.setResponse(resp); | |
| } else if (resp != null && resp.equals("ko")) { | |
| response.setError(500, "Error adding fingerprints"); | |
| } | |
| } else { | |
| response.setError(400, "Invalid request"); | |
| } | |
| } catch (Exception e) { | |
| logger.error(e.getMessage()); | |
| response.setError(500, "Error while adding fingerprints data"); | |
| } | |
| return response.toString(); | |
| } | |
| @PostMapping(value = "add/fingerprint/wo", produces = { "application/json" }) | |
| public String addFingerPrints(@RequestBody FingerPrintDTO comingRequest) { | |
| OutputResponse response = new OutputResponse(); | |
| try { | |
| if (comingRequest != null | |
| && comingRequest.getUserName() != null | |
| && !comingRequest.getUserName().trim().isEmpty()) { | |
| String resp = registrarService.saveFingerprints(comingRequest); | |
| if ("ok".equals(resp)) { | |
| response.setResponse(resp); | |
| } else { | |
| response.setError(422, "Failed to save fingerprints"); | |
| } | |
| } else { | |
| response.setError(400, "Invalid request"); | |
| } | |
| } catch (Exception e) { | |
| logger.error(e.getMessage()); | |
| response.setError(500, "Internal server error while adding fingerprints"); | |
| } | |
| return response.toString(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/hwc/controller/wo/UserBiomectrics.java lines 54 to 72,
improve error handling by replacing string comparisons of "ok"/"ko" with clearer
status checks or exceptions from registrarService, validate the FingerPrintDTO
input more thoroughly before processing, and use appropriate HTTP status codes
such as 400 for bad requests and 500 for server errors in the response. Also,
enhance the catch block to log full exception details and ensure consistent
error response formatting.
| public String getFingerprintsByUsername(@PathVariable("userID") Long userID) { | ||
| logger.info("Get fingerprint by username ..." + userID); | ||
| OutputResponse response = new OutputResponse(); | ||
| UserBiometricsMapping user = registrarService.getFingerprintsByUserID(userID); | ||
| if (user != null) { | ||
| Gson gson = new Gson(); | ||
| UserBiometricsMapping userBiometricsMapping = new UserBiometricsMapping(); | ||
| userBiometricsMapping.setUserID(user.getUserID()); | ||
| userBiometricsMapping.setFirstName(user.getFirstName()); | ||
| userBiometricsMapping.setLastName(user.getLastName()); | ||
| userBiometricsMapping.setUserName(user.getUserName()); | ||
| userBiometricsMapping.setCreatedBy(user.getUserName()); | ||
| userBiometricsMapping.setRightThumb(user.getRightThumb()); | ||
| userBiometricsMapping.setRightIndexFinger(user.getRightIndexFinger()); | ||
| userBiometricsMapping.setLeftThumb(user.getLeftThumb()); | ||
| userBiometricsMapping.setLeftIndexFinger(user.getLeftIndexFinger()); | ||
|
|
||
| response.setResponse(gson.toJson(userBiometricsMapping)); | ||
| } | ||
| else{ | ||
| response.setError(404, "User with userID: "+userID+" not found"); | ||
| } | ||
| logger.info("Get fingerprint " + response.toString()); | ||
| return response.toString(); | ||
| } | ||
| response.setResponse(gson.toJson(userBiometricsMapping)); | ||
| } else { | ||
| response.setError(404, "User with userID: " + userID + " not found"); | ||
| } | ||
| logger.info("Get fingerprint " + response.toString()); | ||
| return response.toString(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate unnecessary object copying and improve error handling.
The method creates an unnecessary copy of the UserBiometricsMapping object and has inconsistent HTTP status codes.
Apply this refactor:
@GetMapping(value = "/get/fingerprint/{userID}/wo")
public String getFingerprintsByUsername(@PathVariable("userID") Long userID) {
logger.info("Get fingerprint by username ..." + userID);
OutputResponse response = new OutputResponse();
+
+ if (userID == null || userID <= 0) {
+ response.setError(400, "Invalid user ID");
+ return response.toString();
+ }
+
UserBiometricsMapping user = registrarService.getFingerprintsByUserID(userID);
if (user != null) {
Gson gson = new Gson();
- UserBiometricsMapping userBiometricsMapping = new UserBiometricsMapping();
- userBiometricsMapping.setUserID(user.getUserID());
- userBiometricsMapping.setFirstName(user.getFirstName());
- userBiometricsMapping.setLastName(user.getLastName());
- userBiometricsMapping.setUserName(user.getUserName());
- userBiometricsMapping.setCreatedBy(user.getUserName());
- userBiometricsMapping.setRightThumb(user.getRightThumb());
- userBiometricsMapping.setRightIndexFinger(user.getRightIndexFinger());
- userBiometricsMapping.setLeftThumb(user.getLeftThumb());
- userBiometricsMapping.setLeftIndexFinger(user.getLeftIndexFinger());
-
- response.setResponse(gson.toJson(userBiometricsMapping));
+ response.setResponse(gson.toJson(user));
} else {
response.setError(404, "User with userID: " + userID + " not found");
}
logger.info("Get fingerprint " + response.toString());
return response.toString();
}📝 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.
| public String getFingerprintsByUsername(@PathVariable("userID") Long userID) { | |
| logger.info("Get fingerprint by username ..." + userID); | |
| OutputResponse response = new OutputResponse(); | |
| UserBiometricsMapping user = registrarService.getFingerprintsByUserID(userID); | |
| if (user != null) { | |
| Gson gson = new Gson(); | |
| UserBiometricsMapping userBiometricsMapping = new UserBiometricsMapping(); | |
| userBiometricsMapping.setUserID(user.getUserID()); | |
| userBiometricsMapping.setFirstName(user.getFirstName()); | |
| userBiometricsMapping.setLastName(user.getLastName()); | |
| userBiometricsMapping.setUserName(user.getUserName()); | |
| userBiometricsMapping.setCreatedBy(user.getUserName()); | |
| userBiometricsMapping.setRightThumb(user.getRightThumb()); | |
| userBiometricsMapping.setRightIndexFinger(user.getRightIndexFinger()); | |
| userBiometricsMapping.setLeftThumb(user.getLeftThumb()); | |
| userBiometricsMapping.setLeftIndexFinger(user.getLeftIndexFinger()); | |
| response.setResponse(gson.toJson(userBiometricsMapping)); | |
| } | |
| else{ | |
| response.setError(404, "User with userID: "+userID+" not found"); | |
| } | |
| logger.info("Get fingerprint " + response.toString()); | |
| return response.toString(); | |
| } | |
| response.setResponse(gson.toJson(userBiometricsMapping)); | |
| } else { | |
| response.setError(404, "User with userID: " + userID + " not found"); | |
| } | |
| logger.info("Get fingerprint " + response.toString()); | |
| return response.toString(); | |
| } | |
| @GetMapping(value = "/get/fingerprint/{userID}/wo") | |
| public String getFingerprintsByUsername(@PathVariable("userID") Long userID) { | |
| logger.info("Get fingerprint by username ..." + userID); | |
| OutputResponse response = new OutputResponse(); | |
| if (userID == null || userID <= 0) { | |
| response.setError(400, "Invalid user ID"); | |
| return response.toString(); | |
| } | |
| UserBiometricsMapping user = registrarService.getFingerprintsByUserID(userID); | |
| if (user != null) { | |
| Gson gson = new Gson(); | |
| response.setResponse(gson.toJson(user)); | |
| } else { | |
| response.setError(404, "User with userID: " + userID + " not found"); | |
| } | |
| logger.info("Get fingerprint " + response.toString()); | |
| return response.toString(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/hwc/controller/wo/UserBiomectrics.java between lines
76 and 99, eliminate the redundant copying of the UserBiometricsMapping object
by directly using the object returned from
registrarService.getFingerprintsByUserID. Also, improve error handling by
ensuring the HTTP status code in the response accurately reflects the error
condition, standardizing it to a consistent and appropriate value such as 404
for not found. Adjust the response construction accordingly to avoid unnecessary
object creation and maintain clear, consistent status reporting.
| private boolean isOriginAllowed(String origin) { | ||
| if (origin == null || allowedOrigins == null || allowedOrigins.trim().isEmpty()) { | ||
| logger.warn("No allowed origins configured or origin is null"); | ||
| return false; | ||
| } | ||
|
|
||
| return Arrays.stream(allowedOrigins.split(",")).map(String::trim) | ||
| .anyMatch(pattern -> origin.matches(pattern.replace(".", "\\.").replace("*", ".*"))); | ||
| } |
There was a problem hiding this comment.
Fix potential security vulnerability in regex pattern conversion.
The regex conversion logic has a security vulnerability - it doesn't properly escape special regex characters and could be exploited.
The current implementation:
.anyMatch(pattern -> origin.matches(pattern.replace(".", "\\.").replace("*", ".*")));Only escapes dots and asterisks, but other regex special characters like +, ?, ^, $, |, (, ), [, ], {, } are not escaped, potentially allowing regex injection.
Apply this fix:
private boolean isOriginAllowed(String origin) {
if (origin == null || allowedOrigins == null || allowedOrigins.trim().isEmpty()) {
logger.warn("No allowed origins configured or origin is null");
return false;
}
- return Arrays.stream(allowedOrigins.split(",")).map(String::trim)
- .anyMatch(pattern -> origin.matches(pattern.replace(".", "\\.").replace("*", ".*")));
+ return Arrays.stream(allowedOrigins.split(","))
+ .map(String::trim)
+ .anyMatch(pattern -> {
+ if (pattern.equals("*")) {
+ return true; // Allow all origins
+ }
+ // Escape all regex special characters except * which we want to use as wildcard
+ String escapedPattern = pattern.replaceAll("([\\[\\](){}+?^$|\\\\.])", "\\\\$1")
+ .replace("*", ".*");
+ return origin.matches(escapedPattern);
+ });
}🤖 Prompt for AI Agents
In src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java around lines
121 to 129, the method isOriginAllowed improperly escapes regex special
characters when converting allowed origin patterns to regex, which can lead to
security vulnerabilities. To fix this, replace the manual string replacements
with a proper escaping method that escapes all regex special characters except
the wildcard '*', then convert '*' to '.*' for matching. This ensures the
pattern is safely converted to a regex without allowing injection.
vishwab1
changed the title
📋 Description
JIRA ID: AMM 593
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
Added cors orign method to restrict from the browsers
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit