-
Notifications
You must be signed in to change notification settings - Fork 44
Fix(cors): Update CORS setup to use property-based origins #195
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 change centralizes and standardizes Cross-Origin Resource Sharing (CORS) configuration for the application. It removes all Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant DynamicCorsFilter
participant JwtUserIdValidationFilter
participant Controller
Browser->>DynamicCorsFilter: HTTP Request (with Origin header)
DynamicCorsFilter->>DynamicCorsFilter: Check if Origin is allowed
alt Origin allowed
DynamicCorsFilter->>Browser: Set Access-Control-Allow-Origin header
end
alt OPTIONS request
DynamicCorsFilter->>Browser: Respond 200 OK (preflight)
else Other methods
DynamicCorsFilter->>JwtUserIdValidationFilter: Continue filter chain
JwtUserIdValidationFilter->>JwtUserIdValidationFilter: Set CORS headers
alt OPTIONS request
JwtUserIdValidationFilter->>Browser: Respond 200 OK
else Other methods
JwtUserIdValidationFilter->>Controller: Continue filter chain
Controller->>Browser: Handle request and respond
end
end
Suggested reviewers
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ 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 (2)
src/main/java/com/iemr/common/controller/location/LocationController.java (1)
1-171
: 💡 Verification agent🧩 Analysis chain
Verify the centralized CORS configuration is properly implemented
While removing the annotations is correct, it's important to verify that the new CORS configuration setup is properly implemented and functioning as expected, especially since this controller handles sensitive location data.
🏁 Script executed:
#!/bin/bash # Search for the new CorsConfig class implementation echo "Checking for CorsConfig implementation..." fd -t f "CorsConfig.java" cat $(fd -t f "CorsConfig.java" | head -n 1) # Check the environment properties file for CORS settings echo "Checking for CORS properties configuration..." fd -t f "common_ci.properties" rg "cors.allowed-origins" $(fd -t f "common_ci.properties" | head -n 1) # Check for DynamicCorsFilter implementation echo "Checking for DynamicCorsFilter implementation..." fd -t f "DynamicCorsFilter.java" cat $(fd -t f "DynamicCorsFilter.java" | head -n 1)Length of output: 3067
Fix CORS configuration and ensure allowed‐origins are set
It looks like you’ve got two CORS mechanisms in place –
CorsConfig
(implements WebMvcConfigurer) andDynamicCorsFilter
– but your CI properties (src/main/environment/common_ci.properties
) currently definecors.allowed-origins=
(with no values), so neither configuration will actually allow any origins.
Please update your environment configs and remove any redundant filter/bean so only one CORS setup is in effect.
Points to address:
- Environment properties (
src/main/environment/common_ci.properties
and othercommon_*.properties
):
• Populatecors.allowed-origins
with a comma-separated list of permitted origins (e.g.https://app.example.com,https://api.example.com
).- Remove or consolidate duplicate CORS handling:
• Either keep theCorsConfig
class or theDynamicCorsFilter
– not both – to avoid conflicts.- Verify at runtime that the correct
Access-Control-Allow-Origin
header is sent for valid requests (including preflight OPTIONS).src/main/java/com/iemr/common/controller/customization/CustomizationController.java (1)
1-266
: 💡 Verification agent🧩 Analysis chain
Verify centralized CORS configuration is working correctly
While removing the annotations is correct, we should ensure the new centralized CORS configuration (via the new
CorsConfig
class and properties) is working as expected for all these endpoints.
🏁 Script executed:
#!/bin/bash # Check for the implementation of the CorsConfig class echo "Searching for CorsConfig implementation..." rg -A 20 "class CorsConfig" --type java # Check for cors.allowed-origins property in properties files echo "Checking for cors.allowed-origins property in configuration files..." rg "cors\.allowed-origins" --type propertiesLength of output: 1868
I’d like to double-check that the
CorsConfig
bean is actually being picked up and that the allowed-origins property is defined. Let’s run:
🏁 Script executed:
#!/bin/bash # Verify CorsConfig is annotated for component scanning echo "Checking for @Configuration or @Component on CorsConfig..." rg -n "@Configuration" src/main/java/com/iemr/common/config/CorsConfig.java rg -n "@Component" src/main/java/com/iemr/common/config/CorsConfig.java # Ensure cors.allowed-origins is set in application properties or YAML echo "Searching for cors.allowed-origins in config files..." rg -n "cors\.allowed-origins" --glob "*.properties" --glob "*.yml"Length of output: 545
Ensure CORS configuration is correctly applied and origins are set
The centralized
CorsConfig
bean is properly annotated with@Configuration
and implementsWebMvcConfigurer
, but I noticed thecors.allowed-origins
property is declared yet empty insrc/main/environment/common_ci.properties
:
- CorsConfig picks up
@Value("${cors.allowed-origins}")
cors.allowed-origins=
has no value in CI configPlease populate
cors.allowed-origins
with your allowed origin(s) in each environment’s properties or YAML (e.g.application-dev.yml
,application-prod.properties
) and verify by sending actual cross-origin requests against your endpoints (GET/POST/OPTIONS) to confirm CORS headers are returned as expected.
🧹 Nitpick comments (10)
src/main/environment/common_ci.properties (1)
171-171
: Document the new CORS property.
Thecors.allowed-origins
key is crucial for the dynamic CORS setup. Please add a comment or README entry explaining its expected format (e.g., comma-separated list) and the default behavior when left empty.src/main/java/com/iemr/common/controller/eausadha/EAusadhaController.java (1)
6-6
: Remove unused import.
Since the@CrossOrigin
annotation has been removed, the import fororg.springframework.web.bind.annotation.CrossOrigin
is now unused—please delete it to clean up imports.src/main/java/com/iemr/common/controller/directory/DirectoryController.java (1)
32-33
: Remove unused CrossOrigin importThe CrossOrigin annotation is imported but no longer used in the class since all annotations have been removed as part of centralizing CORS configuration.
-import org.springframework.web.bind.annotation.CrossOrigin;
src/main/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationController.java (1)
30-30
: Clean up unused importThe
CrossOrigin
import is no longer used since all annotations have been removed.-import org.springframework.web.bind.annotation.CrossOrigin;
src/main/java/com/iemr/common/controller/customization/CustomizationController.java (1)
9-9
: Clean up unused importThe
CrossOrigin
import is no longer used and can be removed.-import org.springframework.web.bind.annotation.CrossOrigin;
src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java (1)
29-29
: Remove unused CrossOrigin import
TheCrossOrigin
import is no longer used and should be deleted to clean up warnings.-import org.springframework.web.bind.annotation.CrossOrigin;
src/main/java/com/iemr/common/controller/email/EmailController.java (1)
29-29
: Remove unused CrossOrigin import
Delete the now-unused import to avoid IDE/compiler warnings.-import org.springframework.web.bind.annotation.CrossOrigin;
src/main/java/com/iemr/common/config/CorsConfig.java (1)
20-21
: Consider documenting the purpose of exposed headers.The explicit exposure of "Authorization" and "Jwttoken" headers is correct for authentication functionality, but adding a brief comment explaining why these specific headers need to be exposed would improve code maintainability.
- .exposedHeaders("Authorization", "Jwttoken") // Explicitly expose headers if needed + .exposedHeaders("Authorization", "Jwttoken") // Expose these headers to allow client-side access for authenticationsrc/main/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppController.java (1)
27-28
: Remove unused import.The import for
CrossOrigin
is no longer needed since all@CrossOrigin
annotations have been removed.-import org.springframework.web.bind.annotation.CrossOrigin;
src/main/java/com/iemr/common/utils/DynamicCorsFilter.java (1)
26-29
: Consider setting additional CORS headers for preflight responses.The current implementation only sets the
Access-Control-Allow-Origin
header, but preflight responses typically require additional headers likeAccess-Control-Allow-Methods
,Access-Control-Allow-Headers
, etc. While these might be set by theCorsConfig
class, it would be more robust to set them here as well for preflight OPTIONS requests.if (origin != null && Arrays.asList(allowedOrigins).contains(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", "*"); + response.setHeader("Access-Control-Allow-Credentials", "true"); + response.setHeader("Access-Control-Max-Age", "3600"); + // Add these only for OPTIONS requests to ensure complete preflight response + if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { + response.setHeader("Access-Control-Expose-Headers", "Authorization, Jwttoken"); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
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
(2 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
(0 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
(0 hunks)src/main/java/com/iemr/common/controller/everwellTest/EverwellController.java
(0 hunks)src/main/java/com/iemr/common/controller/feedback/FeedbackController.java
(2 hunks)src/main/java/com/iemr/common/controller/grievance/GrievanceController.java
(1 hunks)src/main/java/com/iemr/common/controller/helpline104history/Helpline104BeneficiaryHistoryController.java
(1 hunks)src/main/java/com/iemr/common/controller/honeywell/HoneywellController.java
(1 hunks)src/main/java/com/iemr/common/controller/institute/InstituteController.java
(2 hunks)src/main/java/com/iemr/common/controller/kmfilemanager/KMFileManagerController.java
(1 hunks)src/main/java/com/iemr/common/controller/language/LanguageController.java
(0 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/NHMDetailCallReportScheduler.java
(0 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/QuestionTypeController.java
(0 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/services/CommonController.java
(0 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
(1 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/java/com/iemr/common/controller/version/VersionController.java
(0 hunks)src/main/java/com/iemr/common/utils/DynamicCorsFilter.java
(1 hunks)src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java
(3 hunks)src/main/resources/application.properties
(1 hunks)
💤 Files with no reviewable changes (10)
- src/main/java/com/iemr/common/controller/language/LanguageController.java
- src/main/java/com/iemr/common/controller/questionconfig/QuestionTypeController.java
- src/main/java/com/iemr/common/controller/version/VersionController.java
- src/main/java/com/iemr/common/controller/services/CommonController.java
- src/main/java/com/iemr/common/controller/carestream/CareStreamCreateOrderController.java
- src/main/java/com/iemr/common/controller/everwellTest/EverwellController.java
- src/main/java/com/iemr/common/controller/brd/BRDIntegrationController.java
- src/main/java/com/iemr/common/controller/everwell/callhandle/EverwellCallController.java
- src/main/java/com/iemr/common/controller/nhmdashboard/NHMDetailCallReportScheduler.java
- src/main/java/com/iemr/common/controller/esanjeevani/ESanjeevaniController.java
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Build
- GitHub Check: Analyze (java)
🔇 Additional comments (86)
src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java (1)
24-24
: Global CORS config applied: annotation removal approved.
The explicit@CrossOrigin
annotation has been correctly removed to defer to the centralizedCorsConfig
. No further changes needed here.src/main/java/com/iemr/common/controller/report/CustomerRelationshipReports.java (1)
57-57
: Global CORS config applied: annotation removal approved.
The method-level@CrossOrigin
annotation has been removed in favor of the global CORS settings defined inCorsConfig
. This aligns with the PR objectives.src/main/java/com/iemr/common/controller/directory/DirectoryController.java (1)
82-82
: LGTM: Removed CrossOrigin annotationsThe removal of CrossOrigin annotations aligns with the PR objective to centralize CORS configuration through a property-based approach. This change improves security by ensuring CORS is managed consistently through environment-specific configuration rather than hard-coded annotations.
Also applies to: 103-103, 127-127, 151-151
src/main/java/com/iemr/common/controller/lungassessment/LungAssessmentController.java (1)
47-47
: LGTM: Removed CrossOrigin annotationsThe removal of CrossOrigin annotations from the controller methods is consistent with the PR objective to centralize CORS configuration. This approach improves security by managing CORS headers through environment-specific properties rather than controller-level annotations.
Also applies to: 65-65, 83-83
src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java (1)
56-56
: LGTM: Removed CrossOrigin annotationsThe removal of CrossOrigin annotations from these controller methods properly implements the centralized CORS configuration approach. This change enhances security by enabling environment-specific control over allowed origins.
Also applies to: 78-78, 100-100
src/main/java/com/iemr/common/controller/location/LocationController.java (1)
51-51
: LGTM: Removed CrossOrigin annotationsThe removal of CrossOrigin annotations from all controller methods aligns with the centralized CORS configuration strategy. This change improves security by enabling environment-specific configuration of allowed origins.
Also applies to: 67-67, 83-83, 100-100, 116-116, 133-133, 154-154
src/main/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReports.java (2)
46-48
: Removed unused CrossOrigin import.
The import for@CrossOrigin
has been removed—good cleanup.
54-56
: Removed per-endpoint@CrossOrigin
annotations.
Centralizing CORS inCorsConfig
removes scattered annotations in these handlers—this is correct and consistent with the new approach.Also applies to: 80-82, 108-110, 136-138, 163-165, 189-191, 214-217, 271-273, 297-299
src/main/java/com/iemr/common/controller/questionconfig/QuestionnaireController.java (2)
56-59
: Removed@CrossOrigin
fromcreateQuestionnaire
.
Relying on the global CORS config instead of per-method annotations is the intended change.
71-74
: Removed@CrossOrigin
fromquestionTypeList
.
This aligns with the centralized CORS configuration strategy.src/main/java/com/iemr/common/controller/sms/SMSController.java (1)
62-65
: Removed@CrossOrigin
annotations from all SMS endpoints.
Each method now relies solely on the global CORS setup—this is correct and cohesive.Also applies to: 81-84, 100-103, 120-123, 143-146, 164-167, 183-186
src/main/java/com/iemr/common/controller/otp/OTPGateway.java (1)
50-54
: Removed@CrossOrigin
from OTP endpoints.
Consolidating CORS handling into the newCorsConfig
and filters ensures uniform behavior across these methods.Also applies to: 73-76, 97-100
src/main/java/com/iemr/common/controller/helpline104history/Helpline104BeneficiaryHistoryController.java (4)
2-21
: License header formatting adjusted.
The multi-line comment block was reformatted for consistent indentation—this is purely stylistic and acceptable.
24-39
: Cleaned up unused imports.
Removed imports forArrays
,List
,Objects
,CrossOrigin
,RequestMethod
, and unused JSON libraries. The remaining imports accurately reflect used classes.
46-50
: Field declarations reformatted.
Indentation ofinputMapper
andlogger
fields was standardized—this improves readability.
53-57
: Switched from@RequestMapping(method = RequestMethod.POST)
to@PostMapping
.
Using the specialized@PostMapping
enhances clarity and removes the need forRequestMethod
imports.src/main/java/com/iemr/common/controller/institute/InstituteController.java (1)
1-183
: CORS annotations removed - part of centralized CORS configurationThe removal of
@CrossOrigin
annotations from this controller aligns with the PR objective to centralize CORS configuration through the new property-based approach. This is a security improvement as it eliminates potentially overly permissive cross-origin settings at the method level.src/main/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationController.java (1)
59-527
: CORS annotations removed - part of centralized CORS configurationThe removal of
@CrossOrigin
annotations from all methods in this controller aligns with the centralized CORS management approach being implemented in this PR.src/main/java/com/iemr/common/controller/services/CategoryController.java (1)
50-67
: CORS annotations removed - part of centralized CORS configurationThe removal of
@CrossOrigin
annotations from this method aligns with the centralized CORS configuration approach.src/main/java/com/iemr/common/controller/customization/CustomizationController.java (1)
33-264
: CORS annotations removed - part of centralized CORS configurationThe removal of
@CrossOrigin
annotations from all methods in this controller is consistent with the PR's objective to centralize CORS configuration via property-based settings.src/main/java/com/iemr/common/controller/snomedct/SnomedController.java (2)
53-53
: Method-level @crossorigin removal is correct
Centralized CORS via CorsConfig replaces per-method annotations—no functionality change here.
79-79
: Removed redundant @crossorigin annotation
Global CORS configuration now handles all origins; method‐level annotations are no longer needed.src/main/java/com/iemr/common/controller/mctshistory/OutboundHistoryController.java (2)
47-47
: Removed method-level @crossorigin
Centralized CORS config applies to this endpoint—removal aligns with the new globals.
63-63
: Removed @crossorigin from second endpoint
Consistent with migrating all controller CORS to CorsConfig.src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java (3)
62-62
: Removed @crossorigin on GET master endpoint
CORS is now managed by CorsConfig; method annotations are redundant.
87-87
: Removed @crossorigin on POST getCovidVaccinationDetails
Global CORS covers this—removal is correct.
131-131
: Removed @crossorigin on POST saveCovidVaccinationDetails
Endpoint will use centralized CORS rules from CorsConfig.src/main/java/com/iemr/common/controller/email/EmailController.java (3)
58-58
: @crossorigin annotation removed for SendEmail
Centralized CORS config supersedes per-method annotations.
77-77
: Removed @crossorigin from getAuthorityEmailID
Consistent with global CORS configuration in CorsConfig.
92-92
: Removed @crossorigin from sendEmailGeneral
Global handler now applies CORS policy uniformly.src/main/java/com/iemr/common/controller/scheme/SchemeController.java (3)
54-54
: Removed @crossorigin on saveSchemeDetails
CorsConfig provides CORS for all paths—method annotations are redundant.
81-81
: Removed @crossorigin on getSchemeList
Leverages centralized CORS settings; removal is expected.
107-107
: Removed @crossorigin on deleteScheme
Global CORS configuration covers this endpoint.src/main/java/com/iemr/common/controller/notification/NotificationController.java (13)
51-54
: Removed method-level @crossorigin for getNotification
Per the new centralized CORS configuration in CorsConfig, the explicit@CrossOrigin
annotation above thegetNotification
endpoint has been removed. This aligns with the PR objective to manage CORS via a property-driven, global approach.
71-74
: Removed method-level @crossorigin for getSupervisorNotification
The@CrossOrigin
annotation on thegetSupervisorNotification
method has been deleted to defer CORS handling to the global config. No functional logic was affected.
91-94
: Removed method-level @crossorigin for createNotification
Deletion of the@CrossOrigin
annotation oncreateNotification
is correct under the new global CORS setup.
119-122
: Removed method-level @crossorigin for updateNotification
The explicit CORS annotation forupdateNotification
is no longer needed with the centralized implementation.
145-148
: Removed method-level @crossorigin for getNotificationType
Clearing the per-endpoint CORS tag here follows the shift to CorsConfig-based handling.
181-184
: Removed method-level @crossorigin for createNotificationType
Consistent with the global CORS configuration, the endpoint-level annotation has been removed.
201-204
: Removed method-level @crossorigin for updateNotificationType
This change correctly centralizes CORS policy application-wide.
219-222
: Removed method-level @crossorigin for getEmergencyContacts
EndpointgetEmergencyContacts
no longer carries an inline CORS directive, relying on CorsConfig.
234-237
: Removed method-level @crossorigin for getSupervisorEmergencyContacts
Removal is aligned with property-driven origin management.
257-260
: Removed method-level @crossorigin for createEmergencyContacts
Deleting this annotation ensures all CORS rules emanate from the centralized config.
279-282
: Removed method-level @crossorigin for updateEmergencyContacts
Appropriate clean-up of the inline CORS annotation; no logic changes.
444-449
: Removed deprecated userAuthenticateV1 method
Cleaning up commented-out legacy code reduces noise. This unused, commented block can be safely deleted.
472-476
: Removed method-level @crossorigin for getLoginResponse
ThegetLoginResponse
endpoint now relies on the newDynamicCorsFilter
and global CORS mappings instead of inline annotations.src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (13)
105-108
: Removed method-level @crossorigin for userAuthenticateNew
The inline CORS annotation aboveuserAuthenticateNew
is gone, deferring origin checks to the centralized CORS configuration.
120-123
: Removed method-level @crossorigin for userAuthenticate
Deletion aligns this critical authentication endpoint with the global CORS policy.
279-283
: Removed method-level @crossorigin for refreshToken
Inline CORS no longer needed—requests will be filtered byDynamicCorsFilter
and CorsConfig.
317-320
: Removed method-level @crossorigin for logOutUserFromConcurrentSession
Streamlines endpoint definitions and defers CORS to the new global setup.
444-449
: Removed commented-out legacy userAuthenticateV1 code
Dropping this dead code improves readability and reduces maintenance burden.
472-476
: Removed method-level @crossorigin for getLoginResponse
Consistent CORS enforcement now happens at the filter/config level.
650-653
: Removed method-level @crossorigin for forgetPassword
Clean removal of inline CORS annotation; global config handles preflight and origins.
667-670
: Removed method-level @crossorigin for setForgetPassword
Inline CORS directive removed in favor of centralized management.
709-712
: Removed method-level @crossorigin for changePassword
Endpoint updated to rely onDynamicCorsFilter
and property-based allowed origins.
731-734
: Removed method-level @crossorigin for saveUserSecurityQuesAns
Now CORS is uniformly managed without repetitive annotations.
769-772
: Removed method-level @crossorigin for getSecurityts
CORS logic is no longer at the controller layer—central config is authoritative.
818-821
: Removed method-level @crossorigin for forceLogout
Consistent with global CORS approach; no inline annotations remain.
909-912
: Removed method-level @crossorigin for userAuthenticateByEncryption
Defers cross-origin checks to the global filter and configuration.src/main/java/com/iemr/common/controller/feedback/FeedbackController.java (2)
2-21
: Standardized license header formatting
The license block has been reformatted for consistency across controllers. No change to legal text.
60-63
: Removed class-level @crossorigin on FeedbackController
Shifting to CorsConfig centralizes CORS rules; this annotation is no longer required.src/main/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardController.java (1)
45-48
: Removed method-level @crossorigin for pushAbandonCallsFromC_Zentrix
Inline CORS annotation removal is appropriate since CorsConfig now applies global origin rules.src/main/java/com/iemr/common/controller/kmfilemanager/KMFileManagerController.java (2)
2-21
: Standardized license header formatting
Header reformatted for consistency; legal content unchanged.
48-50
: Removed class-level @crossorigin on KMFileManagerController
Global CORS configuration via CorsConfig replaces scattered annotations.src/main/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationController.java (3)
1-21
: Code formatting and license header improvements look good.The license header has been properly formatted with consistent indentation, making it more readable.
77-79
: Clean annotation and class declaration.The spacing and formatting around the
@RequestMapping
annotation and class declaration follow good Java conventions.
81-577
: Consistent indentation and formatting throughout the controller.The code now has consistent indentation, proper spacing between methods, and good organization of service injections and controller methods. This improves readability and maintainability.
src/main/java/com/iemr/common/controller/callhandling/CallController.java (1)
73-608
: Consistent spacing between methods improves readability.The addition of consistent empty lines between method declarations improves the code's readability and organization. This is a good practice for separating logical blocks of code.
src/main/java/com/iemr/common/controller/lonic/LonicController.java (3)
1-21
: License header formatting is improved.The license header now has consistent indentation, making it more readable.
42-75
: Well-formatted code with proper indentation.The code is now properly indented with consistent spacing, improving readability and maintainability.
51-75
: @crossorigin annotation has been removed as part of CORS centralization.This change aligns with the PR objective of centralizing CORS configuration. The controller will now use the global CORS configuration from the new CorsConfig class, which provides more security and flexibility.
src/main/java/com/iemr/common/controller/uptsu/UPTechnicalSupportController.java (3)
1-21
: License header formatting improvements.The license header now has consistent indentation, making it more readable.
40-82
: Consistent formatting and indentation throughout the controller.The code now has proper indentation, consistent spacing, and clear organization of methods, improving readability and maintainability.
49-63
: @crossorigin annotations removed as part of CORS centralization.Removing method-level CORS annotations aligns with the PR objective of centralizing CORS configuration. This approach provides better security by using environment-specific allowed origins.
Also applies to: 66-81
src/main/java/com/iemr/common/controller/honeywell/HoneywellController.java (4)
38-41
: LGTM: Removed@CrossOrigin
annotation in favor of centralized CORS configuration.The removal of
@CrossOrigin
annotations at the controller level is consistent with the PR objective of centralizing CORS configuration through the newCorsConfig
class. This approach enhances security by allowing environment-specific control over allowed origins.
48-59
: LGTM: Removed@CrossOrigin
annotation from method while preserving functionality.The removal of the method-level
@CrossOrigin
annotation is consistent with the PR objective of centralizing CORS configuration. The method's functionality, error handling, and response generation remain intact.
62-74
: LGTM: Removed@CrossOrigin
annotation from method while preserving functionality.The removal of the method-level
@CrossOrigin
annotation is consistent with the PR objective of centralizing CORS configuration. The method's functionality, error handling, and response generation remain intact.
77-89
: LGTM: Removed@CrossOrigin
annotation from method while preserving functionality.The removal of the method-level
@CrossOrigin
annotation is consistent with the PR objective of centralizing CORS configuration. The method's functionality, error handling, and response generation remain intact.src/main/java/com/iemr/common/config/CorsConfig.java (1)
1-25
: LGTM: Well-implemented centralized CORS configuration.The new
CorsConfig
class provides a centralized and secure approach to CORS management:
- Properly reads allowed origins from an external property
- Applies comprehensive CORS settings to all endpoints
- Explicitly exposes necessary headers for authentication
- Sets appropriate security constraints
This implementation aligns perfectly with the PR objectives to enhance security by enabling environment-specific control over allowed origins.
src/main/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppController.java (3)
42-45
: LGTM: Removed@CrossOrigin
annotation in favor of centralized CORS configuration.The removal of
@CrossOrigin
annotations at the controller level is consistent with the PR objective of centralizing CORS configuration through the newCorsConfig
class. This approach enhances security by allowing environment-specific control over allowed origins.
52-71
: LGTM: Removed@CrossOrigin
annotation from method while preserving functionality.The removal of the method-level
@CrossOrigin
annotation is consistent with the PR objective of centralizing CORS configuration. The method's functionality, error handling, and response generation remain intact.
73-92
: LGTM: Removed@CrossOrigin
annotation from method while preserving functionality.The removal of the method-level
@CrossOrigin
annotation is consistent with the PR objective of centralizing CORS configuration. The method's functionality, error handling, and response generation remain intact.src/main/java/com/iemr/common/utils/DynamicCorsFilter.java (2)
1-37
: LGTM: Dynamic CORS filter implementation complements the centralized configuration.This filter provides dynamic CORS handling based on the request's origin, which complements the static configuration in
CorsConfig
. The implementation correctly:
- Validates the origin against the allowed list
- Sets appropriate CORS headers for valid origins
- Short-circuits OPTIONS requests (preflight) with a 200 OK response
This implementation aligns with the PR objectives to enhance security through environment-specific CORS configuration.
31-35
:❓ Verification inconclusive
Verify coordination with other filters that might need to process OPTIONS requests.
The filter short-circuits OPTIONS requests without proceeding in the filter chain. This is typically correct for CORS preflight handling but might bypass other filters that should process OPTIONS requests. Verify that this approach is compatible with the rest of your security architecture.
🏁 Script executed:
#!/bin/bash # Search for other filters that might process OPTIONS requests rg "OPTIONS|HttpMethod.OPTIONS" --type java -A 5 -B 5 --no-heading --no-filename --no-line-numberLength of output: 2224
Ensure OPTIONS preflight handling won’t bypass other filters
Your custom CORS filter and the JWT validation filter both short-circuit OPTIONS requests, so you need to confirm that no critical processing is skipped:
- Verify the registration order of
DynamicCorsFilter
vs.JwtUserIdValidationFilter
(and any other security or logging filters) in yourWebSecurityConfigurer
orFilterRegistrationBean
setup.- If you rely on Spring MVC’s
addCorsMappings
(viaCorsRegistry
), ensure you’re not duplicating or conflicting with the custom filter.- Adjust filter order or merge CORS preflight logic into a single, well-ordered component so that all required filters still execute (or explicitly skip) as intended.
src/main/java/com/iemr/common/controller/grievance/GrievanceController.java (1)
123-156
: Good improvement in response handling!The change from returning a JSON string to using Spring's
ResponseEntity
with a structured map is a good practice. This approach:
- Leverages Spring's built-in HTTP response handling
- Provides a consistent response structure
- Maintains proper error handling with detailed information
- Eliminates manual JSON serialization
This change aligns with modern RESTful API best practices.
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (1)
50-54
: Good implementation of OPTIONS request handlingCorrectly handling preflight OPTIONS requests by returning 200 OK status and skipping JWT validation is a good practice for CORS implementation.
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java
Outdated
Show resolved
Hide resolved
@@ -168,5 +168,7 @@ grievanceAllocationRetryConfiguration=3 | |||
start-grievancedatasync-scheduler=false | |||
cron-scheduler-grievancedatasync=0 0/2 * * * ? | |||
|
|||
cors.allowed-origins= |
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.
@@ variable seems missing
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.
Updated
@drtechie Please find the tested result when Success with 4209 to 8083 when fail |
@@ -192,3 +192,4 @@ grievanceAllocationRetryConfiguration=3 | |||
logging.path=logs/ | |||
logging.file.name=logs/common-api.log | |||
|
|||
cors.allowed-origins=http://localhost:4000,http://localhost:3000 |
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.
shouldn't we use localhost:* here?
|
📋 Description
JIRA ID: AMM-1246
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
Created a CorsConfig class to manage CORS settings using Spring's WebMvcConfigurer. Previously, the application allowed all origins by default, which is not secure. This change introduces dynamic origin handling based on the cors.allowed-origins property defined in the environment (.env or application properties file).
By moving to a property-driven approach, we can now specify allowed origins per environment (e.g., dev, staging, production), ensuring better control and security over cross-origin requests.
✅ 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
Bug Fixes
Style