CORS Configuration for TM API Services #89
Conversation
WalkthroughThis change removes all Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Filter (JwtUserIdValidationFilter)
participant Controller
Client->>Filter: HTTP Request (with Origin header)
alt OPTIONS request
Filter-->>Client: 200 OK with CORS headers (if origin allowed)
else Non-OPTIONS request
Filter->>Filter: Validate Origin against allowedOrigins
alt Origin allowed
Filter->>Client: Add CORS headers
Filter->>Controller: Forward request
Controller-->>Filter: Response
Filter-->>Client: Response with CORS headers
else Origin not allowed
Filter->>Client: No CORS headers, log warning
Filter->>Controller: Forward request
Controller-->>Filter: Response
Filter-->>Client: Response
end
end
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 (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (18)
src/main/environment/common_ci.properties (1)
79-80: Introducecors.allowed-originsconfiguration
Adds a dynamic list of allowed CORS origins. Ensure the filter handles missing or invalid values gracefully, and document the expected format (e.g., comma-separated list) in your README or config reference.src/main/environment/common_example.properties (1)
77-78: Clarify wildcard usage for example CORS origins
The patternhttp://localhost:*may not be recognized by all CORS matchers. Consider showing explicit host:port pairs or documenting supported wildcard semantics to avoid confusion.src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java (1)
28-30: Cross-origin import successfully removed – please verify global cleanupThe
@CrossOriginimport disappeared here, which aligns with the new filter-based CORS approach.
To avoid stray annotations/imports elsewhere (which would compile but be ignored, causing silent config drift), a quick repo-wide check is worthwhile.#!/bin/bash # Find any remaining @CrossOrigin usages or imports rg --line-number "@CrossOrigin" src/main/java || echo "✅ No @CrossOrigin annotations found" rg --line-number "import .*CrossOrigin" src/main/java || echo "✅ No CrossOrigin imports found"src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java (2)
96-100: Path overlap may cause ambiguous mapping
/call/{fromuserID}/{touserID}and/call/{fromuserID}/{touserID}/{type}are bothGETendpoints.
Calling/call/1/2matches the first mapping, but Spring will still consider the second pattern as a potential candidate (it differs only by an optional segment); under some Spring versions this logs an “Ambiguous mapping” warning.Consider making the second mapping explicit, e.g.
-@GetMapping(value = "/call/{fromuserID}/{touserID}/{type}", …) +@GetMapping(value = "/callWithType/{fromuserID}/{touserID}/{type}", …)or merge the logic into one method with
@RequestParam("type")if backwards-compatible.
151-154: Same potential ambiguity for/callvan/**endpointsThe two
callvanmappings have the same pattern issue described above.
Refactor as suggested to avoid route-resolution warnings.src/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.java (1)
59-63: Delete commented-out autowiring blockDead, commented code makes the file noisy and risks diverging from the real implementation. Safe to remove entirely.
- // @Autowired - // public void setGeneralOPDServiceImpl(GeneralOPDServiceImpl - // generalOPDServiceImpl) { - // this.generalOPDService = generalOPDService; - // }src/main/java/com/iemr/tm/controller/anc/AntenatalCareController.java (2)
124-128: Capture exception stacktrace for easier debuggingOnly the message is logged; consider adding the throwable to keep the stacktrace:
- logger.error("Error while saving doctor data:" + e.getMessage()); + logger.error("Error while saving doctor data", e);
450-454: Same logging issue as aboveReplicate the improved logging here too.
- logger.error("Unable to modify data. " + e.getMessage()); + logger.error("Unable to modify data", e);src/main/java/com/iemr/tm/controller/foetalmonitor/FoetalMonitorController.java (1)
166-175: Consider removing the gigantic JSON string from the@ParamannotationThe long in-line JSON sample clutters the annotation, adds CR/LF escape sequences, and hurts readability. A cleaner approach is to drop the
@Paramcompletely (Swagger/OpenAPI will already pick up the@RequestBody) or move the sample into a dedicated DTO / schema example.- @Param("\r\n" + "{ ... very long sample ... }") @RequestBody String requestObj, + @RequestBody String requestObj,Optional but will make the source far easier to scan.
src/main/java/com/iemr/tm/controller/common/main/WorklistController.java (1)
99-116: Minor: null-checks on mandatory@PathVariableintegers are redundant
providerServiceMapID,serviceID, andvanIDare primitives in the URL – Spring will fail to bind if they are absent, so the subsequentnullchecks (lines 107-115) are never hit.
Removing them would simplify the method.src/main/java/com/iemr/tm/controller/nurse/vitals/AnthropometryVitalsController.java (2)
31-33:@Paramis the wrong annotation for a REST payload
org.springframework.data.repository.query.Paramis meant for Spring-Data repositories, not for MVC controllers. It adds no value here and is confusing.
Use only@RequestBody, or if you need to document the schema for OpenAPI use@io.swagger.v3.oas.annotations.media.Schemaon the DTO instead.- public String getBenHeightDetailsFrmNurse( - @Param(value = "{\"benRegID\":\"Long\"}") @RequestBody String comingRequest) { + public String getBenHeightDetailsFrmNurse(@RequestBody String comingRequest) {
35-36: Avoid logging raw request bodies
benRegIDis personally-identifiable information. Dumping the entire JSON into application logs can violate audit / privacy requirements and inflate log size.
Log only a correlation ID or the parsedbenRegID, not the full payload.src/main/java/com/iemr/tm/utils/FilterConfig.java (2)
12-14: Fail fast whencors.allowed-originsis missingIf the property is absent or empty the filter silently blocks every origin, which is hard to diagnose.
Consider validating the property at startup and throwing anIllegalStateException, or supply a sensible default (e.g.""meaning “no CORS allowed”).
20-24:allowedOriginsshould be parsed once, not per request
JwtUserIdValidationFiltercurrently splits & builds a regex for every request. Move the parsing into the configuration class when you create the filter and pass a pre-compiledList<Pattern>(or even aPredicate<String>). This avoids garbage on every call and simplifies the filter.src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java (3)
5-9: Remove unused@ComponentimportThe annotation was deleted but the import lingers, producing an IDE/compile warning.
-import org.springframework.stereotype.Component;
39-53: Header name mismatch may confuse clientsThe pre-flight response whitelists
Jwttoken, whereas the runtime code expectsJwtToken(capital “T”).
Although header names are case-insensitive, spelling differences trip developers. Add both variants to be explicit.- response.setHeader("Access-Control-Allow-Headers", "Authorization, Content-Type, Accept, Jwttoken"); + response.setHeader("Access-Control-Allow-Headers", + "Authorization, Content-Type, Accept, Jwttoken, JwtToken");
133-150:isOriginAllowedbuilds regexes on every requestSplitting, replacing, and
.matchesfor every call is avoidable.
Cache the compiledPatterns once (e.g. in the constructor) and just iterate over them in O(allowedOrigins) instead of O(allowedOrigins × regex build).src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java (1)
28-28: Transactional annotation belongs in the service layer, not the controller.
Managing transactions in the controller breaks separation of concerns and couples web layer to persistence. Move@Transactionaland its import to the service implementation where business logic resides.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/tm/controller/anc/AntenatalCareController.java(4 hunks)src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java(1 hunks)src/main/java/com/iemr/tm/controller/common/main/WorklistController.java(2 hunks)src/main/java/com/iemr/tm/controller/common/master/CommonMasterController.java(1 hunks)src/main/java/com/iemr/tm/controller/covid19/CovidController.java(1 hunks)src/main/java/com/iemr/tm/controller/dataSyncActivity/StartSyncActivity.java(1 hunks)src/main/java/com/iemr/tm/controller/dataSyncLayerCentral/MMUDataSyncVanToServer.java(1 hunks)src/main/java/com/iemr/tm/controller/foetalmonitor/FoetalMonitorController.java(8 hunks)src/main/java/com/iemr/tm/controller/generalOPD/GeneralOPDController.java(2 hunks)src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java(4 hunks)src/main/java/com/iemr/tm/controller/location/LocationController.java(2 hunks)src/main/java/com/iemr/tm/controller/login/IemrMmuLoginController.java(1 hunks)src/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.java(1 hunks)src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java(2 hunks)src/main/java/com/iemr/tm/controller/nurse/vitals/AnthropometryVitalsController.java(2 hunks)src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java(11 hunks)src/main/java/com/iemr/tm/controller/pnc/PostnatalCareController.java(1 hunks)src/main/java/com/iemr/tm/controller/quickBlox/QuickbloxController.java(2 hunks)src/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.java(1 hunks)src/main/java/com/iemr/tm/controller/registrar/main/RegistrarController.java(7 hunks)src/main/java/com/iemr/tm/controller/report/CRMReportController.java(1 hunks)src/main/java/com/iemr/tm/controller/snomedct/SnomedController.java(2 hunks)src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java(1 hunks)src/main/java/com/iemr/tm/controller/version/VersionController.java(2 hunks)src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java(3 hunks)src/main/java/com/iemr/tm/utils/FilterConfig.java(1 hunks)src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
🔇 Additional comments (25)
src/main/java/com/iemr/tm/controller/ncdCare/NCDCareController.java (1)
31-32: Review import cleanup
This blank-line change reflects removal of controller-level CORS annotations and their imports. Please verify that no@CrossOriginimports remain and clean up any unused imports.src/main/java/com/iemr/tm/controller/covid19/CovidController.java (1)
31-32: Review import cleanup
Confirm that all@CrossOriginimports have been removed following the shift to centralized CORS, and remove any leftover unused imports.src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java (1)
29-29: Review import cleanup
Ensure no residual CORS-related imports or blank lines remain after migrating CORS handling into the filter layer.src/main/java/com/iemr/tm/controller/common/master/CommonMasterController.java (1)
29-32: Same import removal here – double-check that every module compilesNothing functionally wrong, but do a full build after the annotation purge; IDEs sometimes leave unused import statements that fail CI. The verification script in the previous comment will catch leftovers.
src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java (1)
27-32: CORS import gone – good. Build still includesRequestMethod& friendsNo action needed; import list is minimal and compiles.
src/main/java/com/iemr/tm/controller/registrar/main/RegistrarController.java (1)
301-305: Minor message tweak looks fineOnly the error string changed; no behavioural impact.
src/main/java/com/iemr/tm/controller/dataSyncActivity/StartSyncActivity.java (1)
28-36: Import housekeeping acknowledgedCORS import removed here as well; ensure the full build passes.
src/main/java/com/iemr/tm/controller/quickconsult/QuickConsultController.java (1)
29-29: Removed CrossOrigin import: centralize CORS handling
The@CrossOriginimport has been removed to delegate all CORS logic to theJwtUserIdValidationFilter. Ensure this filter is registered with highest precedence and correctly applies CORS headers (including preflight OPTIONS) for all QuickConsult endpoints.src/main/java/com/iemr/tm/controller/labtechnician/LabtechnicianController.java (5)
27-27: Removed CrossOrigin import
TheCrossOriginannotation import was removed. CORS is now managed globally via the centralized filter.
50-50: Removed class-level @crossorigin
Class-level CORS annotation removed. Confirm thatJwtUserIdValidationFiltercovers requests before controller logic.
52-52: Removed @crossorigin on save endpoint
The explicit CORS annotation onsaveLabTestResulthas been dropped. Verify dynamic origin checks in the filter handle this path.
94-94: Removed @crossorigin on prescribed procedures endpoint
CORS annotation removed from/get/prescribedProceduresList. Ensure preflight and actual requests are correctly handled by the filter.
151-151: Removed @crossorigin on lab result endpoint
Dropped the@CrossOriginfrom/get/labResultForVisitcode. Double-check that allowed origins configuration covers this route.src/main/java/com/iemr/tm/controller/dataSyncLayerCentral/MMUDataSyncVanToServer.java (1)
27-27: Removed CrossOrigin import
Eliminated controller-level CORS import to centralize handling in the filter. Verify that config propertycors.allowed-originsis injected intoFilterConfigand applied here.src/main/java/com/iemr/tm/controller/pnc/PostnatalCareController.java (1)
29-29: Removed CrossOrigin import
TheCrossOriginimport was removed in favor of global filter-based CORS. Ensure that all PNC endpoints are now covered byJwtUserIdValidationFilter.src/main/java/com/iemr/tm/controller/location/LocationController.java (2)
28-28: Removed CrossOrigin import
Dropped the CORS annotation import. Centralized filter must now handle all location endpoints.
43-43: Removed class-level @crossorigin
Removed the explicit@CrossOriginon the controller. Confirm the filter logic correctly applies CORS headers for GET and POST routes here.src/main/java/com/iemr/tm/controller/snomedct/SnomedController.java (1)
24-30: ```shell
#!/bin/bashShow where Origin is read and CORS headers are set
echo "=== getHeader usage ==="
grep -F -n 'getHeader' src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java || trueecho
echo "=== File snippet 1–80 ==="
sed -n '1,80p' src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java</details> <details> <summary>src/main/java/com/iemr/tm/controller/quickBlox/QuickbloxController.java (1)</summary> `52-55`: **Pre-flight may bypass this handler because of `headers="Authorization"`** All endpoint mappings under `/quickblox` still demand an `Authorization` header. Browsers do NOT include that header in the CORS pre-flight `OPTIONS` request, so Spring will not match the mapping and will fall back to `404` unless the filter short-circuits the request earlier. Verify with an actual `OPTIONS` call from the front-end after this change. </details> <details> <summary>src/main/java/com/iemr/tm/controller/version/VersionController.java (1)</summary> `54-57`: **No issues with CORS removal here** `/version` never required cross-site credentials, so moving the CORS logic to the global filter is risk-free. </details> <details> <summary>src/main/java/com/iemr/tm/controller/foetalmonitor/FoetalMonitorController.java (1)</summary> `77-78`: **Looks good – just a split line, no functional impact** No concerns with the wrapped call; readability is preserved. </details> <details> <summary>src/main/java/com/iemr/tm/controller/patientApp/master/PatientAppCommonMasterController.java (1)</summary> `62-68`: **LGTM – annotation wording change only** The updated summary string is clear and does not affect behaviour. </details> <details> <summary>src/main/java/com/iemr/tm/controller/report/CRMReportController.java (1)</summary> `30-35`: **Imports adjusted correctly after removing `@CrossOrigin`** Compilation should succeed; no unused imports introduced. </details> <details> <summary>src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java (1)</summary> `28-33`: **Import block update aligns with centralised CORS refactor** Nothing else changed – looks fine. </details> <details> <summary>src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java (1)</summary> `54-58`: **Pre-flight path returns 200 even for disallowed origins** When `origin` is not allowed you skip CORS headers but still return `200 OK` for `OPTIONS`. Browsers will still block the actual call, but you spend CPU validating JWTs later. Consider short-circuiting with `SC_FORBIDDEN` (403) when the origin is not on the allow-list to surface mis-configurations early. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java
Show resolved
Hide resolved
📋 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
Chores