feat(cors): Global CORS Configuration for AMRIT API Services #48
Conversation
|
Warning Rate limit exceeded@kevalkanp1011 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 53 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 (5)
WalkthroughThis change introduces a centralized, global CORS (Cross-Origin Resource Sharing) configuration for the application. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SpringApp
participant CorsConfig
participant Controller
Client->>SpringApp: HTTP Request (with Origin header)
SpringApp->>CorsConfig: Evaluate CORS policy (allowed origins, methods, headers)
CorsConfig-->>SpringApp: CORS headers set (if allowed)
SpringApp->>Controller: Route request
Controller-->>SpringApp: Response
SpringApp-->>Client: Response (with CORS headers if applicable)
Possibly related issues
✨ 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: 0
♻️ Duplicate comments (8)
src/main/java/com/iemr/tm/controller/specialist/SpecialistController.java (2)
69-69: Duplicate: see comment at 53Consistent removal of
@CrossOriginacross methods aligns with the global configuration.
104-104: Duplicate: see comment at 53Consistent removal of
@CrossOriginacross methods aligns with the global configuration.src/main/java/com/iemr/tm/controller/schedule/SchedulingController.java (6)
75-75: Duplicate: see comment at 55Removal of
@CrossOriginis consistent with the centralized CORS configuration.
94-94: Duplicate: see comment at 55Removal of
@CrossOriginis consistent with the centralized CORS configuration.
112-112: Duplicate: see comment at 55Removal of
@CrossOriginis consistent with the centralized CORS configuration.
134-134: Duplicate: see comment at 55Removal of
@CrossOriginis consistent with the centralized CORS configuration.
152-152: Duplicate: see comment at 55Removal of
@CrossOriginis consistent with the centralized CORS configuration.
170-170: Duplicate: see comment at 55Removal of
@CrossOriginis consistent with the centralized CORS configuration.
🧹 Nitpick comments (5)
src/main/environment/common_ci.properties (1)
27-27: Ensure environment variable injection for CORS allowed originsThe placeholder
@CORS_ALLOWED_ORIGINS@must be provided by your CI pipeline; if it’s unset or empty, CORS requests may be blocked. Consider adding a default fallback (e.g., using Spring’s${…:default}syntax) or documenting this requirement in your deployment guide.src/main/java/com/iemr/tm/config/CorsConfig.java (4)
11-12: Consider binding origins as a list for type safety.
Instead of injecting a raw comma-delimited string, you could use@ConfigurationProperties(prefix = "cors")to bindList<String> allowedOrigins, or inject directly:@Value("${cors.allowed-origins}") private List<String> allowedOrigins;This removes manual splitting and improves readability.
16-17: Trim and validate origin patterns after splitting.
allowedOrigins.split(",")may yield entries with whitespace or empty strings. Use:String[] origins = Arrays.stream(allowedOrigins.split(",")) .map(String::trim) .filter(s -> !s.isEmpty()) .toArray(String[]::new); .allowedOriginPatterns(origins)to avoid invalid patterns.
18-22: Externalize CORS parameters for flexibility.
Hardcoding HTTP methods, headers, exposed headers, credentials allowance, and maxAge limits flexibility. Consider moving these values to properties:cors.allowed-methods=GET,POST,PUT,DELETE,OPTIONS cors.allowed-headers=* cors.exposed-headers=Authorization,Jwttoken cors.allow-credentials=true cors.max-age=3600and bind via
@ConfigurationProperties.
14-23: Add JavaDoc to document CORS policy behavior.
Including Javadoc onaddCorsMappingsexplaining which origins, methods, and headers are configured aids maintainability for future readers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/java/com/iemr/tm/config/CorsConfig.java(1 hunks)src/main/java/com/iemr/tm/controller/schedule/SchedulingController.java(7 hunks)src/main/java/com/iemr/tm/controller/specialist/SpecialistController.java(4 hunks)src/main/java/com/iemr/tm/controller/van/VanController.java(1 hunks)
🔇 Additional comments (7)
src/main/environment/common_example.properties (1)
23-23: Validate wildcard support in global CORS configurationYou’re using wildcard patterns (
http://localhost:*,http://127.0.0.1:*) incors.allowed-origins. Verify that yourCorsConfigcallsallowedOriginPatterns(...)(introduced in Spring 5.3) rather thanallowedOrigins(...), as onlyallowedOriginPatternswill correctly interpret these wildcards.src/main/java/com/iemr/tm/controller/van/VanController.java (1)
49-49: Approve removal of method-level CORS annotationRemoving the
@CrossOriginfrom this endpoint is correct now that you have a centralized CORS policy. Don’t forget to clean up the leftover importorg.springframework.web.bind.annotation.CrossOriginsince it’s no longer used.src/main/java/com/iemr/tm/controller/specialist/SpecialistController.java (1)
53-53: Approve removal of CORS annotationCentralizing CORS in
CorsConfigis a good move—removing this per-method@CrossOriginis appropriate. Please also remove the unused importorg.springframework.web.bind.annotation.CrossOrigin;.src/main/java/com/iemr/tm/controller/schedule/SchedulingController.java (1)
55-55: Approve removal of method-level CORS annotationThis endpoint no longer needs its own
@CrossOrigin—the global CORS setup will apply. Clean up the now-unused importorg.springframework.web.bind.annotation.CrossOrigin.src/main/java/com/iemr/tm/config/CorsConfig.java (3)
3-6: Imports are appropriate. The necessary Spring MVC and configuration imports correctly set up the global CORS config.
8-9: Global CORS configuration is registered correctly. Using@ConfigurationwithWebMvcConfigurercleanly replaces per-controller@CrossOriginannotations.
20-20: Verify the exposed header name for consistency.
The header"Jwttoken"may have inconsistent casing. Ensure this matches the actual header sent by clients, and consider defining these as constants to prevent typos.
|
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
This PR introduces a global CORS configuration for the AMRIT platform API services, in alignment with requirements. The goal is to enhance cross-origin request handling by removing controller-level CORS annotations and setting up centralized, environment-based CORS policies.
✅ 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