Conversation
WalkthroughThis change centralizes CORS (Cross-Origin Resource Sharing) configuration by introducing a global CORS configuration class and removing all Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant SpringApp
participant CorsConfig
participant JwtFilter
participant Controller
Browser->>SpringApp: OPTIONS /api/endpoint (preflight)
SpringApp->>CorsConfig: Apply global CORS config
CorsConfig-->>SpringApp: Set CORS headers
SpringApp->>JwtFilter: Pass request
JwtFilter-->>SpringApp: Allow OPTIONS, skip auth
SpringApp-->>Browser: Respond with CORS headers
Browser->>SpringApp: Actual request (GET/POST/...)
SpringApp->>CorsConfig: Apply global CORS config
CorsConfig-->>SpringApp: Set CORS headers
SpringApp->>JwtFilter: Pass request
JwtFilter->>SpringApp: Validate JWT
SpringApp->>Controller: Handle request
Controller-->>SpringApp: Return response
SpringApp-->>Browser: Respond with data + CORS headers
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ 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 (
|
![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
🧹 Nitpick comments (2)
src/main/environment/common_example.properties (1)
205-206: Wildcard port pattern may not match as intended
http://localhost:*is accepted by Spring’sallowedOriginPatterns, but many developers expect “any port” to be expressed ashttp://localhost:[*]. Consider clarifying in a comment or documentation to avoid confusion.src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (1)
95-95: Minor log typo
"Common-API incoming userAget : "→"Common-API incoming userAgent : "
Small, but fixing avoids noisy grep searches.-logger.info("Common-API incoming userAget : " + userAgent); +logger.info("Common-API incoming userAgent : " + userAgent);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/common/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java(0 hunks)src/main/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationController.java(0 hunks)src/main/java/com/iemr/common/controller/brd/BRDIntegrationController.java(0 hunks)src/main/java/com/iemr/common/controller/callhandling/CallController.java(1 hunks)src/main/java/com/iemr/common/controller/carestream/CareStreamCreateOrderController.java(0 hunks)src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java(0 hunks)src/main/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationController.java(0 hunks)src/main/java/com/iemr/common/controller/customization/CustomizationController.java(0 hunks)src/main/java/com/iemr/common/controller/directory/DirectoryController.java(0 hunks)src/main/java/com/iemr/common/controller/door_to_door_app/DoorToDoorAppController.java(0 hunks)src/main/java/com/iemr/common/controller/eausadha/EAusadhaController.java(0 hunks)src/main/java/com/iemr/common/controller/email/EmailController.java(0 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(0 hunks)src/main/java/com/iemr/common/controller/helpline104history/Helpline104BeneficiaryHistoryController.java(0 hunks)src/main/java/com/iemr/common/controller/honeywell/HoneywellController.java(0 hunks)src/main/java/com/iemr/common/controller/institute/InstituteController.java(0 hunks)src/main/java/com/iemr/common/controller/kmfilemanager/KMFileManagerController.java(0 hunks)src/main/java/com/iemr/common/controller/language/LanguageController.java(0 hunks)src/main/java/com/iemr/common/controller/location/LocationController.java(0 hunks)src/main/java/com/iemr/common/controller/lonic/LonicController.java(0 hunks)src/main/java/com/iemr/common/controller/lungassessment/LungAssessmentController.java(0 hunks)src/main/java/com/iemr/common/controller/mctshistory/OutboundHistoryController.java(0 hunks)src/main/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardController.java(0 hunks)src/main/java/com/iemr/common/controller/notification/NotificationController.java(0 hunks)src/main/java/com/iemr/common/controller/otp/OTPGateway.java(0 hunks)src/main/java/com/iemr/common/controller/questionconfig/QuestionTypeController.java(0 hunks)src/main/java/com/iemr/common/controller/questionconfig/QuestionnaireController.java(0 hunks)src/main/java/com/iemr/common/controller/report/CustomerRelationshipReports.java(0 hunks)src/main/java/com/iemr/common/controller/scheme/SchemeController.java(0 hunks)src/main/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReports.java(0 hunks)src/main/java/com/iemr/common/controller/services/CategoryController.java(0 hunks)src/main/java/com/iemr/common/controller/services/CommonController.java(0 hunks)src/main/java/com/iemr/common/controller/sms/SMSController.java(0 hunks)src/main/java/com/iemr/common/controller/snomedct/SnomedController.java(0 hunks)src/main/java/com/iemr/common/controller/uptsu/UPTechnicalSupportController.java(0 hunks)src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java(0 hunks)src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(0 hunks)src/main/java/com/iemr/common/controller/version/VersionController.java(0 hunks)src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java(2 hunks)
💤 Files with no reviewable changes (40)
- src/main/java/com/iemr/common/controller/questionconfig/QuestionTypeController.java
- src/main/java/com/iemr/common/controller/mctshistory/OutboundHistoryController.java
- src/main/java/com/iemr/common/controller/services/CategoryController.java
- src/main/java/com/iemr/common/controller/language/LanguageController.java
- src/main/java/com/iemr/common/controller/questionconfig/QuestionnaireController.java
- src/main/java/com/iemr/common/controller/helpline104history/Helpline104BeneficiaryHistoryController.java
- src/main/java/com/iemr/common/controller/snomedct/SnomedController.java
- src/main/java/com/iemr/common/controller/uptsu/UPTechnicalSupportController.java
- src/main/java/com/iemr/common/controller/lonic/LonicController.java
- src/main/java/com/iemr/common/controller/kmfilemanager/KMFileManagerController.java
- src/main/java/com/iemr/common/controller/eausadha/EAusadhaController.java
- src/main/java/com/iemr/common/controller/lungassessment/LungAssessmentController.java
- src/main/java/com/iemr/common/controller/location/LocationController.java
- src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java
- src/main/java/com/iemr/common/controller/esanjeevani/ESanjeevaniController.java
- src/main/java/com/iemr/common/controller/secondaryReport/CustomerRelationshipSecondaryReports.java
- src/main/java/com/iemr/common/controller/email/EmailController.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/door_to_door_app/DoorToDoorAppController.java
- src/main/java/com/iemr/common/controller/feedback/FeedbackController.java
- src/main/java/com/iemr/common/controller/brd/BRDIntegrationController.java
- src/main/java/com/iemr/common/controller/scheme/SchemeController.java
- src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
- src/main/java/com/iemr/common/controller/nhmdashboard/NationalHealthMissionDashboardController.java
- src/main/java/com/iemr/common/controller/everwellTest/EverwellController.java
- src/main/java/com/iemr/common/controller/report/CustomerRelationshipReports.java
- src/main/java/com/iemr/common/controller/directory/DirectoryController.java
- src/main/java/com/iemr/common/controller/users/EmployeeSignatureController.java
- src/main/java/com/iemr/common/controller/carestream/CareStreamCreateOrderController.java
- src/main/java/com/iemr/common/controller/institute/InstituteController.java
- src/main/java/com/iemr/common/controller/cti/ComputerTelephonyIntegrationController.java
- src/main/java/com/iemr/common/controller/sms/SMSController.java
- src/main/java/com/iemr/common/controller/everwell/callhandle/EverwellCallController.java
- src/main/java/com/iemr/common/controller/otp/OTPGateway.java
- src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java
- src/main/java/com/iemr/common/controller/honeywell/HoneywellController.java
- src/main/java/com/iemr/common/controller/notification/NotificationController.java
- src/main/java/com/iemr/common/controller/customization/CustomizationController.java
- src/main/java/com/iemr/common/controller/beneficiary/BeneficiaryRegistrationController.java
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Build
- GitHub Check: Analyze (java)
- GitHub Check: Package-test
🔇 Additional comments (2)
src/main/environment/common_ci.properties (1)
181-182: Property placeholder added – verify runtime substitutionThe placeholder
@CORS_ALLOWED_ORIGINS@relies on external substitution (CI/helm). Make sure the deployment pipeline always populates this value; an empty string will breakCorsConfig->.allowedOriginPatterns()and effectively block every origin.src/main/java/com/iemr/common/controller/callhandling/CallController.java (1)
501-504: Unchanged logic flagged as modified – no action neededThe line appears only re-formatted; functional behavior is identical.
|
Success: |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (3)
41-48: Expose full CORS header set & cache pre-flight response
Access-Control-Max-AgeandAccess-Control-Expose-Headersare missing.
Without them browsers will re-issue pre-flights on every request and some
non-simple headers won’t be available to JS.response.setHeader("Access-Control-Allow-Credentials", "true"); +response.setHeader("Access-Control-Expose-Headers", + "Authorization, Jwttoken, Location"); +response.setHeader("Access-Control-Max-Age", "3600");Optional but recommended for better UX and reduced latency.
109-109: Typo in log message –userAget-logger.info("Common-API incoming userAget : " + userAgent); +logger.info("Common-API incoming userAgent : " + userAgent);Minor, but helps when grepping logs.
127-136:isOriginAlloweddoes per-request regex compilation – cache patterns onceSplitting and transforming the pattern list on every request is avoidable GC
pressure. Pre-compile in the constructor:-private final String allowedOrigins; +private final List<Pattern> allowedOriginPatterns; // constructor -this.allowedOrigins = allowedOrigins; +this.allowedOriginPatterns = Arrays.stream(allowedOrigins.split(",")) + .map(String::trim) + .map(p -> Pattern.compile( + p.replace(".", "\\.").replace("*", ".*"))) + .collect(Collectors.toList());Then
isOriginAllowedsimply iterates over the compiled list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/iemr/common/utils/FilterConfig.java(1 hunks)src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
- GitHub Check: Build
🔇 Additional comments (1)
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (1)
49-53: OPTIONS branch should still invokefilterChainReturning immediately prevents later filters (e.g., logging, metrics) and Spring
MVC from writing additional headers. Safer pattern:- response.setStatus(HttpServletResponse.SC_OK); - return; + response.setStatus(HttpServletResponse.SC_OK); + filterChain.doFilter(servletRequest, servletResponse); + return;Keeps the fast-exit behaviour while preserving the normal filter pipeline.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/com/iemr/common/utils/FilterConfig.java (2)
12-14: Property still crashes the app if missing – previously flagged
Lack of a default value means the application will not start ifcors.allowed-originsis absent in an environment. This was pointed out in an earlier review; the risk remains unchanged.
15-25:⚠️ Potential issueFilter now registered twice & runs before built-in
CorsFilter
JwtUserIdValidationFilteris still annotated with@Componentand manually registered here, so two instances are created.
By moving the registration bean toOrdered.HIGHEST_PRECEDENCE, it will now execute before Spring Boot’s auto-configuredCorsFilter(which uses the same order). The final ordering between the two highest-precedence filters is undefined, which can:• cause the JWT filter to run twice
• lead to CORS headers being written twice or overridden
• make OPTIONS requests bypass the built-in CORS handling altogetherConsider either
-@Component // inside JwtUserIdValidationFilter +// remove @Component – managed via FilterRegistrationBean only ... -registrationBean.setOrder(Ordered.HIGHEST_PRECEDENCE); +registrationBean.setOrder(Ordered.HIGHEST_PRECEDENCE + 1); // run immediately after CorsFilteror drop the explicit registration and rely on component scanning to avoid duplication.
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/utils/FilterConfig.java (1)
20-22: Pass a parsed collection instead of a raw comma-separated string
allowedOriginsis injected as a singleString; the filter must parse it every request. Parsing once at config time and passing aSet<String>(orList<String>) would avoid repeated split operations and make intent clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/iemr/common/utils/FilterConfig.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
- GitHub Check: Build
|
3 participants
📋 Description
JIRA ID: AMM-1427
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