-
Notifications
You must be signed in to change notification settings - Fork 25
CORS Configuration for HELP104 API Services #59
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 enhances Cross-Origin Resource Sharing (CORS) configuration for the application. It removes all Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Filter as JwtUserIdValidationFilter
participant Spring as Spring Dispatcher
participant Controller
Browser->>Filter: HTTP Request (with Origin header)
alt OPTIONS (Preflight)
Filter->>Browser: Set CORS headers, 200 OK, skip JWT validation
else Non-OPTIONS
Filter->>Filter: Check Origin against allowed origins
alt Origin allowed
Filter->>Browser: Set CORS headers
Filter->>Filter: Validate JWT (skip for certain paths/clients)
alt JWT valid or bypassed
Filter->>Spring: Forward request
Spring->>Controller: Route to endpoint
Controller-->>Spring: Response
Spring-->>Filter: Response
Filter-->>Browser: Response with CORS headers
else JWT invalid
Filter-->>Browser: 401 Unauthorized
end
else Origin not allowed
Filter-->>Browser: 403 Forbidden (no CORS headers)
end
end
Possibly related PRs
Suggested reviewers
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: 5
π Outside diff range comments (3)
src/main/java/com/iemr/helpline104/controller/covidMaster/CovidMasterController.java (1)
26-26: Avoid mixing JAX-RS and Spring MediaType imports
You're importingjakarta.ws.rs.core.MediaTypebut using it in Spring MVC annotations. Preferorg.springframework.http.MediaTypefor consistency and clarity.src/main/java/com/iemr/helpline104/controller/directory/DirectoryServicesController.java (1)
53-55: Inconsistent endpoint naming
The mapping"/getdirectorySearchHistory"is missing a slash afterget. It should be"/get/directorySearchHistory"to align with the naming conventions used elsewhere.src/main/java/com/iemr/helpline104/controller/epidemicOutbreak/EpidemicOutbreakController.java (1)
108-112: Compile-time failure β wrong variable used inupdateEpidemicOutbreakComplaint
InputMapper.gson()is referenced statically, butInputMapperexposesgson()as an instance method (as used elsewhere in the same class). This line will not compile unless a matching static method exists.- T_EpidemicOutbreak t_epidemicOutbreak = InputMapper.gson().fromJson(request, T_EpidemicOutbreak.class); + T_EpidemicOutbreak t_epidemicOutbreak = inputMapper.gson().fromJson(request, T_EpidemicOutbreak.class);Please fix to restore build.
π§Ή Nitpick comments (8)
src/main/java/com/iemr/helpline104/service/CTI/CTIService.java (1)
29-29: Remove unused import
TheRequestMappingimport isnβt used in this@Serviceclass. Cleaning up unused imports improves code maintainability.src/main/java/com/iemr/helpline104/controller/foodSafetyComplaint/FoodSafetyComplaintController.java (1)
67-68: Remove commented-out logging
The two commentedlogger.infolines are inactive and can be removed to clean up the code.src/main/java/com/iemr/helpline104/controller/beneficiarycall/BeneficiaryCallController.java (1)
90-100: Improve parameter naming and validation
Rename the method parameterString beneficiaryCalltorequestBodyor similar to avoid confusion with theBeneficiaryCallentity. Also consider validating the JSON payload before mapping to guard against malformed input.src/main/java/com/iemr/helpline104/utils/FilterConfig.java (1)
12-26: Duplicate origin parsing logic β consider single source of truthBoth
CorsConfigandJwtUserIdValidationFilterparse the samecors.allowed-originsstring. Divergence over time is very easy (e.g., someone changes the parsing in one place only). Extract a smallCorsProperties@ConfigurationProperties bean and inject it here and intoCorsConfig& the filter.
This keeps behaviour consistent and avoids βworks in dev, breaks in prodβ surprises.src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java (1)
56-60: Consider returning strong types instead of rawStringThe controller converts the POJO list to
toString()and returns it as the body. This produces a non-JSON response and loses type information. Expose the list directly (Spring will serialise to JSON) or wrap it in anOutputResponseDTO and return aResponseEntity.Not blocking the CORS refactor, but improving this now avoids downstream parsing hacks.
src/main/java/com/iemr/helpline104/controller/secondaryCrmReports/SecondaryCRMReports.java (1)
31-35: Unused imports can be removedAfter dropping
@CrossOrigin, theorg.springframework.http.MediaTypeimport at line 30 is never referenced in this file. Trimming it keeps the file tidy and avoids IDE warnings.src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java (2)
6-9: Remove leftover@Componentimport
org.springframework.stereotype.Componentis no longer used now that the annotation was dropped. Keeping it will trigger an βunused importβ compilation warning.-import org.springframework.stereotype.Component;
143-151:isOriginAllowedwildcard conversion is fragileChaining
String.replace()mangles the pattern before the special βlocalhost with any portβ replacement executes, so
http://localhost:*becomeshttp://localhost\:*and the final replace never matches. Consider pre-compiling aPatternor at least changing the order:- return Arrays.stream(allowedOrigins.split(",")) - .map(String::trim) - .anyMatch(pattern -> { - String regex = pattern - .replace(".", "\\.") - .replace("*", ".*") - .replace("http://localhost:.*", "http://localhost:\\d+"); - return origin.matches(regex); - }); + return Arrays.stream(allowedOrigins.split(",")) + .map(String::trim) + .map(this::convertToRegex) + .anyMatch(origin::matches); ... + private String convertToRegex(String pattern) { + if ("*".equals(pattern)) { + return ".*"; + } + + if (pattern.startsWith("http://localhost:") && pattern.endsWith("*")) { + // wildcard port + return pattern.replace("*", "\\\\d+"); + } + return pattern.replace(".", "\\\\.").replace("*", ".*"); + }
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (35)
src/main/environment/104_ci.properties(1 hunks)src/main/environment/104_example.properties(1 hunks)src/main/java/com/iemr/helpline104/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/helpline104/controller/IMRMMR/IMRMMRController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/balVivha/BalVivahController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/beneficiarycall/BeneficiaryCallController.java(1 hunks)src/main/java/com/iemr/helpline104/controller/bloodComponent/BloodComponentController.java(2 hunks)src/main/java/com/iemr/helpline104/controller/bloodRequest/BloodRequestController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/callqamapping/CallQAMappingController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/casesheet/Helpline104BeneficiaryHistoryController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/cdss/ClinicalDecisionSupportController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/covidMaster/CovidMasterController.java(2 hunks)src/main/java/com/iemr/helpline104/controller/directory/DirectoryServicesController.java(1 hunks)src/main/java/com/iemr/helpline104/controller/disease/DiseaseController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/diseaseScreening/DiseaseScreeningController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java(6 hunks)src/main/java/com/iemr/helpline104/controller/epidemicOutbreak/EpidemicOutbreakController.java(1 hunks)src/main/java/com/iemr/helpline104/controller/feedback/FeedbackController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/feedbackType/FeedbackTypeController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/foodSafetyComplaint/FoodSafetyComplaintController.java(1 hunks)src/main/java/com/iemr/helpline104/controller/healthCareWorkerType/HealthCareWorkerTypeController.java(1 hunks)src/main/java/com/iemr/helpline104/controller/hihl/HIHLController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/location/CountryCityController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/location/LocationController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/organDonation/OrganDonationController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/prescription/PrescriptionController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/scheme/SchemeController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/secondaryCrmReports/SecondaryCRMReports.java(16 hunks)src/main/java/com/iemr/helpline104/controller/sioHistory/ServiceImprovementOfficerController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/snomedct/SnomedController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/users/IEMRAdminController.java(0 hunks)src/main/java/com/iemr/helpline104/controller/version/VersionController.java(1 hunks)src/main/java/com/iemr/helpline104/service/CTI/CTIService.java(1 hunks)src/main/java/com/iemr/helpline104/utils/FilterConfig.java(1 hunks)src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java(5 hunks)
π€ Files with no reviewable changes (19)
- src/main/java/com/iemr/helpline104/controller/hihl/HIHLController.java
- src/main/java/com/iemr/helpline104/controller/location/CountryCityController.java
- src/main/java/com/iemr/helpline104/controller/disease/DiseaseController.java
- src/main/java/com/iemr/helpline104/controller/feedback/FeedbackController.java
- src/main/java/com/iemr/helpline104/controller/cdss/ClinicalDecisionSupportController.java
- src/main/java/com/iemr/helpline104/controller/diseaseScreening/DiseaseScreeningController.java
- src/main/java/com/iemr/helpline104/controller/balVivha/BalVivahController.java
- src/main/java/com/iemr/helpline104/controller/callqamapping/CallQAMappingController.java
- src/main/java/com/iemr/helpline104/controller/casesheet/Helpline104BeneficiaryHistoryController.java
- src/main/java/com/iemr/helpline104/controller/feedbackType/FeedbackTypeController.java
- src/main/java/com/iemr/helpline104/controller/organDonation/OrganDonationController.java
- src/main/java/com/iemr/helpline104/controller/scheme/SchemeController.java
- src/main/java/com/iemr/helpline104/controller/IMRMMR/IMRMMRController.java
- src/main/java/com/iemr/helpline104/controller/snomedct/SnomedController.java
- src/main/java/com/iemr/helpline104/controller/location/LocationController.java
- src/main/java/com/iemr/helpline104/controller/sioHistory/ServiceImprovementOfficerController.java
- src/main/java/com/iemr/helpline104/controller/bloodRequest/BloodRequestController.java
- src/main/java/com/iemr/helpline104/controller/users/IEMRAdminController.java
- src/main/java/com/iemr/helpline104/controller/prescription/PrescriptionController.java
π§° Additional context used
𧬠Code Graph Analysis (2)
src/main/java/com/iemr/helpline104/config/CorsConfig.java (1)
src/main/java/com/iemr/helpline104/utils/FilterConfig.java (1)
Configuration(9-28)
src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java (1)
src/main/java/com/iemr/helpline104/utils/UserAgentContext.java (1)
UserAgentContext(3-18)
πͺ ast-grep (0.38.1)
src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java
[warning] 180-180: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
β° Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
π Additional comments (10)
src/main/java/com/iemr/helpline104/controller/version/VersionController.java (1)
31-31: Approve removal of@CrossOriginimport
Centralizing CORS viaCorsConfigreplaces per-controller annotations; removing the unusedCrossOriginimport aligns this controller with the new global policy.src/main/environment/104_ci.properties (1)
30-30: Approve addition ofcors.allowed-origins
This new property feeds intoCorsConfigfor global CORS management. Ensure the CI environment variableCORS_ALLOWED_ORIGINSis defined in your deployment pipeline.src/main/environment/104_example.properties (2)
25-25: Unrelated change detected
The update tologging.file.nameisnβt mentioned in the PR summary. Please confirm whether this change should be scoped into this CORS-focused pull request.Likely an incorrect or invalid review comment.
27-27: Approve addition ofcors.allowed-origins
Settingcors.allowed-origins=http://localhost:*in the example properties aligns with the centralized CORS configuration approach.src/main/java/com/iemr/helpline104/controller/bloodComponent/BloodComponentController.java (2)
53-53: Approve formatting refinement
Removed extra spaces in the@Parameterannotation description forsaveBloodComponentDetails, improving consistency with JSON schema representations.
72-72: Approve formatting refinement
Cleaned up spacing in the@Parameterannotation forgetBloodComponentDetails, aligning with code style standards.src/main/java/com/iemr/helpline104/controller/healthCareWorkerType/HealthCareWorkerTypeController.java (2)
30-30: Remove redundant CrossOrigin import
The@CrossOriginimport was removed to delegate CORS handling to the global configuration. Ensure yourCorsConfigcovers this controller's/beneficiarypath.
52-53: Removed method-level CORS annotation
The@CrossOriginannotation was deleted here; confirm that your centralized CORS settings allow requests to/beneficiary/get/healthCareWorkerTypes. Consider adding an integration test to verify CORS preflight on this endpoint.src/main/java/com/iemr/helpline104/controller/directory/DirectoryServicesController.java (1)
30-30: Removed CrossOrigin import
The standalone@CrossOriginimport was removed; ensure your globalCorsConfigcovers all/beneficiaryendpoints.src/main/java/com/iemr/helpline104/controller/beneficiarycall/BeneficiaryCallController.java (1)
92-99: Null-check logic is sound
Good addition of thenullguard forbeneficiaryRegID. It prevents NPEs and ensures updates are only attempted when an ID is provided.



π 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
Bug Fixes