feat(cors): Global CORS Configuration for AMRIT API Services #182
Conversation
WalkthroughThe changes introduce a new global CORS configuration by adding the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant CorsConfig
participant DynamicCorsFilter
participant Controller
Client->>Server: Sends HTTP Request (with Origin header)
Server->>DynamicCorsFilter: Check Origin header
alt Origin allowed
DynamicCorsFilter->>Server: Set Access-Control-Allow-Origin header
else Origin not allowed
DynamicCorsFilter->>Server: Do not set CORS headers
end
alt OPTIONS Preflight Request
DynamicCorsFilter->>Server: Respond with 200 OK and CORS headers
else Other HTTP Methods
Server->>CorsConfig: Apply global CORS configuration
Server->>Controller: Forward request
Controller-->>Server: Processed response
end
Server->>Client: Return HTTP Response with CORS headers if applicable
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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: 0
🔭 Outside diff range comments (1)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
936-936: 🛠️ Refactor suggestionRemove remaining @crossorigin annotation for consistency.
All other @crossorigin annotations have been removed in favor of the global CORS configuration, but this one remains. It should be removed to maintain consistency and centralized CORS management.
- @CrossOrigin @Operation(summary = "Validating security question and answers for password change") @RequestMapping(value = { "/validateSecurityQuestionAndAnswer" }, method = { RequestMethod.POST })
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/config/CorsConfig.java (1)
14-22: Consider using "/" pattern instead of "/" for comprehensive path mapping.**The current mapping uses "/" which may not cover all API paths. Using "/**" would ensure all endpoints in the application have CORS properly applied.
- public void addCorsMappings(CorsRegistry registry) { - registry.addMapping("/") + public void addCorsMappings(CorsRegistry registry) { + registry.addMapping("/**") .allowedOrigins(allowedOrigins.split(",")) .allowedMethods("GET", "POST", "PUT", "DELETE", "OPTIONS") .allowedHeaders("*") .allowCredentials(true) .maxAge(3600); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/iemr/common/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(21 hunks)src/main/resources/application.properties(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: style-check / checkstyle
- GitHub Check: Analyze (java)
🔇 Additional comments (4)
src/main/resources/application.properties (1)
187-187: CORS configuration added correctly.The new property
cors.allowed-originsis properly configured to allow requests from frontend development servers running on standard ports (3000 for React, 4200 for Angular). This configuration will be used by the new global CORS configuration class.src/main/java/com/iemr/common/config/CorsConfig.java (2)
1-7: LGTM! Appropriate imports for CORS configuration.All necessary imports for implementing the CORS configuration are present.
8-13: LGTM! Configuration class properly set up with property injection.The class is correctly annotated with
@Configurationand properly injects the CORS allowed origins from application properties.src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
109-110: LGTM! Successfully centralized CORS configuration.The removal of method-level @crossorigin annotations is appropriate now that CORS is managed globally through the CorsConfig class. This change improves maintainability by centralizing CORS configuration and ensuring consistent behavior across endpoints.
Also applies to: 124-125, 284-285, 477-478, 490-491, 530-531, 569-570, 616-617, 639-640, 672-673, 714-715, 735-736, 756-757, 774-775, 822-823, 837-838, 852-853, 869-870, 914-915, 962-963
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
src/main/java/com/iemr/common/controller/mctshistory/OutboundHistoryController.java (1)
1-83: 💡 Verification agent🧩 Analysis chain
Verify the impact of CORS changes on API consumers.
Make sure that all legitimate API consumers (frontend applications, third-party integrations) are included in the
cors.allowed-originsproperty. This change from per-method CORS annotations to a global configuration might potentially impact existing integrations if the allowed origins aren't properly configured.
🏁 Script executed:
#!/bin/bash # Check the configured allowed origins in the properties file echo "Checking for cors.allowed-origins property in properties files..." find . -name "*.properties" -exec grep -l "cors.allowed-origins" {} \; # Display the content of any files that contain the CORS configuration echo -e "\nContent of the CORS configuration:" find . -name "*.properties" -exec grep -A2 -B2 "cors.allowed-origins" {} \;Length of output: 494
Action Required: Update Global CORS Configuration
The global CORS configuration is in use, but the
cors.allowed-originsproperty in./src/main/environment/common_ci.propertiesis currently empty. Please ensure that this property is updated with a comprehensive list of all legitimate API consumer origins (frontend applications, third-party integrations) to avoid unexpected API access issues.
- File to review:
src/main/environment/common_ci.properties- Property:
cors.allowed-originssrc/main/java/com/iemr/common/controller/honeywell/HoneywellController.java (1)
1-96: 💡 Verification agent🧩 Analysis chain
Verify existence and configuration of the global CORS config class
This file and the other controllers have had their @crossorigin annotations removed, suggesting a move to a global CORS configuration. However, I don't see the implementation of this global configuration in the provided files.
Please ensure that the
CorsConfigclass mentioned in the AI summary has been created and properly configured to allow the necessary origins, HTTP methods, headers, and credentials. This is critical to maintain functionality after removing the @crossorigin annotations.
🏁 Script executed:
#!/bin/bash # Check for the existence of the CorsConfig class echo "Checking for CorsConfig class..." find src/main -name "CorsConfig.java" | grep . # If found, display its content if [ $? -eq 0 ]; then echo -e "\nFound CorsConfig.java. Displaying content:" find src/main -name "CorsConfig.java" -exec cat {} \; else echo "CorsConfig.java not found!" fi # Check for CORS properties in configuration files echo -e "\nChecking for CORS configuration in properties files..." grep -r "cors.allowed-origins" src/Length of output: 1589
Global CORS configuration verified – ensure valid allowed origins are provided
- Confirmed that the
CorsConfigclass exists atsrc/main/java/com/iemr/common/config/CorsConfig.javaand implements a global CORS configuration.- The configuration properly sets the allowed HTTP methods (GET, POST, PUT, DELETE, OPTIONS), allows all headers, explicitly exposes headers (
Authorization,Jwttoken), and permits credentials.- Action Required: The CORS allowed origins are loaded from the property
cors.allowed-origins, which insrc/main/environment/common_ci.propertiesappears empty. Please verify that this property is correctly set in all relevant environment configurations to avoid inadvertently blocking expected origins.
🧹 Nitpick comments (17)
src/main/java/com/iemr/common/controller/services/CategoryController.java (1)
57-57: @crossorigin annotation removed as part of global CORS configurationThe removal of the
@CrossOriginannotation is consistent with implementing a centralized CORS configuration. This is a good practice as it centralizes CORS policy management.As the
CrossOriginannotation is no longer used, consider removing the unused import at line 33:-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/common/controller/eausadha/EAusadhaController.java (1)
27-27: @crossorigin annotation removed as part of global CORS configurationThe removal of the
@CrossOriginannotation is consistent with implementing a centralized CORS configuration. This is a good practice as it centralizes CORS policy management.As the
CrossOriginannotation is no longer used, consider removing the unused import at line 6:-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/common/controller/lonic/LonicController.java (1)
56-56: @crossorigin annotation removed as part of global CORS configurationThe removal of the
@CrossOriginannotation is consistent with implementing a centralized CORS configuration. This is a good practice as it centralizes CORS policy management.As the
CrossOriginannotation is no longer used, consider removing the unused import at line 27:-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/common/controller/kmfilemanager/KMFileManagerController.java (1)
81-81:❓ Verification inconclusive
Removal of @crossorigin annotations - centralized CORS configuration
The @crossorigin annotations have been removed from all methods in this controller as part of implementing a global CORS configuration. This change promotes consistency in how CORS is handled across the application.
Consider removing the CrossOrigin import at line 32 if it's no longer used elsewhere in this file.
Also applies to: 104-104, 130-130
🏁 Script executed:
#!/bin/bash # Check if CrossOrigin is still used in this file rg -c "@CrossOrigin" "src/main/java/com/iemr/common/controller/kmfilemanager/KMFileManagerController.java"Length of output: 104
Action Required: Verify and Remove Unused @crossorigin Import
- The controller no longer uses any
@CrossOriginannotations, supporting our shift to a centralized CORS configuration.- Please manually verify that the import statement (typically at line 32:
import org.springframework.web.bind.annotation.CrossOrigin;) is not required elsewhere in the file before removing it.- This check also applies to the noted method areas (lines 81, 104, and 130).
src/main/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationController.java (1)
59-59:❓ Verification inconclusive
Removal of @crossorigin annotations - centralized CORS configuration
All @crossorigin annotations have been removed from this controller's methods (22 methods in total) as part of implementing a global CORS configuration. This is a significant improvement as it:
- Centralizes CORS management
- Ensures consistent CORS policies across endpoints
- Makes future CORS policy changes easier to implement
Consider removing the CrossOrigin import at line 30 if it's no longer used elsewhere in this file.
Also applies to: 81-81, 102-102, 123-123, 146-146, 168-168, 189-189, 210-210, 232-232, 257-257, 278-278, 299-299, 322-322, 345-345, 368-368, 389-389, 410-410, 431-431, 452-452, 475-475, 501-501, 527-527
🏁 Script executed:
#!/bin/bash # Check if CrossOrigin is still used in this file rg -c "@CrossOrigin" "src/main/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationController.java"Length of output: 109
Action Required: Verify Removal of @crossorigin and Clean Up Imports
The @crossorigin annotations appear to be removed in line with the centralized CORS configuration, which is a solid improvement for consistent endpoint management. However, the initial verification script did not return clear results regarding any remaining @crossorigin occurrences. Please manually verify the following:
- Ensure that no stray @crossorigin annotations remain in the file.
- Confirm if the unused CrossOrigin import at line 30 is indeed redundant and can be safely removed.
Once these points have been manually verified, the cleanup can be finalized.
src/main/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardController.java (3)
29-29: Remove unused import.The
CrossOriginimport is no longer used since all@CrossOriginannotations have been removed from the controller methods. This is part of the centralization of CORS configuration.- import org.springframework.web.bind.annotation.CrossOrigin;
53-53: Clean up unnecessary whitespace.This empty line was likely left after removing the
@CrossOriginannotation. Remove it to maintain clean code.
22-112: Verify global CORS configuration for these endpoints.The
@CrossOriginannotations have been removed from all controller methods, which means these endpoints now rely on the global CORS configuration in theCorsConfigclass. Ensure that the global configuration properly handles cross-origin requests for these specific endpoints.This change centralizes CORS management, which is good for maintainability but could cause issues if specific endpoints need different CORS settings.
src/main/java/com/iemr/common/controller/institute/InstituteController.java (3)
30-30: Remove unused import.The
CrossOriginimport is no longer used since all@CrossOriginannotations have been removed from the controller methods. This is part of the centralization of CORS configuration.- import org.springframework.web.bind.annotation.CrossOrigin;
78-78: Clean up unnecessary whitespace.These empty lines were likely left after removing the
@CrossOriginannotations. Remove them to maintain clean code.Also applies to: 99-99, 120-120, 137-137, 153-153, 170-170
47-186: Verify global CORS configuration for institute endpoints.The
@CrossOriginannotations have been removed from all controller methods, which means these endpoints now rely on the global CORS configuration in theCorsConfigclass. Ensure that the global configuration properly handles cross-origin requests for these specific endpoints, particularly those that client applications may be directly calling.This change centralizes CORS management, which is good for maintainability but could cause issues if specific endpoints need different CORS settings.
src/main/java/com/iemr/common/controller/notification/NotificationController.java (3)
29-29: Remove unused import.The
CrossOriginimport is no longer used since all@CrossOriginannotations have been removed from the controller methods. This is part of the centralization of CORS configuration.- import org.springframework.web.bind.annotation.CrossOrigin;
55-55: Clean up unnecessary whitespace.These empty lines were likely left after removing the
@CrossOriginannotations. Remove them to maintain clean code.Also applies to: 75-75, 95-95, 123-123, 149-149, 166-166, 185-185, 204-204, 222-222, 239-239, 261-261
42-281: Verify global CORS configuration for notification endpoints.The
@CrossOriginannotations have been removed from all controller methods, which means these endpoints now rely on the global CORS configuration in theCorsConfigclass. Ensure that the global configuration properly handles cross-origin requests for notification endpoints.Since notification endpoints might be called from various clients, including external systems, verify that the global CORS policy adequately addresses their specific access requirements.
src/main/java/com/iemr/common/controller/customization/CustomizationController.java (3)
9-9: Remove unused import.The
CrossOriginimport is no longer used since all@CrossOriginannotations have been removed from the controller methods. This is part of the centralization of CORS configuration.- import org.springframework.web.bind.annotation.CrossOrigin;
33-33: Clean up unnecessary whitespace.These empty lines were likely left after removing the
@CrossOriginannotations. Remove them to maintain clean code.Also applies to: 50-50, 66-66, 83-83, 100-100, 117-117, 134-134, 150-150, 166-166, 182-182, 199-199, 216-216, 233-233, 250-250
23-264:❓ Verification inconclusive
Verify global CORS configuration for customization endpoints.
The
@CrossOriginannotations have been removed from all controller methods, which means these endpoints now rely on the global CORS configuration in theCorsConfigclass. Ensure that the global configuration properly handles cross-origin requests for customization endpoints.For UI-focused endpoints like customization, it's particularly important to verify that the CORS configuration allows the appropriate origins where your web applications are hosted.
🏁 Script executed:
#!/bin/bash # Find the CorsConfig class to understand the global configuration fd CorsConfig.java # Check what origins are allowed in the properties fd -t f -e properties | xargs grep "cors.allowed-origins" || echo "No cors.allowed-origins found in properties files"Length of output: 195
Action: Verify Global CORS Settings for Customization Endpoints
The global CORS configuration is correctly centralized in
CorsConfig.java, and the controller endpoints now rely on this configuration rather than using method-level@CrossOriginannotations. Note that in the CI properties file (src/main/environment/common_ci.properties), thecors.allowed-originsproperty is currently empty. Please verify that the desired origins are set in the appropriate environment properties so that UI applications can successfully access these endpoints in production.
📜 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/java/com/iemr/common/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java(1 hunks)src/main/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationController.java(9 hunks)src/main/java/com/iemr/common/controller/brd/BRDIntegrationController.java(0 hunks)src/main/java/com/iemr/common/controller/callhandling/CallController.java(29 hunks)src/main/java/com/iemr/common/controller/carestream/CareStreamCreateOrderController.java(3 hunks)src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java(3 hunks)src/main/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationController.java(22 hunks)src/main/java/com/iemr/common/controller/customization/CustomizationController.java(14 hunks)src/main/java/com/iemr/common/controller/directory/DirectoryController.java(4 hunks)src/main/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppController.java(3 hunks)src/main/java/com/iemr/common/controller/eausadha/EAusadhaController.java(1 hunks)src/main/java/com/iemr/common/controller/email/EmailController.java(3 hunks)src/main/java/com/iemr/common/controller/esanjeevani/ESanjeevaniController.java(0 hunks)src/main/java/com/iemr/common/controller/everwell/callhandle/EverwellCallController.java(9 hunks)src/main/java/com/iemr/common/controller/everwellTest/EverwellController.java(4 hunks)src/main/java/com/iemr/common/controller/feedback/FeedbackController.java(24 hunks)src/main/java/com/iemr/common/controller/helpline104history/Helpline104BeneficiaryHistoryController.java(1 hunks)src/main/java/com/iemr/common/controller/honeywell/HoneywellController.java(4 hunks)src/main/java/com/iemr/common/controller/institute/InstituteController.java(6 hunks)src/main/java/com/iemr/common/controller/kmfilemanager/KMFileManagerController.java(3 hunks)src/main/java/com/iemr/common/controller/location/LocationController.java(7 hunks)src/main/java/com/iemr/common/controller/lonic/LonicController.java(1 hunks)src/main/java/com/iemr/common/controller/lungassessment/LungAssessmentController.java(3 hunks)src/main/java/com/iemr/common/controller/mctshistory/OutboundHistoryController.java(2 hunks)src/main/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardController.java(1 hunks)src/main/java/com/iemr/common/controller/notification/NotificationController.java(11 hunks)src/main/java/com/iemr/common/controller/otp/OTPGateway.java(3 hunks)src/main/java/com/iemr/common/controller/questionconfig/QuestionnaireController.java(2 hunks)src/main/java/com/iemr/common/controller/report/CustomerRelationshipReports.java(1 hunks)src/main/java/com/iemr/common/controller/scheme/SchemeController.java(3 hunks)src/main/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReports.java(11 hunks)src/main/java/com/iemr/common/controller/services/CategoryController.java(1 hunks)src/main/java/com/iemr/common/controller/sms/SMSController.java(7 hunks)src/main/java/com/iemr/common/controller/snomedct/SnomedController.java(2 hunks)src/main/java/com/iemr/common/controller/uptsu/UPTechnicalSupportController.java(2 hunks)src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java(3 hunks)src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(22 hunks)src/main/resources/application.properties(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/iemr/common/controller/brd/BRDIntegrationController.java
- src/main/java/com/iemr/common/controller/esanjeevani/ESanjeevaniController.java
✅ Files skipped from review due to trivial changes (9)
- src/main/resources/application.properties
- src/main/java/com/iemr/common/controller/helpline104history/Helpline104BeneficiaryHistoryController.java
- src/main/java/com/iemr/common/controller/uptsu/UPTechnicalSupportController.java
- src/main/java/com/iemr/common/controller/sms/SMSController.java
- src/main/java/com/iemr/common/controller/carestream/CareStreamCreateOrderController.java
- src/main/java/com/iemr/common/controller/everwell/callhandle/EverwellCallController.java
- src/main/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationController.java
- src/main/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReports.java
- src/main/java/com/iemr/common/controller/callhandling/CallController.java
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build
- GitHub Check: Analyze (java)
- GitHub Check: Package-test
🔇 Additional comments (50)
src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java (1)
27-27: Removed @crossorigin annotation for centralized CORS managementThe removal of the method-level
@CrossOriginannotation aligns with the PR objective of implementing a global CORS configuration. This change means that the endpoint will now use the centralized CORS configuration instead of having its own settings.src/main/java/com/iemr/common/controller/otp/OTPGateway.java (3)
54-54: Removed @crossorigin annotation from sendOTP methodThe removal of the method-level
@CrossOrigin()annotation for the sendOTP method is part of the global CORS configuration implementation. This endpoint will now use the centralized CORS settings.
77-77: Removed @crossorigin annotation from validateOTP methodThe removal of the method-level
@CrossOrigin()annotation for the validateOTP method is part of the global CORS configuration implementation. This endpoint will now use the centralized CORS settings.
101-101: Removed @crossorigin annotation from resendOTP methodThe removal of the method-level
@CrossOrigin()annotation for the resendOTP method is part of the global CORS configuration implementation. This endpoint will now use the centralized CORS settings.src/main/java/com/iemr/common/controller/snomedct/SnomedController.java (3)
56-56: Removed @crossorigin annotation from getSnomedCTRecord methodThe removal of the method-level
@CrossOrigin()annotation for the getSnomedCTRecord method is consistent with the global CORS configuration implementation. This endpoint will now use the centralized CORS settings.
82-82: Removed @crossorigin annotation from getSnomedCTRecordList methodThe removal of the method-level
@CrossOrigin()annotation for the getSnomedCTRecordList method is consistent with the global CORS configuration implementation. This endpoint will now use the centralized CORS settings.
1-109:✅ Verification successful
Verify CorsConfig implementation for global configuration
While removing the individual @crossorigin annotations is a good step toward centralized CORS configuration, it's important to verify that the new CorsConfig class is properly implemented to support all needed CORS functionality.
🏁 Script executed:
#!/bin/bash # Check for the new CorsConfig class implementation echo "Looking for CorsConfig implementation:" cat src/main/java/com/iemr/common/config/CorsConfig.java echo -e "\nChecking CORS properties configuration:" grep -r "cors.allowed-origins" --include="*.properties" .Length of output: 1287
Global CORS Setup Verified
The new CorsConfig class insrc/main/java/com/iemr/common/config/CorsConfig.javais implemented correctly—it centralizes CORS configuration by reading the allowed origins (splitting a comma‐delimited string from thecors.allowed-originsproperty) and properly configuring allowed methods, headers, exposed headers, credentials, and max age. One point to note is that thecors.allowed-originsproperty insrc/main/environment/common_ci.propertiesis currently empty. Please verify that this property is set appropriately for each deployment environment to ensure the expected CORS behavior.src/main/java/com/iemr/common/config/CorsConfig.java (1)
1-25: Centralized CORS configuration looks good.This class implements a global CORS configuration for the application, which is a more maintainable approach than having individual
@CrossOrigin()annotations scattered across controller methods. The implementation correctly:
- Uses a property injection for allowed origins
- Configures standard HTTP methods
- Exposes necessary headers for authentication
- Properly sets allowCredentials and maxAge
src/main/java/com/iemr/common/controller/feedback/FeedbackController.java (1)
99-99: CORS configuration cleanup looks good.The removal of individual
@CrossOrigin()annotations from all these methods is consistent with the new approach of using a centralized CORS configuration through theCorsConfigclass.Also applies to: 116-116, 132-132, 148-148, 166-166, 181-181, 195-195, 211-211, 227-227, 243-243, 259-259, 286-286, 299-299, 312-312, 328-328, 344-344, 364-364, 381-381, 405-405, 424-424, 443-443, 460-460, 477-477, 492-492
src/main/java/com/iemr/common/controller/questionconfig/QuestionnaireController.java (1)
59-59: CORS configuration cleanup is consistent.The removal of
@CrossOrigin()annotations from these methods aligns with the global CORS configuration approach implemented in this PR.Also applies to: 74-74
src/main/java/com/iemr/common/controller/mctshistory/OutboundHistoryController.java (1)
50-50: CORS configuration cleanup is consistent.The removal of
@CrossOrigin()annotations from these methods follows the same pattern as in other controllers, moving to the centralized CORS configuration approach.Also applies to: 66-66
src/main/java/com/iemr/common/controller/email/EmailController.java (1)
58-58:✅ Verification successful
CORS annotations removed in favor of global configuration
The @crossorigin annotations have been removed from the SendEmail, getAuthorityEmailID, and sendEmailGeneral methods. This change aligns with the PR's objective to implement a global CORS configuration.
Also applies to: 77-77, 92-92
🏁 Script executed:
#!/bin/bash # Check if a global CORS configuration exists in the codebase echo "Searching for CorsConfig class..." rg -p "class.*CorsConfig.*implements.*WebMvcConfigurer" --type javaLength of output: 255
Global CORS Configuration Verified and Approved
- Confirmed that the global CORS setup is active via the presence of
src/main/java/com/iemr/common/config/CorsConfig.java(e.g., line 9 verifying it implementsWebMvcConfigurer).- Removal of
@CrossOriginannotations from theSendEmail,getAuthorityEmailID, andsendEmailGeneralmethods inEmailController.java(lines 58, 77, and 92) is consistent with this centralized configuration.No further changes are required.
src/main/java/com/iemr/common/controller/scheme/SchemeController.java (1)
59-59: CORS annotations removed in favor of global configurationThe @crossorigin annotations have been removed from the saveSchemeDetails, getSchemeList, and deleteScheme methods. This change is consistent with the overall approach of centralizing CORS configuration.
Also applies to: 86-86, 112-112
src/main/java/com/iemr/common/controller/location/LocationController.java (1)
54-54:✅ Verification successful
CORS annotations removed from location endpoints
The @crossorigin annotations have been removed from all methods in the LocationController: getStates, getDistricts, getStatetDistricts, getDistrictBlocks, getCity, getVillages, and getCountries. This aligns with the global CORS configuration approach.
Also applies to: 70-70, 86-86, 103-103, 119-119, 136-136, 157-157
🏁 Script executed:
#!/bin/bash # Examine application properties to check CORS configuration echo "Searching for CORS configuration in properties files..." fd --type f --extension properties | xargs grep -l "cors"Length of output: 220
Approved: Global CORS Configuration Verified
The global CORS setup was confirmed by the properties file (
src/main/environment/common_ci.properties) containing relevant CORS settings. This validates the removal of the@CrossOriginannotations from the LocationController methods (lines 54, 70, 86, 103, 119, 136, and 157), ensuring consistency across the application.src/main/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppController.java (1)
42-42:✅ Verification successful
CORS annotations removed at both class and method levels
The @crossorigin annotations have been removed from the DoorToDoorAppController class as well as from the getUserDetails and getSuspectedData_HRP_TB_NCD methods. This is part of the architectural change to use global CORS configuration instead of method-level annotations.
Also applies to: 52-52, 73-73
🏁 Script executed:
#!/bin/bash # Look for the implementation of the global CORS configuration echo "Checking CorsConfig implementation details..." fd "CorsConfig.java" --type f | xargs catLength of output: 1066
Global CORS Configuration Verified
The removal of the
@CrossOriginannotations from theDoorToDoorAppController(lines 42, 52, and 73) is confirmed by our global CORS configuration inCorsConfig.java. The implementation inCorsConfig.javacorrectly sets up CORS using allowed origin patterns and the necessary HTTP methods. This centralized approach aligns with the architectural shift away from method-level annotations.src/main/java/com/iemr/common/controller/lungassessment/LungAssessmentController.java (3)
50-50: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the startAssesment method is consistent with the centralized CORS approach being implemented in this PR.
68-68: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the getAssessment method is consistent with the centralized CORS approach being implemented in this PR.
86-86: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the getAssessmentDetails method is consistent with the centralized CORS approach being implemented in this PR.
src/main/java/com/iemr/common/controller/directory/DirectoryController.java (4)
82-82: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the getDirectory method is consistent with the centralized CORS approach being implemented in this PR.
103-103: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the getDirectoryV1 method is consistent with the centralized CORS approach being implemented in this PR.
127-127: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the getSubDirectory method is consistent with the centralized CORS approach being implemented in this PR.
151-151: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the getInstitutesDirectories method is consistent with the centralized CORS approach being implemented in this PR.
src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java (3)
59-59: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the fetchFile method is consistent with the centralized CORS approach being implemented in this PR.
81-81: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the fetchFileFromCentral method is consistent with the centralized CORS approach being implemented in this PR.
103-103: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the existFile method is consistent with the centralized CORS approach being implemented in this PR.
src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java (4)
62-62: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the getVaccinationTypeAndDoseTaken method is consistent with the centralized CORS approach being implemented in this PR.
87-87: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the getCovidVaccinationDetails method is consistent with the centralized CORS approach being implemented in this PR.
131-131: Removal of @crossorigin annotation aligns with global CORS configuration.The removal of the @crossorigin annotation from the saveCovidVaccinationDetails method is consistent with the centralized CORS approach being implemented in this PR.
1-167:✅ Verification successful
Consider adding exception handling for potential CORS-related issues
With the transition to a global CORS configuration, it's important to ensure that any CORS-related exceptions are properly handled. Consider adding appropriate exception handling or logging for situations where CORS might still cause issues.
🏁 Script executed:
#!/bin/bash # Check if there's appropriate CORS exception handling in the new CorsConfig class fd "CorsConfig" --type f --exec cat {} \; | grep -B 5 -A 10 "implements WebMvcConfigurer"Length of output: 728
CORS Exception Handling Verified via Global Configuration
After verifying the global configuration in the
CorsConfigclass (which implementsWebMvcConfigurerand correctly sets up CORS mappings and allowed origins), it appears that potential CORS issues are already managed centrally. There is no need to add additional exception handling in theCovidVaccinationController.
- Verified File:
CorsConfigproperly configures CORS viaaddCorsMappings.- Action: No changes required in
CovidVaccinationControllerfor CORS exception handling.src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (21)
109-109: CORS configuration now centralizedThe removal of
@CrossOrigin()annotation is part of the global CORS configuration implementation. This change improves security and maintainability by enforcing a consistent CORS policy across all endpoints.
124-124: Consistent CORS policy enforcementRemoving method-level
@CrossOrigin()annotation aligns with the PR objective of implementing a global CORS configuration, making the security policy more consistent and maintainable.
284-284: Centralized CORS handlingThe removal of
@CrossOrigin()annotation from this method supports the global CORS configuration approach, reducing duplication and improving security governance.
449-475: Commented code removalThe commented-out
userAuthenticateV1method is being fully removed. This is good practice as it reduces code clutter and prevents potential confusion.
477-477: Standardized CORS managementRemoving the method-level CORS annotation ensures that all authentication endpoints follow the same cross-origin policy defined in the global configuration.
490-490: Consistent security policyRemoving the
@CrossOrigin()annotation from the password management endpoint ensures uniform security policy enforcement across the application.
530-530: Global CORS configuration adoptionThe removal of method-specific CORS annotation aligns with the PR's goal of centralizing CORS management, which helps prevent security misconfiguration.
569-569: Centralized access controlRemoving the
@CrossOrigin()annotation from this sensitive endpoint (password change) improves security by ensuring it adheres to the globally defined CORS rules.
639-639: Consolidated CORS configurationThe
@CrossOrigin()annotation removal from this endpoint contributes to the unified CORS policy implementation across the application.
672-672: Standardized access controlRemoving the method-level CORS annotation ensures consistent handling of cross-origin requests for role-based access endpoints.
714-714: Unified CORS managementCentralizing CORS configuration by removing method-specific annotations improves maintainability and security consistency for user service point endpoints.
735-735: Standardized cross-origin policyThe removal of
@CrossOrigin()annotation from this data access endpoint ensures it follows the globally defined security rules.
756-756: Centralized security controlRemoving method-level CORS configuration contributes to a more maintainable and consistent security posture for location-based endpoints.
774-774: Uniform CORS handlingThe removal of
@CrossOrigin()annotation from the logout endpoint ensures consistent security behavior across authentication-related operations.
822-822: Consolidated access controlRemoving the method-specific CORS annotation from this session management endpoint improves security governance through centralized configuration.
837-837: Standardized security policyThe
@CrossOrigin()annotation removal ensures that the logout functionality adheres to the globally defined CORS policy.
852-852: Improved CORS governanceRemoving the method-level CORS annotation contributes to a more consistent and maintainable cross-origin security implementation.
869-869: Centralized cross-origin controlThe removal of
@CrossOrigin()annotation from this authentication endpoint aligns with the global CORS configuration strategy.
936-936: Consistent security implementationRemoving method-specific CORS annotation ensures that security validation endpoints follow the centralized cross-origin policy.
962-962: Unified CORS managementRemoving the
@CrossOrigin()annotation from this authentication endpoint ensures consistent cross-origin request handling across the application.
75-1037:❓ Verification inconclusive
Verify global CORS configuration compatibility with all endpoints
All method-level CORS annotations have been removed in favor of a centralized configuration. Make sure that the global CORS settings in the new
CorsConfigclass provide appropriate access for all these endpoints, especially considering the various authentication, user management, and administrative operations they handle.
🏁 Script executed:
#!/bin/bash # Check for the new CorsConfig class to understand the global configuration fd -e java "CorsConfig.java" | xargs cat # Check application properties to see CORS-related settings fd -e properties | xargs grep -l "cors" | xargs catLength of output: 7258
Global CORS Configuration Verification
The centralized CORS setup in
CorsConfignow applies to all endpoints (with the pattern/**), allowing the HTTP methods GET, POST, PUT, DELETE, and OPTIONS; all headers are allowed; credentials are enabled; and specific headers like "Authorization" and "Jwttoken" are exposed. This effectively removes method-level CORS annotations.Key points for verification:
- The configuration derives allowed origins from the
cors.allowed-originsproperty. However, the properties output shows this value is currently empty.- Ensure that in each deployment environment (or via environment variables), the
cors.allowed-originsproperty is properly set; an empty value may lead to unintended restrictions on frontend access.- Double-check that this global configuration meets the security and functionality requirements for all authentication, user management, and administrative operations.
Please verify that the intended origins are correctly configured (and are not left empty) so that all endpoints are accessible as expected.
| @@ -167,5 +167,7 @@ grievanceAllocationRetryConfiguration=3 | |||
| start-grievancedatasync-scheduler=false | |||
| cron-scheduler-grievancedatasync=0 0/2 * * * ? | |||
|
|
|||
| cors.allowed-origins= | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Empty value for CORS allowed origins
The cors.allowed-origins property has been added but left empty. This could lead to either all cross-origin requests being blocked or potentially allowing all origins depending on how the property is processed in the CorsConfig class.
Please specify appropriate values for this property based on your environment requirements. Typical values might include:
- Specific domain:
https://example.com - Multiple domains:
https://example.com,https://another-example.com - Wildcard:
*(allows all origins, but should be used cautiously in production)
🏁 Script executed:
#!/bin/bash
# Check how cors.allowed-origins is being used in the code
grep -r "cors.allowed-origins" --include="*.java" .Length of output: 144
Action: Update CORS Allowed Origins Configuration
The cors.allowed-origins property in src/main/environment/common_ci.properties is currently empty. The property is injected in CorsConfig.java via @Value("${cors.allowed-origins}"), meaning that if this value remains unset, the application’s CORS behavior could inadvertently either block all cross-origin requests or potentially allow all origins depending on the internal processing logic.
Please specify the correct origin(s) based on your environment requirements. For example:
- A specific domain:
https://example.com - Multiple domains:
https://example.com,https://another-example.com - A wildcard (
*): Use cautiously in non-production setups
| @@ -58,7 +58,7 @@ public void setCallReportsService(CallReportsService callReportsService) { | |||
| @Autowired | |||
| Report1097Mapper mapper; | |||
|
|
|||
| @CrossOrigin() | |||
|
|
|||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Removed @crossorigin annotation as part of global CORS configuration
The removal of the method-level @CrossOrigin() annotation is consistent with the PR objective of implementing a global CORS configuration. Note that this controller still has a class-level @CrossOrigin annotation at line 43, which might need to be addressed for complete centralization.
🏁 Script executed:
#!/bin/bash
# Check if any controllers in the codebase still have class-level @CrossOrigin annotations
# that might override the global configuration
grep -r "@CrossOrigin" --include="*.java" src/main/java/com/iemr/common/controller/Length of output: 1171
Action Required: Evaluate Class-Level @crossorigin in CustomerRelationshipReports
The removal of the method-level @CrossOrigin() annotation aligns with moving to a global CORS configuration. However, the CustomerRelationshipReports.java controller still retains a class-level @CrossOrigin annotation (line 43). Please verify whether this annotation is necessary or if it can be removed to ensure complete centralization of CORS settings.
- File:
src/main/java/com/iemr/common/controller/report/CustomerRelationshipReports.java - Concern: Remnant class-level
@CrossOriginannotation at line 43
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
src/main/java/com/iemr/common/controller/everwellTest/EverwellController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/iemr/common/controller/honeywell/HoneywellController.java
Show resolved
Hide resolved
kevalkanp1011
changed the title
|
@drtechie @ravishanigarapu please review pr. i attached screenshots |
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/utils/DynamicCorsFilter.java (1)
26-29: Consider handling wildcard origins.The current implementation checks if the specific origin is in the allowed list. If you intend to support wildcard origins (e.g., "*"), you'll need to add special handling for this case.
if (origin != null && Arrays.asList(allowedOrigins).contains(origin)) { response.setHeader("Access-Control-Allow-Origin", origin); +} else if (origin != null && Arrays.asList(allowedOrigins).contains("*")) { + response.setHeader("Access-Control-Allow-Origin", "*"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/iemr/common/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/common/utils/DynamicCorsFilter.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/iemr/common/config/CorsConfig.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (1)
src/main/java/com/iemr/common/utils/DynamicCorsFilter.java (1)
1-15: Good implementation of a custom CORS filter.The filter extends Spring's
OncePerRequestFilterwhich ensures the filter is only executed once per request. This is a good practice for implementation of custom filters.
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| } else { | ||
| filterChain.doFilter(request, response); | ||
| } | ||
| } |
There was a problem hiding this comment.
Include all necessary CORS headers for preflight responses.
The current implementation handles OPTIONS requests by returning 200 OK, but it doesn't set all the required CORS headers for a proper preflight response. This could cause CORS issues for complex requests.
if ("OPTIONS".equalsIgnoreCase(request.getMethod())) {
response.setStatus(HttpServletResponse.SC_OK);
+ response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
+ response.setHeader("Access-Control-Allow-Headers", "Origin, X-Requested-With, Content-Type, Accept, Authorization");
+ response.setHeader("Access-Control-Max-Age", "3600");
+ response.setHeader("Access-Control-Allow-Credentials", "true");
} 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.setStatus(HttpServletResponse.SC_OK); | |
| response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS"); | |
| response.setHeader("Access-Control-Allow-Headers", "Origin, X-Requested-With, Content-Type, Accept, Authorization"); | |
| response.setHeader("Access-Control-Max-Age", "3600"); | |
| response.setHeader("Access-Control-Allow-Credentials", "true"); | |
| } else { | |
| filterChain.doFilter(request, response); | |
| } | |
| } |
| @Value("${cors.allowed-origins}") | ||
| private String[] allowedOrigins; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure property is defined in all environment configurations.
The cors.allowed-origins property is injected here but should be verified in all relevant environment configuration files to avoid null pointer exceptions. I see from the PR summary that this property was added to the common_ci.properties file, but ensure it's properly defined across all environments.
🏁 Script executed:
#!/bin/bash
# Search for the cors.allowed-origins property in all properties files
find src/main/environment -name "*.properties" -type f -exec grep -l "cors.allowed-origins" {} \;Length of output: 142
To identify which environments are missing the setting, list all property files first:
🏁 Script executed:
#!/bin/bash
find src/main/environment -name "*.properties"Length of output: 267
Add cors.allowed-origins to all environment property files
The cors.allowed-origins key is currently only defined in common_ci.properties. To prevent null-pointer issues at runtime, please add this property (with appropriate values) to each of the other environment configs:
• src/main/environment/common_dev.properties
• src/main/environment/common_example.properties
• src/main/environment/common_test.properties
• src/main/environment/common_uat.properties
Example entry:
cors.allowed-origins=http://localhost:3000,https://your.domain.com
| @Component | ||
| public class DynamicCorsFilter extends OncePerRequestFilter { | ||
|
|
||
| @Value("${cors.allowed-origins}") | ||
| private String[] allowedOrigins; | ||
|
|
||
| @Override | ||
| protected void doFilterInternal(HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| FilterChain filterChain) | ||
| throws ServletException, IOException { | ||
|
|
||
| String origin = request.getHeader("Origin"); | ||
| if (origin != null && Arrays.asList(allowedOrigins).contains(origin)) { | ||
| response.setHeader("Access-Control-Allow-Origin", origin); | ||
| } | ||
|
|
||
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| } else { | ||
| filterChain.doFilter(request, response); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential redundancy with CorsConfig.
According to the PR summary, there's also a new CorsConfig class that configures CORS globally. Having both a filter and a configuration class could lead to redundancy or conflicts. Consider consolidating the CORS configuration to either the filter or the Spring configuration, but not both, to avoid unexpected behavior.
🌐 Web query:
Is it recommended to use both Spring's CorsRegistry and a custom OncePerRequestFilter for CORS in the same application?
💡 Result:
No, it is generally not recommended to use both Spring's CorsRegistry (usually configured via WebMvcConfigurer) and a custom OncePerRequestFilter for handling CORS in the same application.
Here’s why:
-
Redundancy and Complexity: Both approaches serve the same purpose—handling CORS requests. Using both can lead to redundant processing and increase complexity without added benefit.
-
Potential Conflicts: The built-in Spring CORS support and your custom filter might send conflicting headers or handle preflight requests differently, causing unpredictable behavior for clients.
-
Maintainability: Leveraging Spring’s native CORS support via
CorsRegistry(or annotations like@CrossOrigin) keeps your configuration centralized and easier to maintain.
Recommended Approach
- Use Spring’s built-in CORS support (
CorsRegistryinWebMvcConfigureror@CrossOriginannotations) for standard configurations. - Use a custom filter (
OncePerRequestFilter) only if you need highly customized behavior that Spring’s built-in support cannot accommodate.
If you do need custom behavior, consider:
- Either extend from Spring's CORS configuration rather than completely replacing it.
- Ensure that your custom filter either complements or replaces the default Spring handling, but not both simultaneously.
In summary: prefer one clear, consistent method for CORS handling to avoid conflicts and complexity.
Consolidate CORS handling into a single mechanism
Currently the application defines CORS rules both in
- DynamicCorsFilter (src/main/java/com/iemr/common/utils/DynamicCorsFilter.java)
- CorsConfig (WebMvcConfigurer/CorsRegistry)
Using both approaches leads to redundant processing, conflicting headers and unpredictable preflight behavior. It’s recommended to pick one:
• Remove DynamicCorsFilter and centralize your CORS rules in CorsConfig (leveraging Spring’s built-in support).
• Or, if you need custom logic, disable Spring’s global CORS (in CorsConfig) and implement all rules in your OncePerRequestFilter.
Please update the following accordingly:
- src/main/java/com/iemr/common/utils/DynamicCorsFilter.java
- src/main/java/.../CorsConfig.java
📋 Description
JIRA ID:
This PR introduces a global CORS configuration for the AMRIT platform API services, in alignment with requirements. The goal is to enhance cross-origin request handling by removing controller-level CORS annotations and setting up centralized, environment-based CORS policies.
✅ 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
Summary by CodeRabbit