CORS Configuration for ECD API Services #100
Conversation
|
Warning Rate limit exceeded@vishwab1 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis change centralizes and enhances CORS (Cross-Origin Resource Sharing) configuration. It removes all Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Filter as JwtUserIdValidationFilter
participant WebConfig as WebConfiguration
participant Controller
Client->>Filter: HTTP Request (with Origin header)
Filter->>Filter: Check if Origin is allowed
alt Origin allowed
Filter->>Client: Set CORS headers in response
else Origin not allowed
Filter->>Client: No CORS headers, log warning
end
alt OPTIONS request
Filter->>Client: Respond 200 OK (preflight)
else
Filter->>Controller: Forward request for processing
Controller-->>Filter: Response
Filter->>Client: Forward response (with CORS headers if allowed)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/main/java/com/iemr/ecd/repo/call_conf_allocation/OutboundCallsRepo.java (1)
124-131: Parameter order in count query differs from select query
getMotherUnAllocatedCountHRno longer takesfDate/tDatewhile the count-LR versions do. Paging components that rely on counts vs. list queries may display mismatched totals. Align the signatures or document the difference explicitly.
♻️ Duplicate comments (4)
src/main/java/com/iemr/ecd/controller/quality/ChartsController.java (1)
42-48: Same CORS centralisation caveat as previous controllerThe observations & script in the previous comment apply to this controller as well.
No further issues spotted.src/main/java/com/iemr/ecd/controller/quality/AgentQualityAuditorMappingController.java (1)
45-51: Same CORS centralisation caveat as previous controllerSee the first review comment for checks to perform.
src/main/java/com/iemr/ecd/controller/associate/AutoPreviewDialingController.java (1)
45-51: Same CORS centralisation caveat as previous controllerSee the first review comment for checks to perform.
src/main/java/com/iemr/ecd/controller/callallocation/CallAllocationController.java (1)
49-55: Same CORS centralisation caveat as previous controllerSee the first review comment for checks to perform.
🧹 Nitpick comments (10)
src/main/java/com/iemr/ecd/controller/callallocation/CallAllocationController.java (1)
156-160: Readability nit – long parameter listThe wrapped call is easier to read now, but seven positional String parameters are still error-prone. Consider introducing a small DTO for this request to improve maintainability.
src/main/java/com/iemr/ecd/controller/quality/QualityAuditController.java (1)
91-93: Prefer usingResponseEntity.ok(…)for brevityThe explicit constructor call is perfectly valid, but the more idiomatic Spring style is the static helper:
- return new ResponseEntity<>( - qualityAuditImpl.getQualityAuditorWorklistDatewise(qualityAuditorWorklistDatewiseRequestDTO), - HttpStatus.OK); + return ResponseEntity.ok( + qualityAuditImpl.getQualityAuditorWorklistDatewise(qualityAuditorWorklistDatewiseRequestDTO));Same payload, cleaner code.
src/main/java/com/iemr/ecd/controller/outboundworklist/OutBoundWorklistController.java (1)
69-71: Minor stylistic tweak – replace verbose constructor with helper- return new ResponseEntity<>(resp, HttpStatus.OK); + return ResponseEntity.ok(resp);No behavioural change, just terser.
src/main/java/com/iemr/ecd/controller/masters/MastersController.java (1)
238-244:authorizationheader collected but never usedThe method demands an
Authorizationheader yet immediately discards it, which:
- Creates dead-code noise.
- Forces callers to send a header that serves no purpose.
- Risks forgetting to forward the token to the service layer if that becomes necessary later.
Either drop the parameter or forward it:
-public ResponseEntity<List<AgentsViewMaster>> getAgentsByRoleIdAndLanguage( - @RequestHeader(value = "Authorization") String authorization, @PathVariable Integer roleId, - @PathVariable String preferredLanguage) { - return new ResponseEntity<>(masterServiceImpl.getAgentByRoleIdAndLanguage(roleId, preferredLanguage), - HttpStatus.OK); +public ResponseEntity<List<AgentsViewMaster>> getAgentsByRoleIdAndLanguage( + @PathVariable Integer roleId, + @PathVariable String preferredLanguage) { + return ResponseEntity.ok( + masterServiceImpl.getAgentByRoleIdAndLanguage(roleId, preferredLanguage)); }If the service really needs the token, pass it explicitly; otherwise the parameter and its annotation can be safely deleted.
src/main/java/com/iemr/ecd/config/WebConfiguration.java (2)
37-55: Guard against empty / wildcardcors.allowed-originsIf
cors.allowed-originsis empty or contains only a wildcard,originPatternsbecomes[""], effectively allowing no origins. This is a silent foot-gun on fresh deployments.Add a fallback:
String[] originPatterns = Arrays.stream(allowedOrigins.split(",")) .map(String::trim) .filter(s -> !s.isEmpty()) .toArray(String[]::new); if (originPatterns.length == 0) { originPatterns = new String[] {"*"}; // or log and fail fast }Also consider externalising allowed methods to config so ops can adjust without a code change.
46-54: Minor: useallowedOriginsoverallowedOriginPatternswhen wildcards are not required
allowedOriginPatternsdisables CORS pre-flight optimisation for exact matches and is only needed for wildcard domains. If you never expect wildcards, prefer.allowedOrigins(originPatterns)– it is stricter and marginally faster.src/main/java/com/iemr/ecd/repo/call_conf_allocation/OutboundCallsRepo.java (1)
59-65: NewCURRENT_TIMESTAMPpredicate may require an indexFiltering by
t.callDateTo >= CURRENT_TIMESTAMPwas added to many JPQL queries.
UnlesscallDateTois already indexed, every call list/count will trigger a full table scan – these methods are on hot paths (allocation dashboards). Verify there is a composite index starting withcallDateTo.Example migration (PostgreSQL):
CREATE INDEX idx_outbound_calls_callDateTo ON outbound_calls(call_date_to);src/main/java/com/iemr/ecd/controller/reports/ReportController.java (1)
33-34: Import list still contains unusedMediaTypeafter CORS refactor
MediaTypeis required, but ensure any now-unused imports (e.g.@CrossOrigin) were fully removed to keep warnings at zero. Otherwise, controller logic remains untouched.src/main/java/com/iemr/ecd/utils/mapper/JwtUserIdValidationFilter.java (2)
49-54: Pre-flight path: terminate the response cleanlyAfter setting status 200 on an
OPTIONSrequest you should also:
- Explicitly flush/commit the response to ensure headers are sent.
- Consider returning after writing CORS headers (already done) but before any body; adding a
returnimmediately afterflushBuffer()avoids accidental fall-through if future code is appended.if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { logger.info("OPTIONS request - skipping JWT validation"); response.setStatus(HttpServletResponse.SC_OK); + response.flushBuffer(); return; }
131-148:isOriginAllowedis re-evaluated per request – pre-compile patternsSplitting and building regexes on every call is O(N) string work per request and allocates many temporary objects.
Move the compilation to the constructor and reusePatterns:- private final String allowedOrigins; + private final List<Pattern> allowedOriginPatterns; public JwtUserIdValidationFilter(JwtAuthenticationUtil jwtAuthenticationUtil, - String allowedOrigins) { + String allowedOrigins) { this.jwtAuthenticationUtil = jwtAuthenticationUtil; - this.allowedOrigins = allowedOrigins; + this.allowedOriginPatterns = Arrays.stream( + Optional.ofNullable(allowedOrigins).orElse("") + .split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .map(JwtUserIdValidationFilter::toRegex) + .map(Pattern::compile) + .toList(); } ... -private boolean isOriginAllowed(String origin) { - if (origin == null || allowedOrigins == null || allowedOrigins.trim().isEmpty()) { +private boolean isOriginAllowed(String origin) { + if (origin == null || allowedOriginPatterns.isEmpty()) { logger.warn("No allowed origins configured or origin is null"); return false; } - return Arrays.stream(allowedOrigins.split(",")) - .map(String::trim) - .anyMatch(pattern -> origin.matches(wildcardToRegex(pattern))); + return allowedOriginPatterns.stream() + .anyMatch(p -> p.matcher(origin).matches()); }Besides performance, compiling once avoids subtle regex-escaping errors that can creep in during the ad-hoc
replacechain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
src/main/environment/ecd_ci.properties(1 hunks)src/main/environment/ecd_example.properties(1 hunks)src/main/java/com/iemr/ecd/config/WebConfiguration.java(1 hunks)src/main/java/com/iemr/ecd/controller/associate/AutoPreviewDialingController.java(1 hunks)src/main/java/com/iemr/ecd/controller/associate/BeneficiaryCallHistoryController.java(1 hunks)src/main/java/com/iemr/ecd/controller/associate/BeneficiaryRegistrationController.java(1 hunks)src/main/java/com/iemr/ecd/controller/associate/CallClosureController.java(1 hunks)src/main/java/com/iemr/ecd/controller/callallocation/CallAllocationController.java(4 hunks)src/main/java/com/iemr/ecd/controller/callallocation/CallConfigurationController.java(1 hunks)src/main/java/com/iemr/ecd/controller/dataupload/DataTemplateController.java(1 hunks)src/main/java/com/iemr/ecd/controller/dataupload/DataUploadController.java(1 hunks)src/main/java/com/iemr/ecd/controller/masters/MastersController.java(2 hunks)src/main/java/com/iemr/ecd/controller/outboundworklist/CallStatisticsController.java(1 hunks)src/main/java/com/iemr/ecd/controller/outboundworklist/OutBoundWorklistController.java(2 hunks)src/main/java/com/iemr/ecd/controller/quality/AgentQualityAuditorMappingController.java(1 hunks)src/main/java/com/iemr/ecd/controller/quality/ChartsController.java(1 hunks)src/main/java/com/iemr/ecd/controller/quality/GradeConfigurationController.java(1 hunks)src/main/java/com/iemr/ecd/controller/quality/QualityAuditController.java(4 hunks)src/main/java/com/iemr/ecd/controller/quality/QualityAuditQuestionConfigurationController.java(1 hunks)src/main/java/com/iemr/ecd/controller/quality/QualityAuditSectionConfigurationController.java(1 hunks)src/main/java/com/iemr/ecd/controller/quality/SampleSelectionConfigurationController.java(1 hunks)src/main/java/com/iemr/ecd/controller/questionare/EcdQuestionareController.java(6 hunks)src/main/java/com/iemr/ecd/controller/reports/ReportController.java(1 hunks)src/main/java/com/iemr/ecd/repo/call_conf_allocation/OutboundCallsRepo.java(2 hunks)src/main/java/com/iemr/ecd/utils/mapper/FilterConfig.java(1 hunks)src/main/java/com/iemr/ecd/utils/mapper/JwtUserIdValidationFilter.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/iemr/ecd/utils/mapper/FilterConfig.java (1)
src/main/java/com/iemr/ecd/config/WebConfiguration.java (1)
Configuration(32-56)
src/main/java/com/iemr/ecd/config/WebConfiguration.java (1)
src/main/java/com/iemr/ecd/utils/mapper/FilterConfig.java (1)
Configuration(9-28)
src/main/java/com/iemr/ecd/utils/mapper/JwtUserIdValidationFilter.java (1)
src/main/java/com/iemr/ecd/utils/constants/Constants.java (1)
Constants(5-24)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (15)
src/main/java/com/iemr/ecd/controller/callallocation/CallConfigurationController.java (1)
30-38: Import cleanup is correct – controller no longer depends on@CrossOrigin.
Compilation path stays green and supports the new global CORS strategy.src/main/java/com/iemr/ecd/controller/dataupload/DataUploadController.java (1)
28-33: Good removal of per-controller CORS annotation.
No functional side-effects detected; the controller now relies on the centralisedWebConfiguration.src/main/java/com/iemr/ecd/controller/quality/GradeConfigurationController.java (1)
30-37: Per-controller CORS annotation successfully dropped.
Nothing else changed in the controller logic – looks good.src/main/java/com/iemr/ecd/controller/outboundworklist/CallStatisticsController.java (1)
28-33: CORS annotation removal aligns with new global policy.
Implementation remains unaffected.src/main/environment/ecd_ci.properties (1)
32-33: Ensure the placeholder is resolved at deploy time.
If the@CORS_ALLOWED_ORIGINS@token is not replaced by the CI pipeline, Spring will read it literally, effectively blocking every origin. Please verify that:
- The build system filters this file and injects the real value, or
- A default value is supplied via
SPRING_APPLICATION_JSON/--cors.allowed-origins=at runtime.Otherwise all browsers will receive
CORS403 responses.src/main/java/com/iemr/ecd/controller/associate/BeneficiaryCallHistoryController.java (1)
28-28: Removed redundant@CrossOriginimport and annotation
The per-controller CORS annotation has been deleted to centralize CORS settings. Ensure your globalWebConfigurationcovers this endpoint so that CORS requests still succeed.src/main/java/com/iemr/ecd/controller/associate/CallClosureController.java (1)
28-28: Removed controller‐level CORS annotation
Good removal of theCrossOriginimport—CORS is now handled globally. Verify that/closure/closeCallis accessible from the intended origins via the new filter/config setup.src/main/java/com/iemr/ecd/controller/quality/QualityAuditSectionConfigurationController.java (1)
30-30: Centralized CORS: drop@CrossOrigin
TheCrossOriginimport was removed to rely on the global policy. Confirm that yourcors.allowed-originscovers/sectionConfigurationroutes in all deployment environments.src/main/java/com/iemr/ecd/controller/quality/QualityAuditQuestionConfigurationController.java (1)
30-30: Removed per-controller CORS configuration
Nice cleanup—controller is leaner without@CrossOrigin. Please validate that frontend apps can still call/questionnaireConfigurationunder the centralized CORS policy.src/main/java/com/iemr/ecd/controller/quality/SampleSelectionConfigurationController.java (1)
45-51: ```shell
#!/bin/bash1. Check for any remaining @crossorigin annotations
rg -n '@crossorigin' --glob '*.java'
2. Locate the WebConfiguration class
rg -l 'class WebConfiguration' -g '*.java'
3. Inspect its CORS setup (including allowedOrigins, allowedMethods, allowedHeaders, allowCredentials)
rg -n 'addCorsMappings' -g '*.java' -A10 -B2
</details> <details> <summary>src/main/java/com/iemr/ecd/controller/dataupload/DataTemplateController.java (1)</summary> `32-34`: **Import list clean-up acknowledged** The removal of `CrossOrigin` import aligns with the new centralised CORS configuration – good catch. </details> <details> <summary>src/main/java/com/iemr/ecd/controller/associate/BeneficiaryRegistrationController.java (1)</summary> `28-30`: **CORS annotation removed – verify global config allows `Authorization` header** Now that CORS is centralised, ensure the new `WebConfiguration`’s `allowedHeaders` contains `Authorization`; otherwise pre-flight requests to these endpoints will fail. </details> <details> <summary>src/main/java/com/iemr/ecd/utils/mapper/FilterConfig.java (1)</summary> `12-25`: ```shell #!/bin/bash set -e echo "Locating JwtUserIdValidationFilter class..." rg "class.*JwtUserIdValidationFilter" -n echo -e "\nIdentifying source file path..." FILTER_FILE=$(rg -l "class.*JwtUserIdValidationFilter" -n) echo -e "\nShowing first 200 lines of the filter implementation:" sed -n '1,200p' "$FILTER_FILE" echo -e "\nSearching for CORS header manipulation in the filter:" grep -R "Access-Control" -n "$(dirname "$FILTER_FILE")"src/main/java/com/iemr/ecd/repo/call_conf_allocation/OutboundCallsRepo.java (1)
92-97: Inconsistent date-range criteria for MO flow
getMotherRecordsForMOnow only appliescallDateTo >= CURRENT_TIMESTAMP, whereas the ANM/Associate equivalents still constrain by(:fDate between …) OR (:tDate between …).Was the omission intentional?
If not, the MO list will ignore the selected date window and may return records far outside the UI filter.src/main/java/com/iemr/ecd/controller/questionare/EcdQuestionareController.java (1)
30-38: Unused import cleanup acknowledgedThe
@CrossOriginimport removal is consistent with project-wide CORS centralisation; nothing else changed. 👍
|
📋 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