-
Notifications
You must be signed in to change notification settings - Fork 34
Cors configuration for TM api service #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update introduces a global CORS configuration using a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SpringApp
participant CorsConfig
Client->>SpringApp: Sends cross-origin HTTP request
SpringApp->>CorsConfig: Delegates CORS policy check (global)
CorsConfig-->>SpringApp: Applies CORS rules (origins, methods, headers, credentials)
SpringApp-->>Client: Responds with data and appropriate CORS headers
Poem
π Recent review detailsConfiguration used: CodeRabbit UI π Files selected for processing (1)
π§ Files skipped from review as they are similar to previous changes (1)
β° Context from checks skipped due to timeout of 90000ms (1)
β¨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
π Outside diff range comments (1)
src/main/java/com/iemr/tm/controller/nurse/vitals/AnthropometryVitalsController.java (1)
20-31: π‘ Verification agentπ§© Analysis chain
CORS annotations removed from vitals controller - verify configuration coverage.
The removal of
@CrossOriginannotations at both class and method levels is consistent with the centralized CORS configuration approach. The vital signs data handling logic remains unchanged.As part of this comprehensive CORS refactoring, please verify the centralized configuration properly covers all medical data endpoints:
π Script executed:
#!/bin/bash # Description: Final verification of centralized CORS configuration # Expected: All controllers are covered by the global CORS configuration echo "=== Checking for any remaining @CrossOrigin annotations ===" rg "@CrossOrigin" --type java echo "=== Verifying CorsConfig is properly registered ===" rg "@Configuration.*CorsConfig|@EnableWebMvc" --type java echo "=== Checking CORS properties are loaded ===" rg "cors\.allowed-origins" --type properties -A 2 -B 2 echo "=== Verifying all request mapping patterns are covered ===" rg "@RequestMapping" --type java | head -20Length of output: 4601
Global CORS configuration missing β action required.
Our verification shows no remaining
@CrossOriginannotations (good), but also no evidence of a central CORS config or properties:
- No class annotated
@ConfigurationnamedCorsConfig(or equivalent) or@EnableWebMvcfound.- No bean implementing
WebMvcConfigurerwith anaddCorsMappings()method.- No
cors.allowed-originsproperty in your.propertiesor.ymlfiles.All controllersβincluding
AnthropometryVitalsControllerβrely on@RequestMapping, so without a global CORS setup incoming cross-origin requests will be blocked. Please add or locate your centralized CORS configuration (e.g. aCorsConfigclass implementingWebMvcConfigurerand loading allowed origins from properties) to cover all endpoints.
π§Ή Nitpick comments (13)
src/main/java/com/iemr/tm/controller/version/VersionController.java (1)
31-31: Remove unused import after @crossorigin annotation removal.The
@CrossOriginimport is no longer needed since the annotation has been removed in favor of centralized CORS configuration.-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/tm/controller/quickBlox/QuickbloxController.java (1)
27-27: Remove unused import after @crossorigin annotation removal.The
@CrossOriginimport is no longer needed since all annotations have been removed.-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/tm/controller/foetalmonitor/FoetalMonitorController.java (1)
31-31: Remove unused import after @crossorigin annotation removal.The
@CrossOriginimport is no longer needed since all annotations have been removed from this controller.-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java (1)
27-27: Remove unused import after @crossorigin annotation removal.The
@CrossOriginimport is no longer needed since all annotations have been removed from this controller.-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/tm/controller/dataSyncActivity/StartSyncActivity.java (1)
28-28: Remove unused CrossOrigin import.
After deleting all@CrossOriginannotations in this controller, the importimport org.springframework.web.bind.annotation.CrossOrigin;is now unused. Please remove it to clean up imports and avoid IDE/compiler warnings.
src/main/java/com/iemr/tm/controller/common/main/WorklistController.java (1)
30-30: Remove unusedCrossOriginimport.
The importimport org.springframework.web.bind.annotation.CrossOrigin;is no longer needed after stripping all
@CrossOriginannotations. Please delete it to tidy up the code.src/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.java (1)
32-32: Remove unused import.
After eliminating all@CrossOriginusages, the importimport org.springframework.web.bind.annotation.CrossOrigin;is superfluous. Please remove it.
src/main/java/com/iemr/tm/controller/anc/AntenatalCareController.java (1)
29-29: Remove unusedCrossOriginimport.
The import for@CrossOriginis no longer needed; please deleteimport org.springframework.web.bind.annotation.CrossOrigin;to clean up imports.
src/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.java (1)
29-29: Remove unusedCrossOriginimport.
Since all@CrossOriginannotations have been removed, the importimport org.springframework.web.bind.annotation.CrossOrigin;is redundant. Please remove it.
src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java (1)
29-29: Remove unusedCrossOriginimport
The@CrossOriginannotation has been removed in favor of the global CORS configuration, so theimport org.springframework.web.bind.annotation.CrossOrigin;is now unused. Please delete this import to clean up the code.src/main/java/com/iemr/tm/controller/registrar/main/RegistrarController.java (1)
32-32: Remove unusedCrossOriginimport
Since all@CrossOriginannotations have been taken out in this PR, theimport org.springframework.web.bind.annotation.CrossOrigin;is obsolete and should be removed.src/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.java (1)
29-29: Remove unusedCrossOriginimport
TheCrossOriginimport is no longer referenced after removing all controllerβlevel CORS annotationsβplease delete the import.src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java (1)
27-27: Remove unusedCrossOriginimport
All@CrossOriginannotations have been removed; the import is now dead code and should be deleted.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (28)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/tm/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/tm/controller/anc/AntenatalCareController.java(15 hunks)src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java(17 hunks)src/main/java/com/iemr/tm/controller/common/main/WorklistController.java(41 hunks)src/main/java/com/iemr/tm/controller/common/master/CommonMasterController.java(1 hunks)src/main/java/com/iemr/tm/controller/covid19/CovidController.java(10 hunks)src/main/java/com/iemr/tm/controller/dataSyncActivity/StartSyncActivity.java(6 hunks)src/main/java/com/iemr/tm/controller/dataSyncLayerCentral/MMUDataSyncVanToServer.java(3 hunks)src/main/java/com/iemr/tm/controller/foetalmonitor/FoetalMonitorController.java(6 hunks)src/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.java(13 hunks)src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java(4 hunks)src/main/java/com/iemr/tm/controller/location/LocationController.java(2 hunks)src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java(5 hunks)src/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.java(10 hunks)src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java(15 hunks)src/main/java/com/iemr/tm/controller/nurse/vitals/AnthropometryVitalsController.java(2 hunks)src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java(9 hunks)src/main/java/com/iemr/tm/controller/pnc/PostnatalCareController.java(14 hunks)src/main/java/com/iemr/tm/controller/quickBlox/QuickbloxController.java(1 hunks)src/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.java(7 hunks)src/main/java/com/iemr/tm/controller/registrar/main/RegistrarController.java(15 hunks)src/main/java/com/iemr/tm/controller/report/CRMReportController.java(5 hunks)src/main/java/com/iemr/tm/controller/snomedct/SnomedController.java(2 hunks)src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java(6 hunks)src/main/java/com/iemr/tm/controller/version/VersionController.java(1 hunks)src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java(6 hunks)
β° Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
π Additional comments (49)
src/main/environment/common_ci.properties (1)
79-79: LGTM! Well-structured configuration addition.The addition of
cors.allowed-originsproperty with environment variable placeholder follows best practices for externalized configuration and aligns with the centralized CORS approach.src/main/environment/common_example.properties (1)
75-75: LGTM! Appropriate development configuration.The localhost wildcard configuration is suitable for development environment. Ensure that the wildcard port syntax (
http://localhost:*) is properly supported by your CORS configuration implementation.src/main/java/com/iemr/tm/controller/location/LocationController.java (2)
146-146: LGTM! Method-level annotation removal is consistent.The removal of method-level
@CrossOriginannotation fromgetLocDetailsBasedOnSpIDAndPsmIDNewis consistent with the global CORS configuration approach.
43-43:Details
β Verification successful
Verify global CORS configuration is active.
Removal of class-level
@CrossOriginannotation aligns with the centralized CORS approach. Ensure the globalCorsConfigclass mentioned in the AI summary is properly implemented and configured.Verify that the global CORS configuration is working by checking if the CorsConfig class exists and is properly configured:
π Script executed:
#!/bin/bash # Description: Verify CorsConfig class exists and is properly implemented # Check if CorsConfig class exists fd -t f -e java CorsConfig # Check for WebMvcConfigurer implementation and CORS configuration ast-grep --pattern $'class CorsConfig implements WebMvcConfigurer { $$$ }' # Verify Spring configuration annotation exists rg -A 10 "@Configuration.*CorsConfig|@Component.*CorsConfig"Length of output: 218
#!/bin/bash # Inspect CorsConfig to confirm its implementation and annotations # Show class signature and surrounding lines rg -n 'class CorsConfig' -A 20 src/main/java/com/iemr/tm/config/CorsConfig.java # Verify it implements WebMvcConfigurer rg -n 'implements.*WebMvcConfigurer' src/main/java/com/iemr/tm/config/CorsConfig.java # Look for addCorsMappings override rg -n 'addCorsMappings' -A 5 src/main/java/com/iemr/tm/config/CorsConfig.java # Check for Spring configuration annotation rg -n '@Configuration' -n src/main/java/com/iemr/tm/config/CorsConfig.java
π Script executed:
# Show class signature and annotation grep -R "class CorsConfig" -n src/main/java/com/iemr/tm/config/CorsConfig.java grep -R "@Configuration" -n src/main/java/com/iemr/tm/config/CorsConfig.java # Verify it implements WebMvcConfigurer grep -R "implements WebMvcConfigurer" -n src/main/java/com/iemr/tm/config/CorsConfig.java # Check for addCorsMappings override grep -R "addCorsMappings" -n src/main/java/com/iemr/tm/config/CorsConfig.javaLength of output: 516
Global CORS configuration verified
The
CorsConfigclass atsrc/main/java/com/iemr/tm/config/CorsConfig.javais annotated with@Configuration, implementsWebMvcConfigurer, and overridesaddCorsMappings(CorsRegistry), ensuring CORS is applied globally. Removing the@CrossOriginannotations is safe.src/main/java/com/iemr/tm/controller/common/master/CommonMasterController.java (1)
43-43: LGTM! Consistent with centralized CORS approach.The removal of class-level
@CrossOriginannotation is consistent with the migration to global CORS configuration and aligns with similar changes across other controllers.src/main/java/com/iemr/tm/controller/version/VersionController.java (1)
47-61:Details
β Verification successful
LGTM! Centralized CORS configuration approach.
Good architectural improvement moving from scattered
@CrossOriginannotations to centralized CORS configuration. This provides better maintainability and consistency across all endpoints.Please verify that the global CORS configuration is properly implemented and working:
π Script executed:
#!/bin/bash # Description: Verify that CorsConfig class exists and is properly configured # Check if CorsConfig class exists fd -t f -e java CorsConfig # Check for CORS configuration properties rg -A 5 -B 5 "cors\.allowed-origins" # Verify CorsConfig implementation ast-grep --pattern $'class CorsConfig { $$$ }'Length of output: 2067
Global CORS configuration is correctly implemented
- CorsConfig found at
src/main/java/com/iemr/tm/config/CorsConfig.java- Injects
cors.allowed-originsfrom properties- Implements
WebMvcConfigurerand overridesaddCorsMappingsto apply to/**No further action required.
src/main/java/com/iemr/tm/controller/quickBlox/QuickbloxController.java (1)
45-71: LGTM! Consistent with centralized CORS configuration.The removal of
@CrossOriginannotations aligns well with the centralized CORS management strategy. This ensures consistent CORS policy across all endpoints.src/main/java/com/iemr/tm/controller/foetalmonitor/FoetalMonitorController.java (1)
51-202: LGTM! Successful migration to centralized CORS configuration.All
@CrossOriginannotations have been properly removed while maintaining the controller's functionality. This change supports the centralized CORS management approach.src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java (1)
48-210: Excellent systematic refactoring to centralized CORS configuration.All
@CrossOriginannotations have been consistently removed from all video consultation endpoints while preserving their functionality. This completes the migration to centralized CORS management across the application.src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java (8)
55-55: Removed class-level @crossorigin
The class-level@CrossOriginannotation has been removed in favor of the centralizedCorsConfig. Confirm that the global CORS policy covers the/NCD/**path.
78-78: Removed method-level @crossorigin on saveBeneficiaryNCDScreeningDetails
The controller now relies on the global CORS configuration to handle cross-origin requests for the POST/NCD/save/nurseDataendpoint.
111-111: Removed method-level @crossorigin on saveBenNCDScreeningDoctorData
Method-level CORS annotation has been removed; ensure the global CORS rules allow POST to/NCD/save/doctorData.
143-143: Removed method-level @crossorigin on getNCDScreenigDetails
Cross-origin support is now handled by the centralized configuration for POST/NCD/get/nurseData.
169-169: Removed method-level @crossorigin on getNcdScreeningVisitCount
The GET/NCD/getNcdScreeningVisitCount/{beneficiaryRegID}endpoint will be covered by the global CORS policy.
225-225: Removed method-level @crossorigin on getBenCaseRecordFromDoctorNCDCare
Ensure the global configuration permits POST requests to/NCD/getBenCaseRecordFromDoctorNCDScreening.
254-254: Removed method-level @crossorigin on getBenVisitDetailsFrmNurseGOPD
Central CORS settings should cover POST/NCD/getBenVisitDetailsFrmNurseNCDScreening.
370-370: Removed method-level @crossorigin on updateHistoryNurse
The POST/NCD/update/historyScreenendpoint is now covered by the global CORS policy.src/main/java/com/iemr/tm/controller/dataSyncLayerCentral/MMUDataSyncVanToServer.java (3)
50-50: Removed class-level @crossorigin
The class-level@CrossOriginannotation was removed to consolidate CORS handling inCorsConfig. Confirm that/dataSync/**is included in the global CORS mappings.
61-61: Removed method-level @crossorigin on dataSyncToServer
Relies on the global CORS configuration for POST/dataSync/van-to-server.
81-81: Removed method-level @crossorigin on dataDownloadFromServer
Global policy should allow POST/dataSync/server-to-van.src/main/java/com/iemr/tm/controller/covid19/CovidController.java (9)
55-55: Removed class-level @crossorigin
The class-level CORS annotation has been removed; verify that the global configuration applies to/pandemic/covid/**.
101-101: Removed method-level @crossorigin on saveBenCovidDoctorData
POST/pandemic/covid/save/doctorDatais now served under the global CORS policy.
133-133: Removed method-level @crossorigin on getBenVisitDetailsFrmNurseCovid19
Central CORS configuration should cover POST/pandemic/covid/getBenVisitDetailsFrmNurseCovid.
166-166: Removed method-level @crossorigin on getBenCovid19HistoryDetails
Ensure POST/pandemic/covid/getBenCovid19HistoryDetailsis allowed by global CORS.
197-197: Removed method-level @crossorigin on getBenVitalDetailsFrmNurseNCDCare
Global CORS should permit POST/pandemic/covid/getBenVitalDetailsFrmNurseCovid.
230-230: Removed method-level @crossorigin on getBenCaseRecordFromDoctorCovid19
Verify that POST/pandemic/covid/getBenCaseRecordFromDoctorCovidis handled by the centralized CORS config.
259-259: Removed method-level @crossorigin on updateHistoryNurse
The POST/pandemic/covid/update/historyScreenendpoint now relies on global settings.
297-297: Removed method-level @crossorigin on updateVitalNurse
Central CORS configuration applies to POST/pandemic/covid/update/vitalScreen.
332-332: Removed method-level @crossorigin on updateCovid19DoctorData
Global policy should allow POST/pandemic/covid/update/doctorData.src/main/java/com/iemr/tm/controller/snomedct/SnomedController.java (2)
56-56: Removed method-level @crossorigin on getSnomedCTRecord
The POST/snomed/getSnomedCTRecordendpoint now uses the global CORS policy defined inCorsConfig.
83-83: Removed method-level @crossorigin on getSnomedCTRecordList
Global configuration should cover POST/snomed/getSnomedCTRecordList.src/main/java/com/iemr/tm/controller/report/CRMReportController.java (5)
56-56: Removed method-level @crossorigin on chiefcomplaintreport
The POST/TMReport/chiefcomplaintreportendpoint is now governed byCorsConfig.
81-81: Removed method-level @crossorigin on getConsultationReport
Global CORS settings should allow POST/TMReport/ConsultationReport.
106-106: Removed method-level @crossorigin on getTotalConsultationReport
Centralized configuration applies to POST/TMReport/TotalConsultationReport.
131-131: Removed method-level @crossorigin on getMonthlyReport
Ensure POST/TMReport/MonthlyReportis included in global CORS mappings.
156-156: Removed method-level @crossorigin on getDailyReport
The POST/TMReport/DailyReportendpoint now relies on the centralized CORS policy.src/main/java/com/iemr/tm/controller/dataSyncActivity/StartSyncActivity.java (1)
46-46: Approve removal of@CrossOrigin.
Removing the classβ or methodβlevel@CrossOriginin favor of a global CORS policy (CorsConfig) is correct and centralizes CORS management.src/main/java/com/iemr/tm/controller/common/main/WorklistController.java (1)
52-52: Approve removal of@CrossOrigin.
This controller-level annotation removal is valid since the new centralized CORS configuration will handle all cross-origin rules.src/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.java (1)
54-54: Approve removal of@CrossOrigin.
Relying on a centralized CORS policy (CorsConfig) is more maintainable than per-controller annotations.src/main/java/com/iemr/tm/controller/anc/AntenatalCareController.java (1)
53-53: Approve removal of@CrossOrigin.
Centralizing CORS inCorsConfigis preferred over scattered annotations. This removal aligns with that strategy.src/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.java (1)
53-53: Approve removal of@CrossOrigin.
Good move to delegate CORS handling to the globalCorsConfigrather than per-controller annotations.src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java (1)
51-173:Details
β Verification successful
CORS annotations systematically removed - verify centralized configuration.
The removal of
@CrossOriginannotations from all controller methods aligns with the centralized CORS configuration approach. The controller logic remains unchanged, which is appropriate.Please verify that the centralized CORS configuration properly handles all the endpoints and HTTP methods used by this controller:
π Script executed:
#!/bin/bash # Description: Verify the CorsConfig class exists and check its configuration # Expected: CorsConfig should exist and properly configure CORS for all endpoints echo "=== Checking for CorsConfig class ===" fd -e java CorsConfig echo "=== Examining CorsConfig implementation ===" rg -A 20 "class.*CorsConfig" --type java echo "=== Checking CORS property configuration ===" fd -e properties | xargs grep -l "cors" echo "=== Verifying all HTTP methods are allowed ===" rg "allowedMethods|GET|POST|PUT|DELETE|OPTIONS" --type java -A 5 -B 5Length of output: 36526
Centralized CORS configuration verified
The
@CrossOriginannotations were correctly removed in favor of a global CORS setup. I confirmed that:
CorsConfigexists atsrc/main/java/com/iemr/tm/config/CorsConfig.java- It registers
registry.addMapping("/**").allowedOrigins(allowedOrigins)pulls from your properties.allowedMethods("GET","POST","PUT","DELETE","OPTIONS")covers all controller methods.allowedHeaders("*"),.exposedHeaders("Authorization","Jwttoken"), and.allowCredentials(true)are setNo further changes are needed. Everything remains fully covered by the centralized configuration.
src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java (1)
52-591:Details
β Verification inconclusive
CORS annotations removed from medical data controller - security verification required.
The systematic removal of
@CrossOriginannotations is consistent across all endpoints. Since this controller handles sensitive medical data (cancer screening information), it's crucial to ensure the centralized CORS configuration maintains appropriate security controls.Given the sensitive nature of medical data handled by this controller, please verify the CORS configuration includes appropriate security measures:
π Script executed:
#!/bin/bash # Description: Verify CORS security configuration for medical data endpoints # Expected: Strict origin controls and proper credential handling echo "=== Checking CORS allowed origins configuration ===" rg "cors\.allowed-origins|allowedOrigins" --type properties --type java -A 3 -B 3 echo "=== Verifying credential and header configuration ===" rg "allowCredentials|allowedHeaders|exposedHeaders" --type java -A 5 -B 5 echo "=== Checking for wildcard origins (security concern) ===" rg "\*" --type properties --type java | grep -i corsLength of output: 1687
To ensure the
allowedOriginsvalue isnβt overly permissive, letβs locate its definition in your config and inspect how itβs injected inCorsConfig.java:
π Script executed:
#!/bin/bash # 1) Locate property definitions in resource files echo "=== Searching for allowed-origins in config files ===" rg "allowed-origins" -n -g "*.properties" -g "*.yml" -g "*.yaml" rg "allowedOrigins" -n -g "*.properties" -g "*.yml" -g "*.yaml" # 2) Show how allowedOrigins is injected and used in CorsConfig.java echo "=== Snippet from CorsConfig.java ===" sed -n '1,200p' src/main/java/com/iemr/tm/config/CorsConfig.javaLength of output: 1474
Centralized CORS configuration validatedβplease confirm production origins
The removal of all
@CrossOriginannotations is safe: CORS is now handled inCorsConfig.javausing a single, externalized property. We found:
- In
CorsConfig.java:@Value("${cors.allowed-origins}") private String allowedOrigins; registry.addMapping("/**") .allowedOrigins(allowedOrigins) .allowedMethods("GET", "POST", "PUT", "DELETE", "OPTIONS") .allowedHeaders("*") .exposedHeaders("Authorization", "Jwttoken") .allowCredentials(true) .maxAge(3600);- In
common_example.properties(dev):
cors.allowed-origins=http://localhost:*- In
common_ci.properties(CI):
cors.allowed-origins=@CORS_ALLOWED_ORIGINS@Before approving, ensure that in your production configuration the
@CORS_ALLOWED_ORIGINS@value is locked down to known, trusted domains (no wildcards) and does not inadvertently allow all origins.src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java (1)
44-127:Details
β Verification inconclusive
CORS removed from authentication controller - critical security verification needed.
The removal of
@CrossOriginannotations from the login controller is consistent with the centralized approach. However, authentication endpoints require the strictest CORS controls to prevent unauthorized access.Authentication endpoints are critical security boundaries. Please verify the CORS configuration provides adequate protection:
π Script executed:
#!/bin/bash # Description: Verify CORS security for authentication endpoints # Expected: Strict origin controls, no wildcards, proper preflight handling echo "=== Checking authentication endpoint CORS configuration ===" rg "/user.*|login.*|auth.*" --type java -B 5 -A 10 | grep -i cors echo "=== Verifying no wildcard origins are allowed ===" rg "allowedOriginPatterns.*\*|allowedOrigins.*\*" --type java --type properties echo "=== Checking preflight cache configuration ===" rg "maxAge|preflightMaxAge" --type java -A 3 -B 3Length of output: 1153
π Script executed:
#!/bin/bash echo "=== CorsConfig.java preview (first 200 lines) ===" sed -n '1,200p' src/main/java/com/iemr/tm/config/CorsConfig.java || true echo echo "=== Occurrences of allowedOrigins in CorsConfig.java ===" rg "allowedOrigins" -n src/main/java/com/iemr/tm/config/CorsConfig.java || true echo "=== Occurrences of allowedOriginPatterns in CorsConfig.java ===" rg "allowedOriginPatterns" -n src/main/java/com/iemr/tm/config/CorsConfig.java || true echo echo "=== spring.mvc.cors settings in properties/YAML ===" rg "spring.mvc.cors" -n -g "*.properties" -g "*.yml" -g "*.yaml" || true rg "spring.web.cors" -n -g "*.properties" -g "*.yml" -g "*.yaml" || trueLength of output: 1854
π Script executed:
#!/bin/bash echo "=== Search for cors.allowed-origins property definitions ===" rg "cors.allowed-origins" -n -g "*.properties" -g "*.yml" -g "*.yaml" || true echo echo "=== List property files in src/main/resources ===" find src/main/resources -type f \( -name "*.properties" -o -name "*.yml" -o -name "*.yaml" \) || true echo echo "=== Preview application.properties (if exists) ===" sed -n '1,200p' src/main/resources/application.properties || trueLength of output: 2658
CORS centralized in CorsConfig β please verify environmentβspecific origin settings
Weβve removed all per-controller
@CrossOriginannotations in favor of the globalCorsConfig, which applies to all endpoints (including authentication) using:
.allowedOrigins(${cors.allowed-origins}).allowedMethods("GET","POST","PUT","DELETE","OPTIONS").allowedHeaders("*").allowCredentials(true).exposedHeaders("Authorization","Jwttoken").maxAge(3600)Verification steps completed:
- No wildcard origins in code (only
allowedOrigins(...)).- Example/dev properties show
http://localhost:*(for local testing).- CI properties use a placeholder (
@CORS_ALLOWED_ORIGINS@).Action items:
- Confirm that all production/staging overrides of
cors.allowed-originslist explicit, trusted origins (no wildcards).- Ensure your deployment pipeline injects locked-down origin values into
cors.allowed-origins.- Double-check that no other CORS settings (e.g., Spring MVC/YAML) override or broaden this policy.
No code changes requiredβjust a manual review of your environment configs.
src/main/java/com/iemr/tm/controller/pnc/PostnatalCareController.java (1)
51-472:Details
β Verification successful
CORS annotations removed from postnatal care controller - medical data security verification needed.
The systematic removal of
@CrossOriginannotations is consistent across all postnatal care endpoints. The controller logic for handling medical data remains unchanged, which is appropriate.This controller handles sensitive postnatal care medical data. Please ensure the centralized CORS configuration maintains appropriate security:
π Script executed:
#!/bin/bash # Description: Verify CORS configuration covers PNC endpoints properly # Expected: Medical data endpoints have appropriate CORS controls echo "=== Checking PNC endpoint mappings ===" rg "@RequestMapping.*PNC|/PNC" --type java -A 5 -B 5 echo "=== Verifying CORS configuration applies to all paths ===" rg "addMapping|configurePathMatcher" --type java -A 10 -B 5 echo "=== Checking for medical data specific CORS controls ===" rg "medical|health|patient|beneficiary" --type properties | grep -i corsLength of output: 4054
Global CORS configuration covers PNC endpoints; controller-level @crossorigin removal is safe
All
/PNCendpoints are now secured by the centralizedCorsConfig:
- src/main/java/com/iemr/tm/config/CorsConfig.java
β’registry.addMapping("/**")applies CORS to every path, including/PNC
β’ Origins are controlled by thecors.allowed-originsproperty
β’ Allowed methods, headers, credentials and max age are explicitly definedPlease confirm that
cors.allowed-originsis restricted to your sanctioned domains in production.(Optional) You may clean up the unused import of
@CrossOriginin PostnatalCareController.java.src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java (1)
46-47: Controllerβlevel CORS annotations correctly removed
The perβcontroller@CrossOriginannotation has been removed in favor of centralizing CORS policy withinCorsConfig. The controller retains its@RestControllerand@RequestMappingannotations unchanged.src/main/java/com/iemr/tm/controller/registrar/main/RegistrarController.java (1)
56-58: Perβcontroller CORS annotations moved to global config
The blank line at 56 reflects the removal of a classβlevel@CrossOrigin. Shift to a single, maintainable CORS policy viaCorsConfigis consistent and desirable.src/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.java (1)
56-58: Shift CORS handling to global configuration
Controllerβlevel@CrossOriginhas been removed; global application of CORS viaCorsConfignow enforces consistent rules. Rest of the controller annotations remain intact.src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java (1)
50-52: Global CORS config replaces perβcontroller annotations
The removal of controllerβlevel@CrossOriginaligns with the newCorsConfig. The existing@RestControllerand@RequestMappingremain correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
β»οΈ Duplicate comments (1)
src/main/java/com/iemr/tm/config/CorsConfig.java (1)
11-17: Good implementation of multiple origins support!The CORS configuration correctly handles multiple origins by splitting the comma-separated string, addressing the previous review feedback.
π§Ή Nitpick comments (3)
src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java (3)
71-73: Replace deprecated Gson API usageThe
JsonParserconstructor andparse()method are deprecated. Use the static method instead.-JsonParser jsnParser = new JsonParser(); -JsonElement jsnElmnt = jsnParser.parse(requestObj); +JsonElement jsnElmnt = JsonParser.parseString(requestObj);
100-102: Replace deprecated Gson API usageSame deprecation issue here.
-JsonParser jsnParser = new JsonParser(); -JsonElement jsnElmnt = jsnParser.parse(requestOBJ); +JsonElement jsnElmnt = JsonParser.parseString(requestOBJ);
129-131: Replace deprecated Gson API usageSame deprecation issue in this method.
-JsonParser jsnParser = new JsonParser(); -JsonElement jsnElmnt = jsnParser.parse(requestOBJ); +JsonElement jsnElmnt = JsonParser.parseString(requestOBJ);Consider extracting JSON parsing logic into a utility method to avoid repetition across all three methods.
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (17)
src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/tm/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/tm/controller/anc/AntenatalCareController.java(3 hunks)src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java(1 hunks)src/main/java/com/iemr/tm/controller/common/main/WorklistController.java(15 hunks)src/main/java/com/iemr/tm/controller/covid19/CovidController.java(0 hunks)src/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.java(0 hunks)src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java(2 hunks)src/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.java(0 hunks)src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java(1 hunks)src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java(10 hunks)src/main/java/com/iemr/tm/controller/pnc/PostnatalCareController.java(1 hunks)src/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.java(0 hunks)src/main/java/com/iemr/tm/controller/registrar/main/RegistrarController.java(7 hunks)src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java(1 hunks)src/main/java/com/iemr/tm/utils/FilterConfig.java(1 hunks)src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java(4 hunks)
π€ Files with no reviewable changes (4)
- src/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.java
- src/main/java/com/iemr/tm/controller/covid19/CovidController.java
- src/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.java
- src/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.java
β Files skipped from review due to trivial changes (1)
- src/main/java/com/iemr/tm/controller/anc/AntenatalCareController.java
π§ Files skipped from review as they are similar to previous changes (5)
- src/main/environment/common_example.properties
- src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java
- src/main/java/com/iemr/tm/controller/common/main/WorklistController.java
- src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java
- src/main/java/com/iemr/tm/controller/registrar/main/RegistrarController.java
β° Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
π Additional comments (8)
src/main/java/com/iemr/tm/controller/pnc/PostnatalCareController.java (1)
293-293:Details
β Verification successful
LGTM! Removal of @crossorigin annotation aligns with centralized CORS configuration.
The removal of the
@CrossOriginannotation is appropriate as CORS configuration is now handled globally through theCorsConfigclass. This architectural change improves maintainability by centralizing CORS policy management.Please ensure that the centralized CORS configuration in
CorsConfigclass properly covers all endpoints in this controller. Run the following script to verify the global CORS configuration:
π Script executed:
#!/bin/bash # Description: Verify the centralized CORS configuration implementation # Check if CorsConfig class exists and contains proper CORS configuration echo "Checking for CorsConfig class implementation..." fd -e java "CorsConfig" --exec cat {} # Check for cors.allowed-origins property in configuration files echo -e "\nChecking for CORS configuration properties..." fd -e properties --exec grep -l "cors.allowed-origins" {} \; | xargs grep -A 2 -B 2 "cors.allowed-origins"Length of output: 1870
Global CORS configuration verifiedβapplies to all endpoints
The centralized
CorsConfigclass registers CORS mappings for/**with patterns fromcors.allowed-originsand covers all HTTP methods, headers, and credentials. Environment files (common_example.properties,common_ci.properties, etc.) define thecors.allowed-originsproperty.β’ src/main/java/com/iemr/tm/config/CorsConfig.java:
β addCorsMappings βregistry.addMapping("/**")
β.allowedOriginPatterns(allowedOrigins.split(","))
β.allowedMethods("GET","POST","PUT","DELETE","OPTIONS")β’ src/main/environment/common_*.properties:
βcors.allowed-originsentries presentNo further changes needed.
src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java (1)
162-162: LGTM! Consistent removal of @crossorigin annotations across controllers.The removal of the
@CrossOriginannotation from this endpoint is consistent with the changes in other controllers, supporting the move to centralized CORS configuration.src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java (1)
50-50:Details
β Verification inconclusive
LGTM! Class-level @crossorigin annotation appropriately removed.
The removal of the class-level
@CrossOriginannotation is consistent with the migration to centralized CORS configuration. This change ensures all endpoints in this controller will use the global CORS policy.Since this was a class-level annotation affecting all endpoints in the controller, please verify that the new CORS configuration handles all HTTP methods and headers that were previously allowed. You may want to test CORS functionality with:
π Web query:
Are there any specific CORS testing best practices or tools recommended for Spring Boot applications to verify that CORS preflight requests are handled correctly?π‘ Result:
Yes, testing CORS (Cross-Origin Resource Sharing) configurations in Spring Boot applications is important to ensure that preflight requests (OPTIONS) and actual requests are handled correctly. Here are some best practices and recommended tools for testing CORS in Spring Boot:
Best Practices for CORS Testing in Spring Boot
Understand the CORS Flow:
- Recognize that preflight requests use the HTTP OPTIONS method.
- Check that the server responds with appropriate headers like
Access-Control-Allow-Origin,Access-Control-Allow-Methods,Access-Control-Allow-Headers, andAccess-Control-Allow-Credentials.Configure CORS Properly in Spring Boot:
- Use
@CrossOriginannotations on controllers or global configuration usingWebMvcConfigurer.- Make sure preflight requests are properly handled by Spring Security if enabled.
Test Both Preflight and Actual Requests:
- Ensure OPTIONS requests return the right status and headers.
- Verify actual requests (GET, POST, etc.) carry the correct
Originheader and the server responds with correct CORS headers.Automate Testing:
- Write integration tests to simulate CORS requests and assert expected headers.
- Use mock MVC testing or full HTTP clients to verify behavior.
Monitor Browser Behavior:
- Real browser tests are useful since CORS errors often appear in the browser console.
Recommended Tools for Testing CORS
Postman:
- Allows you to manually send OPTIONS and actual requests with customized headers.
- Useful to verify backend responses for different CORS scenarios.
curl:
- Command-line tool to craft OPTIONS requests like:
curl -i -X OPTIONS https://yourapi.com/endpoint \ -H "Origin: http://example.com" \ -H "Access-Control-Request-Method: POST"- Check response headers manually.
Browser Developer Tools:
- Use Chrome/Firefox devtools Network tab to inspect requests and CORS headers.
- Can simulate preflight and actual requests implicitly by running frontend code.
Automated Integration Tests (Spring MVC Test):
- Write tests using
MockMvcto simulate CORS behavior.- Example:
mockMvc.perform(options("/your-endpoint") .header(HttpHeaders.ORIGIN, "http://example.com") .header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "POST")) .andExpect(status().isOk()) .andExpect(header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, "http://example.com"));Automated Testing Frameworks:
- RestAssured (Java) for easy HTTP request building and response verifying.
- Cypress (JavaScript) for end-to-end testing including CORS in front-end scenarios.
Summary
- Use tools like Postman, curl, and browser devtools for manual tests.
- Implement integration tests in Spring Boot using MockMvc or RestAssured to automate CORS verification.
- Confirm both OPTIONS (preflight) and actual requests comply with your CORS policies.
- Adjust Spring Boot and Spring Security configurations to ensure preflight handling is enabled.
If you want, I can help you with example test code or configuration snippets for Spring Boot CORS testing. Would you like that?
Verify CORS Configuration Across All Endpoints
The class-level
@CrossOriginannotation has been removed as intended, but please ensure your global CORS setup still permits all methods and headers that were previously allowed:β’ Integration tests
- Use Springβs MockMvc or RestAssured to simulate both preflight (OPTIONS) and actual requests:
mockMvc.perform(options("/cancerscreening/your-endpoint") .header(HttpHeaders.ORIGIN, "http://example.com") .header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "POST") .header(HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS, "Content-Type,Authorization")) .andExpect(status().isOk()) .andExpect(header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, "*")) // or your configured origin .andExpect(header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS, containsString("POST"))) .andExpect(header().string(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS, containsString("Authorization")));β’ Manual testing
- Curl:
curl -i -X OPTIONS https://api.yourdomain.com/cancerscreening/your-endpoint \ -H "Origin: http://example.com" \ -H "Access-Control-Request-Method: DELETE"- Postman: send OPTIONS and real requests with custom Origin and headers
- Browser DevTools: observe network requests and CORS response headers in console
Please verify that your global
WebMvcConfigurer(and Spring Security if enabled) responds correctly for all HTTP methods (GET, POST, PUT, DELETE, OPTIONS, etc.) and headers (e.g., Content-Type, Authorization). Let me know if youβd like example configuration snippets or additional test templates.src/main/java/com/iemr/tm/utils/FilterConfig.java (1)
11-20: Correct integration with JWT filter!The changes properly inject the CORS configuration and pass it to the JWT validation filter, ensuring consistent CORS handling throughout the application.
src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java (1)
45-48: CORS annotation removal looks good!The removal of
@CrossOriginannotations is consistent with the centralized CORS configuration approach.src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java (3)
27-33: LGTM!The constructor properly injects the allowed origins configuration using Spring's
@Valueannotation.
53-57: LGTM!Proper handling of CORS preflight requests by responding with 200 OK and skipping JWT validation.
45-51:Details
β Verification inconclusive
Security concern: Be cautious when allowing credentials with dynamic origins.
Setting
Access-Control-Allow-Credentials: truewith dynamically validated origins can introduce security risks if the origin validation is not strict enough. This allows browsers to send cookies and authentication headers cross-origin.Consider:
- Ensure the
allowedOriginsconfiguration is strictly controlled and doesn't use overly permissive wildcards- Document the security implications for administrators configuring this property
- Consider if credentials are truly needed for all CORS requests
Run the following script to check if there are any configuration files defining the allowed origins:
π Script executed:
#!/bin/bash # Description: Find configuration files that define cors.allowed-origins property # Search for cors.allowed-origins in properties files fd -e properties -e yml -e yaml --exec rg -l "cors\.allowed-origins|cors:\s*allowed-origins" {} # Show the actual configuration values fd -e properties -e yml -e yaml --exec rg -A 2 -B 2 "cors\.allowed-origins|cors:\s*allowed-origins" {}Length of output: 528
Security: Confirm strict CORS origin whitelist for credentialed requests
The filter sets
Access-Control-Allow-Credentials: trueand uses the dynamiccors.allowed-originsproperty to validate origins. We located two configuration files:
- src/main/environment/common_example.properties
cors.allowed-origins=@CORS_ALLOWED_ORIGINS@- src/main/environment/common_ci.properties
cors.allowed-origins=http://localhost:*Please ensure:
- Production values for
cors.allowed-originsdo not include broad wildcards (e.g.*orlocalhost:*).- Your
isOriginAllowed(origin)method enforces a strict match against the configured list.- Administrators understand the security risks of allowing credentials in CORS and document the policy.
| String origin = request.getHeader("Origin"); | ||
| 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-Credentials", "true"); | ||
| } | ||
|
|
||
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | ||
| logger.info("OPTIONS request - skipping JWT validation"); | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π‘ Verification agent
π§© Analysis chain
Verify CORS configuration consistency across the application.
The AI summary mentions a new CorsConfig class for global CORS configuration, but this filter also implements CORS handling. Having CORS configuration in multiple places can lead to conflicts and unexpected behavior.
Run the following script to check for other CORS configurations:
π Script executed:
#!/bin/bash
# Description: Search for other CORS configurations in the codebase
# Search for CorsConfig class
fd -e java --exec rg -l "class.*CorsConfig|@Configuration.*Cors" {}
# Search for @CrossOrigin annotations
rg "@CrossOrigin" -A 2 -B 2
# Search for WebMvcConfigurer CORS configurations
ast-grep --pattern 'addCorsMappings($_) {
$$$
}'
# Search for other CORS-related configurations
rg "CorsConfiguration|WebMvcConfigurer.*cors|corsConfiguration" -A 3 -B 3Length of output: 279
π Script executed:
#!/bin/bash
# Inspect global CORS configuration in CorsConfig.java
sed -n '1,200p' src/main/java/com/iemr/tm/config/CorsConfig.javaLength of output: 972
Consolidate CORS configuration
The application currently configures CORS in two places, which can lead to maintenance overhead or unexpected behavior:
- Global CORS in
src/main/java/com/iemr/tm/config/CorsConfig.java - Filter-based CORS in
src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java(lines 45β57)
I recommend centralizing all CORS settings in your CorsConfig and removing the CORS header logic from the JWT filter so that it only handles authentication.
Files to update:
- Remove the CORS block from
JwtUserIdValidationFilter.java(lines 45β57):
--- a/src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java
+++ b/src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java
@@ lines 45-57
- String origin = request.getHeader("Origin");
- 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-Credentials", "true");
- }
-
- if ("OPTIONS".equalsIgnoreCase(request.getMethod())) {
- logger.info("OPTIONS request - skipping JWT validation");
- response.setStatus(HttpServletResponse.SC_OK);
- return;
- }Ensure your CorsConfig.java covers any specific needs (e.g., exposed headers for JWT tokens).
π 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.
| String origin = request.getHeader("Origin"); | |
| 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-Credentials", "true"); | |
| } | |
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | |
| logger.info("OPTIONS request - skipping JWT validation"); | |
| response.setStatus(HttpServletResponse.SC_OK); | |
| return; | |
| } |
π€ Prompt for AI Agents
In src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java between lines
45 and 57, the code manually sets CORS headers, which conflicts with the global
CORS configuration in CorsConfig.java. Remove this CORS handling block from the
filter to avoid redundancy and potential conflicts, ensuring all CORS settings
are managed centrally in CorsConfig.java. Verify that CorsConfig.java properly
covers all necessary CORS policies, including allowed origins, methods, headers,
and credentials.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the wildcard pattern matching to prevent unintended origin matches.
The current implementation has security issues:
- Incorrect wildcard matching: The pattern
*.example.comwould incorrectly matchevilexample.cominstead of only matching subdomains likesub.example.com - Performance: Regex patterns are compiled on every request
- Missing validation: No validation of origin format or protection against regex injection
Apply this improved implementation:
+import java.util.List;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
public class JwtUserIdValidationFilter implements Filter {
private final JwtAuthenticationUtil jwtAuthenticationUtil;
private final Logger logger = LoggerFactory.getLogger(this.getClass().getName());
private final String allowedOrigins;
+ private final List<Pattern> allowedOriginPatterns;
public JwtUserIdValidationFilter(JwtAuthenticationUtil jwtAuthenticationUtil,
@Value("${cors.allowed-origins}") String allowedOrigins) {
this.jwtAuthenticationUtil = jwtAuthenticationUtil;
this.allowedOrigins = allowedOrigins;
+ this.allowedOriginPatterns = compileOriginPatterns(allowedOrigins);
}
+
+ private List<Pattern> compileOriginPatterns(String allowedOrigins) {
+ if (allowedOrigins == null || allowedOrigins.trim().isEmpty()) {
+ return List.of();
+ }
+
+ return Arrays.stream(allowedOrigins.split(","))
+ .map(String::trim)
+ .filter(pattern -> !pattern.isEmpty())
+ .map(this::convertToRegexPattern)
+ .map(Pattern::compile)
+ .collect(Collectors.toList());
+ }
+
+ private String convertToRegexPattern(String pattern) {
+ // Escape special regex characters except *
+ String escaped = pattern.replaceAll("([.+?^${}()|\\[\\]\\\\])", "\\\\$1");
+ // Replace * with proper subdomain matching
+ // *.example.com should match sub.example.com but not example.com or evilexample.com
+ return "^" + escaped.replace("\\*", "[^.]+") + "$";
+ }
private boolean isOriginAllowed(String origin) {
- if (origin == null || allowedOrigins == null || allowedOrigins.trim().isEmpty()) {
+ if (origin == null || allowedOriginPatterns.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 allowedOriginPatterns.stream()
+ .anyMatch(pattern -> pattern.matcher(origin).matches());
}This implementation:
- Pre-compiles patterns for better performance
- Correctly handles wildcards to match only subdomains
- Properly escapes special regex characters
- Adds anchors to ensure full string matching
π€ Prompt for AI Agents
In src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java around lines
128 to 136, the current wildcard pattern matching for allowed origins is
insecure and inefficient, as it improperly matches subdomains and compiles regex
on every request. Fix this by pre-compiling the patterns into regex objects that
correctly match only subdomains when wildcards are used, escaping special
characters and adding anchors for full string match. Store these compiled
patterns during initialization and use them in isOriginAllowed to improve
security and performance.
|


π 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
New Features
Refactor
@CrossOriginannotations from controllers and methods to manage CORS centrally through a new filter.Chores