Cors configuration for admin mmu service #85
Conversation
WalkthroughThis update centralizes CORS (Cross-Origin Resource Sharing) configuration by removing all Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Filter (DynamicCorsFilter/JwtUserIdValidationFilter)
participant Spring Controller
Client->>Filter: HTTP Request (with Origin header)
alt OPTIONS (Preflight)
Filter-->>Client: Set CORS headers, respond 200 OK
else Other Methods
Filter->>Filter: Check Origin, set CORS headers if allowed
Filter->>Spring Controller: Forward request
Spring Controller-->>Filter: Response
Filter-->>Client: Response with CORS headers
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (28)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (26)
✨ 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: 4
🧹 Nitpick comments (9)
src/main/java/com/iemr/mmu/controller/fileSync/FileSyncController.java (1)
27-27: Remove unused import.The
@CrossOriginimport is no longer needed since all@CrossOriginannotations have been removed from this controller as part of the migration to centralized CORS handling.-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/mmu/controller/common/master/CommonMasterController.java (1)
27-27: Remove unused import.The
@CrossOriginimport is no longer needed since the@CrossOriginannotation has been removed from this controller as part of the migration to centralized CORS handling.-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/mmu/controller/quickconsult/QuickConsultController.java (1)
261-264: Enhance JSON parsing robustness
TheparseJsonRequestmethod directly parses the input string without handling malformed JSON. Consider wrappingJsonParser.parseStringin a try/catch to return a controlled error response (e.g., 400 Bad Request) when JSON is invalid.src/main/java/com/iemr/mmu/controller/labtechnician/LabTechnicianController.java (1)
139-142: Improve JSON parse error handling
TheparseJsonRequestutility assumes valid JSON input. It’s advisable to catch parsing exceptions (e.g.,JsonSyntaxException) and translate them into a proper error response instead of propagating exceptions downstream.src/main/java/com/iemr/mmu/controller/covid19/CovidController.java (1)
334-337: Add JSON parsing guardrails
Similar to other controllers,parseJsonRequestshould gracefully handle invalid JSON and respond with a descriptive error status rather than throwing uncaught exceptions.src/main/java/com/iemr/mmu/controller/teleconsultation/TeleConsultationController.java (1)
159-162: Strengthen JSON parsing error handling
The privateparseJsonRequestmethod should catch and handle invalid JSON inputs to avoid unhandled exceptions. Consider returning a structured error response for malformed JSON.src/main/java/com/iemr/mmu/controller/reports/ReportGateway.java (1)
27-27: Remove unusedCrossOriginimport
The@CrossOriginannotation has been removed from this controller, so the corresponding import is now unused. Removing it will clean up imports and avoid warnings.src/main/java/com/iemr/mmu/controller/location/LocationController.java (1)
28-28: Remove unused import.The
@CrossOriginimport is no longer needed since all CORS annotations have been removed from this controller.-import org.springframework.web.bind.annotation.CrossOrigin;src/main/java/com/iemr/mmu/config/CorsConfig.java (1)
16-16: Replace debug output with proper logging.Using
System.out.print()for debugging in production code is not recommended. Use a proper logger instead.- System.out.print(allowedOrigins); + logger.debug("Configured CORS allowed origins: {}", allowedOrigins);Add the logger field at the top of the class:
private static final Logger logger = LoggerFactory.getLogger(CorsConfig.class);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/mmu/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/mmu/controller/anc/ANCController.java(3 hunks)src/main/java/com/iemr/mmu/controller/cancerscreening/CancerScreeningController.java(17 hunks)src/main/java/com/iemr/mmu/controller/common/main/CommonController.java(6 hunks)src/main/java/com/iemr/mmu/controller/common/main/InsertCommonController.java(1 hunks)src/main/java/com/iemr/mmu/controller/common/master/CommonMasterController.java(1 hunks)src/main/java/com/iemr/mmu/controller/covid19/CovidController.java(8 hunks)src/main/java/com/iemr/mmu/controller/dataSyncActivity/StartSyncActivity.java(3 hunks)src/main/java/com/iemr/mmu/controller/dataSyncLayerCentral/MMUDataSyncVanToServer.java(5 hunks)src/main/java/com/iemr/mmu/controller/fileSync/FileSyncController.java(2 hunks)src/main/java/com/iemr/mmu/controller/generalOPD/GeneralOPDController.java(9 hunks)src/main/java/com/iemr/mmu/controller/labtechnician/LabTechnicianController.java(3 hunks)src/main/java/com/iemr/mmu/controller/location/LocationController.java(3 hunks)src/main/java/com/iemr/mmu/controller/login/IemrMmuLoginController.java(4 hunks)src/main/java/com/iemr/mmu/controller/ncdCare/NCDCareController.java(7 hunks)src/main/java/com/iemr/mmu/controller/ncdscreening/NCDController.java(6 hunks)src/main/java/com/iemr/mmu/controller/nurse/vitals/AnthropometryVitalsController.java(2 hunks)src/main/java/com/iemr/mmu/controller/pnc/PostnatalCareController.java(7 hunks)src/main/java/com/iemr/mmu/controller/quickconsult/QuickConsultController.java(5 hunks)src/main/java/com/iemr/mmu/controller/registrar/main/RegistrarController.java(1 hunks)src/main/java/com/iemr/mmu/controller/reports/ReportGateway.java(4 hunks)src/main/java/com/iemr/mmu/controller/snomedct/SnomedController.java(2 hunks)src/main/java/com/iemr/mmu/controller/teleconsultation/TeleConsultationController.java(2 hunks)src/main/java/com/iemr/mmu/utils/DynamicCorsFilter.java(1 hunks)src/main/java/com/iemr/mmu/utils/FilterConfig.java(1 hunks)src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/iemr/mmu/utils/FilterConfig.java (1)
src/main/java/com/iemr/mmu/config/CorsConfig.java (1)
Configuration(8-26)
src/main/java/com/iemr/mmu/config/CorsConfig.java (1)
src/main/java/com/iemr/mmu/utils/FilterConfig.java (1)
Configuration(8-26)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (42)
src/main/environment/common_ci.properties (1)
88-88: LGTM! Property addition follows established patterns.The
cors.allowed-originsproperty is correctly added using the environment variable placeholder pattern consistent with other properties in this configuration file. This enables deployment-specific CORS configuration.src/main/java/com/iemr/mmu/controller/fileSync/FileSyncController.java (1)
48-48: LGTM! Annotation removal aligns with centralized CORS strategy.The removal of
@CrossOrigin()annotations from the methods is correct and consistent with the migration to centralized CORS configuration via the newCorsConfigclass and filters.Also applies to: 68-68
src/main/java/com/iemr/mmu/controller/common/master/CommonMasterController.java (1)
39-39: LGTM! Class-level annotation removal aligns with centralized CORS strategy.The removal of the
@CrossOriginannotation from the class level is correct and consistent with the migration to centralized CORS configuration.src/main/environment/common_example.properties (1)
103-103: LGTM! Development-appropriate CORS configuration.The
cors.allowed-origins=http://localhost:*configuration is appropriate for a development/example environment, allowing CORS requests from any localhost port. This provides flexibility for local development while maintaining security boundaries.src/main/java/com/iemr/mmu/controller/common/main/InsertCommonController.java (1)
43-43:Details
✅ Verification successful
LGTM: Clean removal of @crossorigin annotations.
The removal of
@CrossOriginannotations aligns with the centralized CORS configuration approach. This is a good architectural improvement that consolidates CORS management.However, please verify that the centralized CORS configuration properly handles all endpoints in this controller, especially the file upload endpoint which may have specific CORS requirements:
🏁 Script executed:
#!/bin/bash # Description: Verify centralized CORS configuration exists and covers required endpoints # Expected: Find CorsConfig.java with proper configuration for file uploads echo "=== Checking for centralized CORS configuration ===" fd -t f "CorsConfig.java" --exec cat {} \; echo -e "\n=== Checking for DynamicCorsFilter ===" fd -t f "DynamicCorsFilter.java" --exec cat {} \; echo -e "\n=== Checking CORS properties configuration ===" fd -t f -e properties . --exec grep -l "cors.allowed-origins" {} \; fd -t f -e properties . --exec grep "cors.allowed-origins" {} \;Length of output: 2931
Centralized CORS configuration covers all endpoints including file uploads
I’ve verified that:
- CorsConfig.java registers CORS for “/**” with allowed methods including POST and OPTIONS.
- DynamicCorsFilter sets the Access-Control-Allow-Origin header for valid origins.
- The
cors.allowed-originsproperty is defined in your environment files.The removal of all
@CrossOriginannotations is safe—no further changes needed.src/main/java/com/iemr/mmu/controller/snomedct/SnomedController.java (2)
55-55: LGTM: Consistent removal of method-level @crossorigin annotations.The removal of
@CrossOriginannotations from thegetSnomedCTRecordmethod is consistent with the centralized CORS management approach.
81-81: LGTM: Consistent removal of method-level @crossorigin annotations.The removal of
@CrossOriginannotations from thegetSnomedCTRecordListmethod maintains consistency with the centralized CORS configuration strategy.src/main/java/com/iemr/mmu/controller/login/IemrMmuLoginController.java (4)
58-58: LGTM: Clean removal of @crossorigin from authentication endpoint.The removal of
@CrossOriginannotation fromgetUserServicePointVanDetailsis consistent with the centralized CORS approach.
78-78: LGTM: Consistent CORS annotation removal.The removal from
getServicepointVillagesmaintains consistency with the centralized CORS configuration strategy.
98-98: LGTM: Consistent CORS annotation removal.The removal from
getUserVanSpDetailsaligns with the centralized CORS management approach.
123-123:Details
✅ Verification successful
Ensure secure CORS configuration for authentication endpoints.
While the removal of
@CrossOriginfromgetVanMasteris consistent with the centralized approach, authentication-related endpoints require careful CORS configuration to maintain security.Please verify that the centralized CORS configuration properly secures authentication endpoints while allowing legitimate access:
🏁 Script executed:
#!/bin/bash # Description: Verify CORS configuration for authentication endpoints # Expected: Find secure CORS settings for login/authentication routes echo "=== Checking JWT filter CORS handling ===" fd -t f "JwtUserIdValidationFilter.java" --exec cat {} \; echo -e "\n=== Checking for authentication-specific CORS rules ===" rg -A 10 -B 5 "authentication|login|jwt" --type javaLength of output: 99810
Centralized CORS filter secures authentication endpoints
– The
JwtUserIdValidationFilteris registered for all paths (/*) inFilterConfigand injects CORS headers (Access-Control-Allow-Origin,…-Methods,…-Headers,…-Credentials) based on thecors.allowed-originsproperty before any JWT validation or OPTIONS handling.
– Paths like/user/userAuthenticate,/user/logOutUserFromConcurrentSessionand/user/refreshTokenbypass JWT validation but still receive the configured CORS headers.
– Please confirm that your productioncors.allowed-originsvalue is restricted to trusted domains.src/main/java/com/iemr/mmu/controller/ncdCare/NCDCareController.java (7)
50-50: LGTM: Clean removal of class-level @crossorigin annotation.The removal of class-level
@CrossOriginannotation fromNCDCareControlleris consistent with the centralized CORS configuration approach.
67-67: LGTM: Consistent method-level annotation removal.The removal of
@CrossOriginfromsaveBenNCDCareNurseDatamaintains consistency with the centralized CORS strategy.
101-101: LGTM: Consistent method-level annotation removal.The removal of
@CrossOriginfromsaveBenNCDCareDoctorDataaligns with the centralized CORS management approach.
160-160: LGTM: Consistent method-level annotation removal.The removal of
@CrossOriginfromgetBenNCDCareHistoryDetailsmaintains consistency with the centralized CORS configuration.
224-224: LGTM: Consistent method-level annotation removal.The removal of
@CrossOriginfromgetBenCaseRecordFromDoctorNCDCarealigns with the centralized CORS approach.
314-314: LGTM: Consistent method-level annotation removal.The removal of
@CrossOriginfromupdateNCDCareDoctorDatamaintains consistency with the centralized CORS configuration strategy.
338-342: LGTM: Minor formatting improvement in private method.The formatting changes to the
parseJsonRequestmethod improve code readability by using consistent bracing style. The functionality remains unchanged.src/main/java/com/iemr/mmu/controller/quickconsult/QuickConsultController.java (1)
50-52: Approve removal of controller-level @crossorigin
Removing per-controller CORS annotations in favor of centralized configuration is the correct approach; ensure that the appliedCorsConfigcovers the"/genOPD-QC-quickConsult/**"path and supports all required HTTP methods.Please verify that the global CORS mapping in
CorsConfigis indeed applied to this base path.src/main/java/com/iemr/mmu/controller/labtechnician/LabTechnicianController.java (1)
45-46: Approve removal of controller-level @crossorigin
Per-controller CORS annotations have been removed to centralize cross-origin policy. Confirm that the dynamic CORS filter orCorsConfigincludes the/labTechnician/**endpoints.src/main/java/com/iemr/mmu/controller/covid19/CovidController.java (1)
55-58: Approve centralized CORS handling removal
Eliminating per-controller CORS annotations aligns with the new global CORS configuration. Make sure that theCorsConfigorDynamicCorsFilterexplicitly covers"/pandemic/covid/**"endpoints to allow legitimate browser requests while restricting others.src/main/java/com/iemr/mmu/controller/teleconsultation/TeleConsultationController.java (1)
41-42: Approve removal of @crossorigin on TeleConsultationController
Centralizing CORS via configuration is preferred over scattered annotations. Please verify that/tc/**endpoints are included in the allowed origins list of your CORS configuration.src/main/java/com/iemr/mmu/controller/reports/ReportGateway.java (1)
44-46: Approve class-level CORS annotation removal
Migrating to a centralized CORS policy is beneficial. Ensure that/report/**endpoints are covered inCorsConfigmappings to allow only intended origins.src/main/java/com/iemr/mmu/controller/dataSyncLayerCentral/MMUDataSyncVanToServer.java (1)
47-47:Details
✅ Verification successful
Verify centralized CORS configuration is properly implemented.
The removal of
@CrossOriginannotations from this controller is part of the centralized CORS configuration refactoring. Since this controller handles critical data synchronization operations, ensure that the new centralized CORS setup properly covers these endpoints.Please verify that:
- The new
CorsConfigclass is properly configured- The
DynamicCorsFilteris correctly registered- The
cors.allowed-originsproperty is set in environment configurations- Cross-origin requests to these sync endpoints still work after the change
🏁 Script executed:
#!/bin/bash # Description: Verify centralized CORS configuration files exist and are properly configured # Check for CorsConfig class find . -name "CorsConfig.java" -type f | head -5 # Check for DynamicCorsFilter class find . -name "DynamicCorsFilter.java" -type f | head -5 # Check for CORS properties in config files find . -name "*.properties" -type f -exec grep -l "cors.allowed-origins" {} \; | head -5 # Check FilterConfig for CORS filter registration find . -name "FilterConfig.java" -type f -exec cat {} \; | head -20Length of output: 1273
To confirm that your centralized CORS setup covers all endpoints, let’s inspect the actual configuration in
CorsConfig.javaand the CORS logic inDynamicCorsFilter.java, as well as ensure the filter is registered. Please run:
🏁 Script executed:
#!/bin/bash # 1. Show the global CORS configuration sed -n '1,200p' src/main/java/com/iemr/mmu/config/CorsConfig.java # 2. Show the CORS logic in the dynamic filter sed -n '1,200p' src/main/java/com/iemr/mmu/utils/DynamicCorsFilter.java # 3. Verify where and how DynamicCorsFilter is registered grep -R "DynamicCorsFilter" -n srcLength of output: 2495
Centralized CORS configuration verified
–
CorsConfigimplementsWebMvcConfigurer, mapping “/**” withallowedOriginPatterns, allowed methods (including OPTIONS), headers, credentials and max‐age.
–DynamicCorsFilteris annotated@Component(Spring Boot auto‐registers anyFilterbean), sourcingcors.allowed-originsand settingAccess-Control-Allow-Origin.
–cors.allowed-originsis declared incommon_ci.propertiesandcommon_example.properties.
– No remaining@CrossOriginannotations are needed on your controllers; requests to all sync endpoints will be covered by this global setup.src/main/java/com/iemr/mmu/controller/generalOPD/GeneralOPDController.java (2)
49-49: CORS annotation removal is part of centralized configuration.The removal of
@CrossOriginannotations aligns with the centralized CORS configuration approach. This controller has multiple endpoints that likely receive cross-origin requests from frontend applications.
427-430: Minor formatting improvement in parseJsonRequest method.The formatting changes improve code readability by using consistent brace placement and proper method structure.
src/main/java/com/iemr/mmu/controller/registrar/main/RegistrarController.java (1)
221-221: Minor formatting improvement in logger statement.Added proper spacing after comma in the logger error statement, improving code readability.
src/main/java/com/iemr/mmu/controller/anc/ANCController.java (2)
271-271: CORS annotation removal consistent with centralized approach.The removal of
@CrossOriginannotations from this ANC controller follows the same centralized CORS configuration pattern applied across all controllers in this refactoring.
474-477: Consistent formatting improvement in parseJsonRequest method.The formatting changes are consistent with similar improvements made in other controllers, enhancing code readability and maintaining consistency across the codebase.
src/main/java/com/iemr/mmu/controller/location/LocationController.java (1)
43-43: LGTM! CORS annotations successfully removed for centralized configuration.The removal of
@CrossOriginannotations aligns perfectly with the PR objective to centralize CORS configuration. This approach improves maintainability by having a single point of CORS configuration rather than scattered annotations across controllers.Also applies to: 139-139, 165-165
src/main/java/com/iemr/mmu/controller/common/main/CommonController.java (1)
87-89: Excellent! Comprehensive removal of CORS annotations completed.All
@CrossOriginannotations have been systematically removed from this controller, which contains numerous endpoints. This centralization approach ensures consistent CORS handling across all the doctor/nurse worklist, history, and case sheet endpoints.Also applies to: 161-161, 643-643, 687-687, 821-821, 922-922
src/main/java/com/iemr/mmu/controller/pnc/PostnatalCareController.java (2)
47-47: CORS annotations successfully removed from all PNC endpoints.The systematic removal of
@CrossOriginannotations from all Postnatal Care controller methods supports the centralized CORS configuration strategy. All endpoints for PNC nurse and doctor workflows are now covered by the global CORS configuration.Also applies to: 156-156, 189-189, 221-221, 253-253, 285-285
452-456: Minor formatting improvement to parseJsonRequest method.The formatting changes improve code readability with consistent brace style and indentation. No functional changes to the JSON parsing logic.
src/main/java/com/iemr/mmu/controller/ncdscreening/NCDController.java (2)
51-51: CORS annotations successfully removed from NCD screening endpoints.The removal of
@CrossOriginannotations from all NCD (Non-Communicable Disease) screening controller methods completes the centralization for this domain. Both nurse and doctor workflow endpoints are now managed by the global CORS configuration.Also applies to: 56-56, 75-75, 111-111, 297-297, 330-330
443-447: Consistent formatting improvement applied to parseJsonRequest method.The formatting changes match the style improvements made in other controllers, providing consistency across the codebase. The JSON parsing functionality remains unchanged.
src/main/java/com/iemr/mmu/controller/nurse/vitals/AnthropometryVitalsController.java (1)
21-21: LGTM! Clean removal of @crossorigin annotations.The removal of both class-level and method-level
@CrossOriginannotations is consistent with the centralized CORS configuration approach. The controller logic remains unchanged and will now rely on the global CORS configuration.Also applies to: 31-31
src/main/java/com/iemr/mmu/controller/cancerscreening/CancerScreeningController.java (1)
53-53: LGTM! Comprehensive removal of @crossorigin annotations.All
@CrossOriginannotations have been systematically removed from the class and all endpoint methods. This is consistent with the centralized CORS configuration approach and maintains the existing functionality while delegating CORS handling to the global configuration.Also applies to: 71-71, 112-112, 139-139, 170-170, 200-200, 231-231, 262-262, 295-295, 328-328, 361-361, 393-393, 420-420, 466-466, 504-504, 535-535, 568-568
src/main/java/com/iemr/mmu/utils/FilterConfig.java (1)
3-3: LGTM! Proper integration of centralized CORS configuration.The implementation correctly:
- Imports the necessary
@Valueannotation- Injects the
cors.allowed-originsproperty- Passes the configuration to the
JwtUserIdValidationFilterconstructorThis establishes the proper configuration flow from properties to the filter implementation.
Also applies to: 11-12, 18-25
src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java (2)
4-4: LGTM! Proper constructor enhancement for CORS support.The constructor changes correctly:
- Add necessary imports for Arrays and @value
- Accept the allowedOrigins parameter
- Store the configuration for use in filtering logic
Also applies to: 8-8, 27-27, 29-33
45-57: LGTM! Appropriate CORS header handling and OPTIONS support.The implementation correctly:
- Validates the request origin against allowed origins
- Sets appropriate CORS headers when origin is allowed
- Handles OPTIONS preflight requests by short-circuiting with 200 response
- Skips JWT validation for preflight requests, which is the expected behavior
src/main/java/com/iemr/mmu/utils/DynamicCorsFilter.java (1)
17-18:⚠️ Potential issueFix type mismatch in @value injection.
The
@Valueannotation injects a String, but the field is declared asString[]. This will cause a type conversion error at runtime.- @Value("${cors.allowed-origins}") - private String[] allowedOrigins; + @Value("${cors.allowed-origins}") + private String allowedOriginsStr; + + private String[] getAllowedOrigins() { + if (allowedOriginsStr != null && !allowedOriginsStr.trim().isEmpty()) { + return allowedOriginsStr.split(","); + } + return new String[0]; + }Then update the usage in the filter method:
- if (origin != null && Arrays.asList(allowedOrigins).contains(origin)) { + String[] origins = getAllowedOrigins(); + if (origin != null && Arrays.asList(origins).contains(origin.trim())) {Likely an incorrect or invalid review comment.
src/main/java/com/iemr/mmu/controller/dataSyncActivity/StartSyncActivity.java (1)
47-49: Good centralization of CORS configuration.Removing the
@CrossOriginannotations from the controller and centralizing CORS configuration is a good architectural decision that improves maintainability and consistency.
| private boolean isOriginAllowed(String origin) { | ||
| if (origin == null || allowedOrigins == null || allowedOrigins.trim().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(pattern.replace(".", "\\.").replace("*", ".*"))); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the security of wildcard pattern matching.
The isOriginAllowed method converts wildcard patterns to regex, but this approach may have security implications. The current implementation pattern.replace(".", "\\.").replace("*", ".*") could potentially allow unintended origins.
Please verify that the wildcard pattern matching is secure and behaves as expected:
Consider implementing more precise pattern matching or using a dedicated library for origin validation to prevent potential security bypasses.
🏁 Script executed:
#!/bin/bash
# Description: Test the regex pattern conversion logic to ensure it's secure
# Expected: Wildcard patterns should only match intended origins
# Test various origin patterns that might be problematic
python3 << 'EOF'
import re
def is_origin_allowed_test(origin, allowed_pattern):
"""Test the pattern matching logic from the Java code"""
regex_pattern = allowed_pattern.replace(".", "\\.").replace("*", ".*")
return bool(re.match(regex_pattern, origin))
# Test cases
test_cases = [
# (origin, pattern, expected_result, description)
("https://example.com", "https://example.com", True, "Exact match"),
("https://example.com", "https://*.example.com", False, "Subdomain pattern should not match apex"),
("https://sub.example.com", "https://*.example.com", True, "Subdomain should match"),
("https://malicious.com", "https://*.example.com", False, "Different domain should not match"),
("https://example.com.evil.com", "https://example.com", False, "Domain suffix attack"),
("https://fakeexample.com", "https://*example.com", True, "Potential issue: overly broad pattern"),
]
print("Testing origin pattern matching:")
for origin, pattern, expected, description in test_cases:
result = is_origin_allowed_test(origin, pattern)
status = "✓" if result == expected else "✗ SECURITY ISSUE"
print(f"{status} {description}: origin='{origin}', pattern='{pattern}', result={result}")
EOFLength of output: 709
Secure wildcard origin validation by anchoring regex and parsing origin
The current isOriginAllowed logic converts allowedOrigins into a regex via
pattern.replace(".", "\\.").replace("*", ".*")but doesn’t enforce full‐string matching, allowing spoofed origins like https://example.com.evil.com to pass.
• File to update:
src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java(methodisOriginAllowed, lines 131–139)
• Wrap each converted pattern with^and$so that only exact (or specified wildcard) matches succeed.
• UsePattern.quotefor literal segments if you have more complex allowed‐origin formats.
• For stronger guarantees, parse the incoming origin viajava.net.URIand validate its scheme and host components explicitly, or switch to Spring’s built-in CORS support (CorsConfiguration#setAllowedOrigins/setAllowedOriginPatterns).
Example fix:
return Arrays.stream(allowedOrigins.split(","))
.map(String::trim)
- .anyMatch(pattern -> origin.matches(pattern.replace(".", "\\.").replace("*", ".*")));
+ .anyMatch(pattern -> {
+ String regex = "^"
+ + pattern.replace(".", "\\.").replace("*", ".*")
+ + "$";
+ return origin.matches(regex);
+ });This change prevents domain‐suffix and overly broad wildcard bypasses.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/mmu/utils/JwtUserIdValidationFilter.java around lines
131 to 139, the isOriginAllowed method's regex pattern matching lacks start (^)
and end ($) anchors, allowing spoofed origins like
"https://example.com.evil.com" to match. Fix this by wrapping each converted
pattern with ^ and $ to enforce full-string matching. Additionally, consider
parsing the origin using java.net.URI to validate scheme and host explicitly or
use Spring's CorsConfiguration with setAllowedOrigins or
setAllowedOriginPatterns for more robust origin validation.
| registry.addMapping("/**") | ||
| .allowedOriginPatterns(allowedOrigins.split(",")) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation and improve origin parsing.
The current implementation lacks null checking and proper handling of whitespace in the comma-separated origins list.
- registry.addMapping("/**")
- .allowedOriginPatterns(allowedOrigins.split(","))
+ if (allowedOrigins != null && !allowedOrigins.trim().isEmpty()) {
+ String[] origins = allowedOrigins.split(",");
+ for (int i = 0; i < origins.length; i++) {
+ origins[i] = origins[i].trim();
+ }
+ registry.addMapping("/**")
+ .allowedOriginPatterns(origins)Continue with the rest of the configuration after this validation.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/mmu/config/CorsConfig.java around lines 17 to 18, add
validation to check if allowedOrigins is null or empty before splitting. Trim
whitespace from each origin after splitting the comma-separated string to ensure
clean origin patterns. Then pass the cleaned list to allowedOriginPatterns.
Continue with the rest of the CORS configuration as before.
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| } else { | ||
| filterChain.doFilter(request, response); | ||
| } |
There was a problem hiding this comment.
Complete the CORS preflight response headers.
The current OPTIONS handling only returns HTTP 200 but is missing essential CORS headers required for preflight requests.
if ("OPTIONS".equalsIgnoreCase(request.getMethod())) {
+ response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
+ response.setHeader("Access-Control-Allow-Headers", request.getHeader("Access-Control-Request-Headers"));
+ response.setHeader("Access-Control-Allow-Credentials", "true");
+ response.setHeader("Access-Control-Max-Age", "3600");
response.setStatus(HttpServletResponse.SC_OK);
+ return; // Important: don't continue filter chain for preflight
} else {
filterChain.doFilter(request, response);
}🤖 Prompt for AI Agents
In src/main/java/com/iemr/mmu/utils/DynamicCorsFilter.java around lines 31 to
35, the handling of OPTIONS requests only sets the status to 200 OK but does not
include the necessary CORS headers for preflight responses. To fix this, add the
required CORS headers such as Access-Control-Allow-Origin,
Access-Control-Allow-Methods, Access-Control-Allow-Headers, and
Access-Control-Max-Age to the response before setting the status. This ensures
the preflight request is properly acknowledged with all needed headers.
| @Component | ||
| public class DynamicCorsFilter extends OncePerRequestFilter { | ||
|
|
||
| @Value("${cors.allowed-origins}") | ||
| private String[] allowedOrigins; | ||
|
|
||
| @Override | ||
| protected void doFilterInternal(HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| FilterChain filterChain) | ||
| throws ServletException, IOException { | ||
|
|
||
| String origin = request.getHeader("Origin"); | ||
| if (origin != null && Arrays.asList(allowedOrigins).contains(origin)) { | ||
| response.setHeader("Access-Control-Allow-Origin", origin); | ||
| } | ||
|
|
||
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| } else { | ||
| filterChain.doFilter(request, response); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider redundancy with CorsConfig.
This filter duplicates CORS functionality that's already handled by the CorsConfig class. Having both may lead to conflicts or inconsistent behavior.
Since both CorsConfig and DynamicCorsFilter handle CORS, verify if both are actually needed:
🏁 Script executed:
#!/bin/bash
# Search for CORS-related configurations and filters to understand the full setup
echo "=== Searching for CORS configurations ==="
rg -A 5 -B 5 "cors|CORS|Cross.*Origin" --type java
echo "=== Checking if DynamicCorsFilter is registered as a bean ==="
rg -A 10 "DynamicCorsFilter" --type java
echo "=== Looking for filter registrations ==="
rg -A 10 "FilterRegistrationBean" --type javaLength of output: 21907
🏁 Script executed:
#!/bin/bash
# Display the full CorsConfig to verify its CORS registration logic
sed -n '1,200p' src/main/java/com/iemr/mmu/config/CorsConfig.javaLength of output: 1010
Remove the redundant DynamicCorsFilter in favor of your global CorsConfig
Spring Boot is already auto‐registering your DynamicCorsFilter (it’s a @Component implementing OncePerRequestFilter), but you also have a complete CORS setup in CorsConfig (implements WebMvcConfigurer). Keeping both can lead to subtle conflicts or missing headers (e.g. methods, credentials, exposed headers).
Please clean this up by choosing one approach:
• Remove or disable DynamicCorsFilter:
- File: src/main/java/com/iemr/mmu/utils/DynamicCorsFilter.java
- Delete the class or remove its
@Componentannotation.
• Retain CorsConfig for all paths and origins:
- File: src/main/java/com/iemr/mmu/config/CorsConfig.java
- It already defines
allowedOriginPatterns, methods, headers, credentials and maxAge.
(Optional) Remove controller‐level @CrossOrigin annotations if you rely solely on the global config.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/mmu/utils/DynamicCorsFilter.java lines 14 to 37, the
DynamicCorsFilter duplicates CORS handling already implemented in CorsConfig,
causing potential conflicts. To fix this, remove the DynamicCorsFilter class
entirely or at least delete its @Component annotation to prevent it from being
auto-registered as a filter. Rely solely on the global CORS configuration in
CorsConfig for consistent and centralized CORS management.
vishwab1
force-pushed
the
cors
branch
2 times, most recently
from
48c33f4 to
7a80f3e
Compare
|
📋 Description
JIRA ID: AMM 593
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
Added cors orign method to restrict from the browsers
✅ 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