-
Notifications
You must be signed in to change notification settings - Fork 45
CORS Configuration for HWC API Services #138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughThis update centralizes and standardizes Cross-Origin Resource Sharing (CORS) configuration by removing all Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Filter as JwtUserIdValidationFilter
participant Spring as CorsConfig (WebMvcConfigurer)
participant Controller
Client->>Filter: HTTP Request with Origin header
Filter->>Filter: Check Origin against allowed origins
alt Origin allowed
Filter->>Client: Add CORS headers to response
alt OPTIONS preflight
Filter-->>Client: Respond 200 OK (CORS headers only)
else Other methods
Filter->>Controller: Forward request
Controller-->>Filter: Response
Filter->>Client: Return response with CORS headers
end
else Origin not allowed
Filter->>Controller: Forward request (no CORS headers)
Controller-->>Filter: Response
Filter->>Client: Return response
end
Poem
β¨ 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: 6
π Outside diff range comments (1)
src/main/java/com/iemr/hwc/controller/videoconsultation/VideoConsultationController.java (1)
68-75: Path-variable name mismatch breaks the/call/{β¦}endpoint
@GetMapping("/call/{fromUserID}/{touserID}")declares the second segment astouserID, but the method expects a variable namedtoUserID.
Spring treats the names as case-sensitive; the mismatch causes404 Not Found.-@GetMapping(value = "/call/{fromUserID}/{touserID}", headers = "Authorization", produces = { "application/json" }) +@GetMapping(value = "/call/{fromUserID}/{toUserID}", headers = "Authorization", produces = { "application/json" })
β»οΈ Duplicate comments (2)
src/main/java/com/iemr/hwc/controller/diabetesAndHypertensionOutcome/DiabetesHypertensionScreeningController.java (1)
39-41: Ensure global CORS configuration applies here. Refer to the verification steps inSnomedController.src/main/java/com/iemr/hwc/controller/registrar/master/RegistrarMasterController.java (1)
41-43: Ensure global CORS configuration applies here. Refer to the verification inSnomedController.
π§Ή Nitpick comments (18)
src/main/java/com/iemr/hwc/controller/patientApp/master/PatientAppCommonMasterController.java (2)
64-64: Remove stray semicolon. There's an unnecessary standalone;that should be removed to clean up the code.
78-78: Remove stray semicolon. Another standalone;can be removed for consistency.src/main/java/com/iemr/hwc/controller/version/VersionController.java (1)
61-66: Handle missinggit.properties. Add a null-check forgetResourceAsStream("git.properties")to avoid aNullPointerExceptionif the file isn't on the classpath.ClassLoader classLoader = getClass().getClassLoader(); - InputStream inputStream = classLoader.getResourceAsStream("git.properties"); + InputStream inputStream = classLoader.getResourceAsStream("git.properties"); + if (inputStream == null) { + throw new FileNotFoundException("git.properties not found in classpath"); + }src/main/java/com/iemr/hwc/controller/uptsu/UPTechnicalSupportUnitController.java (1)
28-31: Importing ObjectUtils for null/empty checks
UsingObjectUtils.isEmptyto validate theReferred104Detailsresponse works, but a directnullcheck may be clearer for POJOs.src/main/java/com/iemr/hwc/controller/familyPlanning/FamilyPlanningController.java (1)
29-36: Remove now-unusedRequestMethodimport
RequestMethodis no longer referenced after the class-level@RequestMappinglost itsmethod = β¦attribute when@CrossOriginannotations were removed.
Keeping it will trigger IDE / compiler warnings.-import org.springframework.web.bind.annotation.RequestMethod;src/main/java/com/iemr/hwc/controller/pnc/PostnatalCareController.java (1)
29-36: Drop the unusedRequestMethodimport
RequestMethodis not referenced anywhere in this class since per-method mappings use@PostMapping/@GetMapping. Safe to delete.-import org.springframework.web.bind.annotation.RequestMethod;src/main/java/com/iemr/hwc/controller/videoconsultation/VideoConsultationController.java (1)
24-31: Prune the unusedRequestMethodimportAfter removing
@CrossOrigin,RequestMethodis no longer necessary.-import org.springframework.web.bind.annotation.RequestMethod;src/main/java/com/iemr/hwc/controller/spoke/VanSpokeController.java (1)
28-35: Eliminate the redundantRequestMethodimportNo reference to
RequestMethodexists in the class; deleting avoids dead-code warnings.-import org.springframework.web.bind.annotation.RequestMethod;src/main/java/com/iemr/hwc/controller/ncdscreening/NCDScreeningController.java (1)
30-30: Imported@Transactionalat controller β consider refactoring.
Transaction management belongs in the service layer rather than the controller. Move the@Transactional(rollbackFor = Exception.class)usage into the corresponding service method to maintain separation of concerns.src/main/java/com/iemr/hwc/controller/teleconsultation/TeleConsultationController.java (1)
72-73: Inconsistent@RequestHeaderusage for Authorization.
Other controllers explicitly use@RequestHeader(HttpHeaders.AUTHORIZATION). For consistency, specify the header name here as well.src/main/java/com/iemr/hwc/controller/foetalmonitor/FoetalMonitorController.java (1)
32-32: Mixed return types βStringvsResponseEntity<String>.
Some endpoints now returnResponseEntity<String>, while others still return rawString. Standardize onResponseEntity<T>for all methods to ensure consistent HTTP status handling.src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java (2)
27-33: Property duplication β steer clear of configuration drift
allowedOriginsis injected again in this filter even though the same property is already consumed byCorsConfig. Two independent consumers mean double parsing, duplicated logic, and a high risk of the two implementations diverging.Consider removing CORS handling from this filter and relying solely on the central
CorsConfig, or injectCorsConfig(or a sharedCorsPropertiesbean) rather than the raw string.
78-80: Missing token value in log statement
logger.info("JWT token from header: ");never prints the token value stored injwtTokenFromHeader. Either remove the statement or append the variable to aid debugging.src/main/java/com/iemr/hwc/controller/masterVillage/MasterVillageController.java (2)
55-68: Deeply nested conditional β consider early-return or switch-like mappingThe four-way
if / else-ifchain onrespis harder to scan and easy to extend incorrectly. A small map or early-return pattern simplifies intent:switch (resp) { case "not_ok" -> response.setError(500, "Error setting master village"); case "villageID_not_exist"-> response.setError(404, "Village ID does not exist"); case "userID_not_exist" -> response.setError(404, "User ID does not exist"); default -> response.setResponse(resp); }Reduces branching depth and keeps success & failure paths distinct.
85-96: Error messages expose internal stateReturning
βNo master village record found with userID: β¦βleaks internal identifiers to unauthorised callers. Prefer a generic 404 message and log the details server-side:- response.setError(404, "No master village record found with userID: " + userID); + logger.error("No master village record found for userID {}", userID); + response.setError(404, "Master village record not found");src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java (1)
24-32: Remove the unusedRequestMethodimport
RequestMethodis no longer referenced after switching to@PostMappingon every endpoint. Keeping an unused import produces a compiler warning and clutters the file.@@ -import org.springframework.web.bind.annotation.RequestMethod;src/main/java/com/iemr/hwc/controller/quickconsult/QuickConsultController.java (1)
24-36: Clean up unused imports
RequestMethodis no longer referenced;@Paramfromspring-datais not meant for controller DTO hints and can be removed or replaced with@io.swagger.v3.oas.annotations.Parameterif you only need it for the OpenAPI description.@@ -import org.springframework.data.repository.query.Param; -import org.springframework.web.bind.annotation.RequestMethod; +import io.swagger.v3.oas.annotations.Parameter; // if still requiredRemoving these eliminates compiler warnings and prevents accidental misuse of
@Param.src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java (1)
24-36: Remove unused imports
RequestMethodis not referenced after adopting specific@PostMappingannotations.@@ -import org.springframework.web.bind.annotation.RequestMethod;
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (38)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/hwc/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.java(1 hunks)src/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.java(1 hunks)src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.java(1 hunks)src/main/java/com/iemr/hwc/controller/choApp/CHOAppSyncController.java(10 hunks)src/main/java/com/iemr/hwc/controller/common/main/WorklistController.java(1 hunks)src/main/java/com/iemr/hwc/controller/common/master/CommonMasterController.java(1 hunks)src/main/java/com/iemr/hwc/controller/covid19/CovidController.java(1 hunks)src/main/java/com/iemr/hwc/controller/dataSyncActivity/StartSyncActivity.java(1 hunks)src/main/java/com/iemr/hwc/controller/dataSyncLayerCentral/MMUDataSyncController.java(1 hunks)src/main/java/com/iemr/hwc/controller/diabetesAndHypertensionOutcome/DiabetesHypertensionScreeningController.java(1 hunks)src/main/java/com/iemr/hwc/controller/familyPlanning/FamilyPlanningController.java(1 hunks)src/main/java/com/iemr/hwc/controller/foetalmonitor/FoetalMonitorController.java(3 hunks)src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java(2 hunks)src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java(2 hunks)src/main/java/com/iemr/hwc/controller/location/LocationController.java(1 hunks)src/main/java/com/iemr/hwc/controller/masterVillage/MasterVillageController.java(3 hunks)src/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.java(1 hunks)src/main/java/com/iemr/hwc/controller/ncdscreening/NCDScreeningController.java(1 hunks)src/main/java/com/iemr/hwc/controller/neonatal/NeonatalController.java(1 hunks)src/main/java/com/iemr/hwc/controller/patientApp/master/PatientAppCommonMasterController.java(1 hunks)src/main/java/com/iemr/hwc/controller/pnc/PostnatalCareController.java(1 hunks)src/main/java/com/iemr/hwc/controller/quickBlox/QuickbloxController.java(1 hunks)src/main/java/com/iemr/hwc/controller/quickconsult/QuickConsultController.java(1 hunks)src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java(1 hunks)src/main/java/com/iemr/hwc/controller/registrar/master/RegistrarMasterController.java(1 hunks)src/main/java/com/iemr/hwc/controller/report/CRMReportController.java(1 hunks)src/main/java/com/iemr/hwc/controller/snomedct/SnomedController.java(1 hunks)src/main/java/com/iemr/hwc/controller/spoke/VanSpokeController.java(1 hunks)src/main/java/com/iemr/hwc/controller/teleconsultation/TeleConsultationController.java(1 hunks)src/main/java/com/iemr/hwc/controller/uptsu/UPTechnicalSupportUnitController.java(1 hunks)src/main/java/com/iemr/hwc/controller/version/VersionController.java(2 hunks)src/main/java/com/iemr/hwc/controller/videoconsultation/VideoConsultationController.java(1 hunks)src/main/java/com/iemr/hwc/controller/wo/LocationControllerWo.java(0 hunks)src/main/java/com/iemr/hwc/controller/wo/UserBiomectrics.java(4 hunks)src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java(4 hunks)
π€ Files with no reviewable changes (1)
- src/main/java/com/iemr/hwc/controller/wo/LocationControllerWo.java
π§° Additional context used
𧬠Code Graph Analysis (1)
src/main/java/com/iemr/hwc/utils/JwtUserIdValidationFilter.java (1)
src/main/java/com/iemr/hwc/utils/JwtAuthenticationUtil.java (1)
Component(21-132)
π Additional comments (32)
src/main/environment/common_ci.properties (1)
100-100: Centralize CORS origins via environment variable
Youβve addedcors.allowed-origins=@env.CORS_ALLOWED_ORIGINS@to source allowed origins at runtime. This aligns with the global CORS setup. Ensure that in absence ofCORS_ALLOWED_ORIGINSthe application fails fast or falls back to a safe default.src/main/environment/common_example.properties (1)
99-99: Provide localhost default for CORS in examples
The example file now includescors.allowed-origins=http://localhost:*, which is useful for local testing. Verify that this wildcard pattern is correctly parsed and does not permit unintended origins in production.src/main/java/com/iemr/hwc/controller/location/LocationController.java (1)
28-28: Remove local @crossorigin import
Good removal of the@CrossOriginimport in favor of centralizing CORS viaCorsConfig. Please confirm that your global configuration covers all/location/**endpoints as intended.src/main/java/com/iemr/hwc/controller/common/master/CommonMasterController.java (1)
27-27: Drop redundant CrossOrigin import
Removing controller-level CORS imports is consistent with the new global policy. Double-check that/master/**mappings are included in yourCorsConfig.src/main/java/com/iemr/hwc/controller/ncdCare/NCDCareController.java (1)
33-33: Eliminate per-controller CORS import
The transactional import remains, and the@CrossOrigindependency is removed correctly. Ensure that/NCDCare/**is covered by your centralized CORS configuration.src/main/java/com/iemr/hwc/controller/snomedct/SnomedController.java (2)
28-28: Remove redundant import:CrossOrigin. Controller-level CORS annotations are now centralized inCorsConfig.
43-57: Confirm global CORS configuration for this endpoint. Ensure that the newCorsConfigand enhancedJwtUserIdValidationFiltercorrectly apply CORS headers (including preflight OPTIONS) to/snomed/**requests now that@CrossOriginis removed.#!/bin/bash # Verify global CORS mappings and allowed origins grep -R "addMapping" -n src/main/java/com/iemr/hwc/config/CorsConfig.java grep -R "cors.allowed-origins" -n srcsrc/main/java/com/iemr/hwc/controller/diabetesAndHypertensionOutcome/DiabetesHypertensionScreeningController.java (1)
28-28: Remove redundant import:CrossOrigin. Controller annotations now leverage the centralized CORS setup.src/main/java/com/iemr/hwc/controller/registrar/master/RegistrarMasterController.java (1)
29-29: Remove redundant import:CrossOrigin. This controller will now inherit CORS settings from the global configuration.src/main/java/com/iemr/hwc/controller/patientApp/master/PatientAppCommonMasterController.java (1)
27-27: Remove redundant import:CrossOrigin. CORS is now managed centrally.src/main/java/com/iemr/hwc/controller/version/VersionController.java (1)
31-31: Remove redundant import:CrossOrigin. Global CORS config supersedes controller-level annotations.src/main/java/com/iemr/hwc/controller/adolescent/ChildhoodAdolescenceController.java (2)
30-32: Transactional import added
Importingorg.springframework.transaction.annotation.Transactionalsupports the method-level@Transactional(rollbackFor = Exception.class)annotations used on fetch endpoints.
1-420: I've verified thatCorsConfig.javaexists undersrc/main/java/com/iemr/hwc/config. To ensure there are no leftover controller-level CORS annotations, letβs correctly search for@CrossOriginin the controller package:#!/bin/bash # Properly search for any @CrossOrigin annotations in controllers rg "@CrossOrigin" src/main/java/com/iemr/hwc/controllersrc/main/java/com/iemr/hwc/controller/covid19/CovidController.java (1)
31-33: Transactional import added
The addition oforg.springframework.transaction.annotation.Transactionalimport aligns with transactional annotations on critical endpoints.src/main/java/com/iemr/hwc/controller/dataSyncActivity/StartSyncActivity.java (1)
28-28: Removed@CrossOriginimport
Explicit@CrossOriginannotations have been removed in favor of the centralized CORS setup. Confirm controllers rely on the global configuration.src/main/java/com/iemr/hwc/controller/quickBlox/QuickbloxController.java (1)
26-28: Removed@CrossOriginimport
Controller-level CORS annotations are now centralized; ensure there are no leftoverCrossOriginimports here.src/main/java/com/iemr/hwc/controller/report/CRMReportController.java (1)
27-34: CORS cleanup looks good
@CrossOriginimport removal and reliance on the newCorsConfigis consistent with the refactor. No further action required here.src/main/java/com/iemr/hwc/controller/ncdscreening/NCDScreeningController.java (1)
56-57: Removed@CrossOriginβ validate centralized CORS coverage.
All per-controller CORS annotations were removed. Confirm that your globalCorsConfigincludes a mapping for/NCD/**so these endpoints remain CORS-enabled.#!/bin/bash # Verify global CORS mapping for NCD endpoints rg "addMapping" -n src/main/java/com/iemr/hwc/config/CorsConfig.javasrc/main/java/com/iemr/hwc/controller/choApp/CHOAppSyncController.java (3)
42-43: Non-functional comment formatting changes; no action needed.Also applies to: 54-55
59-59: StandardizedAuthorizationheader binding.
Great improvement using@RequestHeader(HttpHeaders.AUTHORIZATION)across methods. Please verify compatibility with your global security filter.Also applies to: 68-69, 77-79
39-41: Removed@CrossOriginβ verify global CORS applies to/sync/**.
Ensure that the centralized CORS policy inCorsConfig.javaincludes the/sync/**path.#!/bin/bash # Confirm CORS mapping for sync endpoints rg "addMapping" -n src/main/java/com/iemr/hwc/config/CorsConfig.javasrc/main/java/com/iemr/hwc/controller/teleconsultation/TeleConsultationController.java (1)
27-27: Removed@CrossOriginimport β ensure removal completeness.
Verify there are no leftover@CrossOriginannotations and that your global CORS configuration covers/tc/**.#!/bin/bash # Check for any remaining @CrossOrigin rg "@CrossOrigin" -n src/main/java/com/iemr/hwc/controller/teleconsultation # Verify CORS registry mappings rg "addMapping" -n src/main/java/com/iemr/hwc/config/CorsConfig.javasrc/main/java/com/iemr/hwc/controller/foetalmonitor/FoetalMonitorController.java (1)
64-65: Removed@CrossOriginβ validate global CORS for/foetalMonitor/**.
Confirm that/foetalMonitor/**is covered by your centralized CORS configuration so cross-origin calls continue to work.#!/bin/bash rg "addMapping" -n src/main/java/com/iemr/hwc/config/CorsConfig.javasrc/main/java/com/iemr/hwc/controller/dataSyncLayerCentral/MMUDataSyncController.java (1)
27-27: Removed@CrossOriginimport β verify CORS policy inclusion.
Ensure/dataSync/**endpoints are now served under the global CORS rules configured inCorsConfig.java.#!/bin/bash rg "addMapping" -n src/main/java/com/iemr/hwc/config/CorsConfig.javasrc/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java (1)
32-33: ```shell
#!/bin/bashVerify no @crossorigin annotations remain
rg -n '@crossorigin' src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java
Verify no CrossOrigin import remains
rg -n 'import.*CrossOrigin' src/main/java/com/iemr/hwc/controller/registrar/main/RegistrarController.java
</details> <details> <summary>src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.java (1)</summary> `29-31`: **Deleted `@CrossOrigin` import to centralize CORS handling** The removed import supports the shift to a global CORS configuration. Verify no stray `@CrossOrigin` annotations or imports remain. Run: ```shell #!/bin/bash rg -n "@CrossOrigin" src/main/java/com/iemr/hwc/controller/cancerscreening/CancerScreeningController.javasrc/main/java/com/iemr/hwc/controller/neonatal/NeonatalController.java (1)
31-33: Removed@CrossOriginimport for consistency
This deletion aligns with the introduction of a global CORS filter. Please ensure there are no leftover@CrossOriginusages in this controller.Run:
#!/bin/bash rg -n "@CrossOrigin" src/main/java/com/iemr/hwc/controller/neonatal/NeonatalController.javasrc/main/java/com/iemr/hwc/controller/common/main/WorklistController.java (1)
30-32: ```shell
#!/bin/bashSearch for any @crossorigin annotations in all controllers
rg -n "@crossorigin" src/main/java/com/iemr/hwc/controller
Search for any CrossOrigin imports in all controllers
rg -n "import .*CrossOrigin" src/main/java/com/iemr/hwc/controller
</details> <details> <summary>src/main/java/com/iemr/hwc/controller/labtechnician/LabTechnicianController.java (1)</summary> `48-50`: **Verify CORS pre-flight behavior with the `headers = "Authorization"` mapping constraint** The class-level `@RequestMapping` demands the presence of an `Authorization` header. Browser pre-flight `OPTIONS` requests do **not** include this header; Spring will therefore skip this handler mapping and may return `404/405`, breaking CORS even though you now provide a global `CorsConfig`. Please double-check that either 1. the new `CorsConfig` registers its own `CorsConfigurationSource` that bypasses handler mapping evaluation, **or** 2. the header constraint is removed / moved into a security filter instead of the mapping. </details> <details> <summary>src/main/java/com/iemr/hwc/controller/quickconsult/QuickConsultController.java (1)</summary> `54-56`: **CORS pre-flight vs. `headers = "Authorization"` (same concern as other controllers)** All endpoints inherit the `headers = "Authorization"` constraint. Please confirm that the new central CORS configuration or security filter answers `OPTIONS` requests *before* this constraint is evaluated; otherwise CORS will fail for browsers. </details> <details> <summary>src/main/java/com/iemr/hwc/controller/generalOPD/GeneralOPDController.java (1)</summary> `52-54`: **Confirm CORS pre-flight compatibility** Same note as other controllers: the `headers = "Authorization"` condition may reject browser pre-flight requests. Validate against the new `CorsConfig`. </details> <details> <summary>src/main/java/com/iemr/hwc/controller/anc/AntenatalCareController.java (1)</summary> `29-31`: **Removed perβcontroller CORS import; ensure global config applies** The import and any `@CrossOrigin` annotations have been stripped in favor of your centralized CORS configuration. Confirm that the global `CorsConfig` bean is picked up and that no residual controller-level CORS settings remain. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->



π Description
JIRA ID: AMM-1246
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
β Type of Change
βΉοΈ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Refactor
@CrossOriginannotations from controllers in favor of global CORS configuration.Style
Bug Fixes