Skip to content

Conversation

@vishwab1
Copy link
Member

@vishwab1 vishwab1 commented Jun 13, 2025

πŸ“‹ Description

JIRA ID: 593

Please provide a summary of the change and the motivation behind it. Include relevant context and details.


βœ… Type of Change

  • 🐞 Bug fix (non-breaking change which resolves an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • πŸ”₯ Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • πŸ›  Refactor (change that is neither a fix nor a new feature)
  • βš™οΈ Config change (configuration file or build script updates)
  • πŸ“š Documentation (updates to docs or readme)
  • πŸ§ͺ Tests (adding new or updating existing tests)
  • 🎨 UI/UX (changes that affect the user interface)
  • πŸš€ Performance (improves performance)
  • 🧹 Chore (miscellaneous changes that don't modify src or test files)

ℹ️ 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
    • Introduced centralized configuration for Cross-Origin Resource Sharing (CORS), allowing dynamic management of allowed origins and HTTP methods.
  • Refactor
    • Removed all individual CORS annotations from controller classes and methods to rely on the new global CORS configuration.
  • Style
    • Improved code formatting and consistency in various files for better readability.
  • Bug Fixes
    • Updated request filtering to correctly handle HTTP OPTIONS requests, ensuring smoother preflight CORS handling.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 13, 2025

Walkthrough

This change introduces a global CORS configuration for the application by adding a centralized CorsConfig class, reading allowed origins from properties. All @CrossOrigin annotations are removed from controller classes and methods. Related configuration properties are added to environment property files, and the JWT filter is updated to explicitly allow OPTIONS requests.

Changes

Files/Groups Change Summary
src/main/environment/104_ci.properties, 104_example.properties Added cors.allowed-origins property for CORS configuration.
src/main/java/com/iemr/helpline104/config/CorsConfig.java Introduced new global CORS configuration class using WebMvcConfigurer.
All src/main/java/com/iemr/helpline104/controller/.../*Controller.java Removed all @CrossOrigin annotations from controller classes and methods. Minor formatting tweaks.
src/main/java/com/iemr/helpline104/service/CTI/CTIService.java Code formatting and stylistic improvements only; no logic changes.
src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java Allows HTTP OPTIONS requests to bypass JWT validation; minor formatting adjustments.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant SpringApp
    participant CorsConfig
    participant Controller

    Client->>SpringApp: HTTP request (any method)
    alt OPTIONS request
        SpringApp->>CorsConfig: Apply global CORS settings
        SpringApp->>Client: Respond with CORS headers
    else Other requests
        SpringApp->>CorsConfig: Apply global CORS settings
        SpringApp->>Controller: Route to endpoint
        Controller-->>SpringApp: Response
        SpringApp->>Client: Respond with data and CORS headers
    end
Loading

Possibly related issues

Suggested reviewers

  • ravishanigarapu

Poem

A bunny hopped through code one day,
Removing CORS from every way.
"No more here, no more thereβ€”
Let’s configure it all with global care!"
Now origins are set with grace,
And OPTIONS requests can run their race.
πŸ‡βœ¨

✨ Finishing Touches
  • πŸ“ Generate Docstrings

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.

❀️ Share
πŸͺ§ Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

♻️ Duplicate comments (2)
src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java (2)

77-80: Recommend adding explicit consumes = MediaType.APPLICATION_JSON_VALUE and produces = MediaType.APPLICATION_JSON_VALUE to this @PostMapping as well for consistent API contracts.


119-123: As above, please add consumes/produces attributes to this @PostMapping to standardize response and request handling.

🧹 Nitpick comments (6)
src/main/environment/104_example.properties (1)

25-27: Wildcard origin (http://localhost:*) is extremely permissive

Using * for every port on localhost is convenient for dev, but remember to:

  1. Replace with explicit origins before committing to shared or prod examples.
  2. Mention in README how to list multiple origins (comma-separated).

Consider http://localhost:3000,http://localhost:4200 instead.

src/main/java/com/iemr/helpline104/controller/covidMaster/CovidMasterController.java (1)

46-49: Cosmetic change – tighten the annotation wording

Two consecutive spaces in the summary string ("Master data for COVID patient") look accidental and will render oddly in Swagger UI.

-@Operation(summary = "Master data  for COVID patient")
+@Operation(summary = "Master data for COVID patient")
src/main/java/com/iemr/helpline104/controller/foodSafetyComplaint/FoodSafetyComplaintController.java (1)

67-69: Remove commented-out logging instead of keeping stale code

The split, commented log statement is no longer useful and generates noise in diffs.

-// logger.info("saveFoodComplaintDetails request " +
-// t_foodSafetyCopmlaint.toString());
src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java (1)

137-143: Add SameSite attribute when clearing userId cookie

The static-analysis hint is valid: the cleared cookie is re-set without a SameSite flag, weakening CSRF protection.

-        cookie.setSecure(true);
-        cookie.setMaxAge(0); // Invalidate the cookie
-        response.addCookie(cookie);
+        cookie.setSecure(true);
+        cookie.setMaxAge(0);               // invalidate
+        response.addCookie(cookie);
+        // Explicit SameSite attribute (Servlet <6 workaround)
+        response.addHeader("Set-Cookie",
+                "userId=; Path=/; HttpOnly; Secure; SameSite=Strict; Max-Age=0");

This single header addition hardens the endpoint without changing behaviour.

🧰 Tools
πŸͺ› ast-grep (0.38.1)

[warning] 142-142: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application

(cookie-missing-samesite-java)

src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java (1)

128-129: Program to interface
Use the List<DrugStrength> interface rather than the concrete ArrayList<DrugStrength>.

- ArrayList<DrugStrength> drugStrength = drugGroupService.getDrugStrength(...);
+ List<DrugStrength> drugStrength = drugGroupService.getDrugStrength(...);
src/main/java/com/iemr/helpline104/controller/secondaryCrmReports/SecondaryCRMReports.java (1)

486-488: Method name mismatch: endpoint path vs method name.
The @PostMapping value is /getPDSummaryReportByDate but the method is named getPDSummaryReport. Rename it to match the path for clarity and consistency:

- public ResponseEntity<Object> getPDSummaryReport(
+ public ResponseEntity<Object> getPDSummaryReportByDate(
πŸ“œ Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 5c938b0 and 15c56c5.

πŸ“’ Files selected for processing (34)
  • src/main/environment/104_ci.properties (1 hunks)
  • src/main/environment/104_example.properties (1 hunks)
  • src/main/java/com/iemr/helpline104/config/CorsConfig.java (1 hunks)
  • src/main/java/com/iemr/helpline104/controller/IMRMMR/IMRMMRController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/balVivha/BalVivahController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/beneficiarycall/BeneficiaryCallController.java (1 hunks)
  • src/main/java/com/iemr/helpline104/controller/bloodComponent/BloodComponentController.java (2 hunks)
  • src/main/java/com/iemr/helpline104/controller/bloodRequest/BloodRequestController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/callqamapping/CallQAMappingController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/casesheet/Helpline104BeneficiaryHistoryController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/cdss/ClinicalDecisionSupportController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/covidMaster/CovidMasterController.java (2 hunks)
  • src/main/java/com/iemr/helpline104/controller/directory/DirectoryServicesController.java (1 hunks)
  • src/main/java/com/iemr/helpline104/controller/disease/DiseaseController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/diseaseScreening/DiseaseScreeningController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java (6 hunks)
  • src/main/java/com/iemr/helpline104/controller/epidemicOutbreak/EpidemicOutbreakController.java (1 hunks)
  • src/main/java/com/iemr/helpline104/controller/feedback/FeedbackController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/feedbackType/FeedbackTypeController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/foodSafetyComplaint/FoodSafetyComplaintController.java (1 hunks)
  • src/main/java/com/iemr/helpline104/controller/healthCareWorkerType/HealthCareWorkerTypeController.java (1 hunks)
  • src/main/java/com/iemr/helpline104/controller/hihl/HIHLController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/location/CountryCityController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/location/LocationController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/organDonation/OrganDonationController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/prescription/PrescriptionController.java (2 hunks)
  • src/main/java/com/iemr/helpline104/controller/scheme/SchemeController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/secondaryCrmReports/SecondaryCRMReports.java (18 hunks)
  • src/main/java/com/iemr/helpline104/controller/sioHistory/ServiceImprovementOfficerController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/snomedct/SnomedController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/users/IEMRAdminController.java (0 hunks)
  • src/main/java/com/iemr/helpline104/controller/version/VersionController.java (1 hunks)
  • src/main/java/com/iemr/helpline104/service/CTI/CTIService.java (1 hunks)
  • src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java (3 hunks)
πŸ’€ Files with no reviewable changes (18)
  • src/main/java/com/iemr/helpline104/controller/feedbackType/FeedbackTypeController.java
  • src/main/java/com/iemr/helpline104/controller/sioHistory/ServiceImprovementOfficerController.java
  • src/main/java/com/iemr/helpline104/controller/users/IEMRAdminController.java
  • src/main/java/com/iemr/helpline104/controller/hihl/HIHLController.java
  • src/main/java/com/iemr/helpline104/controller/diseaseScreening/DiseaseScreeningController.java
  • src/main/java/com/iemr/helpline104/controller/location/CountryCityController.java
  • src/main/java/com/iemr/helpline104/controller/callqamapping/CallQAMappingController.java
  • src/main/java/com/iemr/helpline104/controller/snomedct/SnomedController.java
  • src/main/java/com/iemr/helpline104/controller/location/LocationController.java
  • src/main/java/com/iemr/helpline104/controller/cdss/ClinicalDecisionSupportController.java
  • src/main/java/com/iemr/helpline104/controller/feedback/FeedbackController.java
  • src/main/java/com/iemr/helpline104/controller/balVivha/BalVivahController.java
  • src/main/java/com/iemr/helpline104/controller/casesheet/Helpline104BeneficiaryHistoryController.java
  • src/main/java/com/iemr/helpline104/controller/scheme/SchemeController.java
  • src/main/java/com/iemr/helpline104/controller/organDonation/OrganDonationController.java
  • src/main/java/com/iemr/helpline104/controller/disease/DiseaseController.java
  • src/main/java/com/iemr/helpline104/controller/IMRMMR/IMRMMRController.java
  • src/main/java/com/iemr/helpline104/controller/bloodRequest/BloodRequestController.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java (1)
src/main/java/com/iemr/helpline104/utils/UserAgentContext.java (1)
  • UserAgentContext (3-18)
πŸͺ› ast-grep (0.38.1)
src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java

[warning] 142-142: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application

(cookie-missing-samesite-java)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (java)
πŸ”‡ Additional comments (12)
src/main/environment/104_ci.properties (1)

29-31: Validate placeholder & value format for cors.allowed-origins

All other placeholders follow the @env.*@ convention, but this one is @CORS_ALLOWED_ORIGINS@.
Confirm that your deployment pipeline replaces this token; otherwise the value will literally be @CORS_ALLOWED_ORIGINS@ at runtime and CORS will fail.

Also document the expected delimiter for multiple origins (comma-separated? space?).

src/main/java/com/iemr/helpline104/controller/healthCareWorkerType/HealthCareWorkerTypeController.java (1)

30-34: Ensure global CORS config permits POST + Authorization for this endpoint

@CrossOrigin was removed, so /beneficiary/get/healthCareWorkerTypes now relies solely on CorsConfig.
Double-check that:

β€’ POST is in the allowed methods list
β€’ Authorization is in allowed and exposed headers
β€’ allowed-origins property covers expected clients

Otherwise existing consumers will see 403 from pre-flight.

src/main/java/com/iemr/helpline104/controller/version/VersionController.java (1)

31-34: Verify CORS accessibility for /version endpoint

Similar to other controllers, @CrossOrigin is gone. If this endpoint is polled by web dashboards, ensure CorsConfig enables GET /version for those origins.

src/main/java/com/iemr/helpline104/controller/epidemicOutbreak/EpidemicOutbreakController.java (1)

30-35: CORS annotation successfully removed – verify allowed-origin list

The only change here is the removal of the @CrossOrigin import (and the related annotations elsewhere).
Given that CORS is now handled globally, this is correct.
Just double-check that the new cors.allowed-origins property contains every domain previously whitelisted on this controller, otherwise you may introduce unexpected 4xx responses in production.

src/main/java/com/iemr/helpline104/controller/covidMaster/CovidMasterController.java (1)

58-63: Global CORS: ensure OPTIONS isn’t blocked by headers filter

With the @CrossOrigin annotation gone, remember that pre-flight OPTIONS calls will no longer include the Authorization header.
Because this mapping is declared with headers = "Authorization", Spring will not match it for an OPTIONS request, so only the global CORS handler must answer.
If you observe 403s from API gateways, consider dropping the headers attribute from the mapping.

src/main/java/com/iemr/helpline104/controller/bloodComponent/BloodComponentController.java (2)

51-57: LGTM – Signature unchanged, only docstring trimmed

The @Parameter description update is non-functional and safe.


70-76: Same comment as above about headers = "Authorization"

If CORS pre-flight failures surface, consider whether this header constraint is still required.

src/main/java/com/iemr/helpline104/controller/prescription/PrescriptionController.java (1)

111-113: No behavioural change – only wrapping line

Nothing further to add.

src/main/java/com/iemr/helpline104/config/CorsConfig.java (1)

12-22:

Details

❌ Incorrect review comment

Avoid NPE when cors.allowed-origins is missing & fall back gracefully

allowedOrigins is injected without a default; if the property is absent or left blank, split(",") will throw a NullPointerException. Provide a default and guard against blank values before splitting.

-    @Value("${cors.allowed-origins}")
-    private String allowedOrigins;
+    @Value("${cors.allowed-origins:}")   // empty-string default
+    private String allowedOrigins;
…
-        registry.addMapping("/**")
-                .allowedOriginPatterns(
-                        Arrays.stream(allowedOrigins.split(","))
-                                .map(String::trim)
-                                .toArray(String[]::new))
+        String[] originArray = allowedOrigins == null || allowedOrigins.isBlank()
+                ? new String[]{"*"}      // sensible fallback
+                : Arrays.stream(allowedOrigins.split(","))
+                        .map(String::trim)
+                        .toArray(String[]::new);
+
+        registry.addMapping("/**")
+                .allowedOriginPatterns(originArray)

Also note that allowCredentials(true) cannot be combined with wildcard origins in some browsersβ€”double-check your runtime configuration.


NPE risk is unlikely; missing property fails startup, not null injection

Spring’s @value("${cors.allowed-origins}") will cause the application to fail on startup if the property is absent, and if the property is present but empty you get an empty string (not null), so allowedOrigins.split(",") won’t throw an NPE.

β€’ If you do want to support β€œno setting” at runtime, add a default in the annotation or via configuration rather than guarding for null:

@Value("${cors.allowed-origins:*}")   // fall back to wildcard if unset
private String allowedOrigins;

β€’ Alternatively, consider using @ConfigurationProperties with validation to enforce a non-blank list.

Also, be aware that some browsers will reject allowCredentials(true) when paired with wildcard originsβ€”double-check your CORS policy if you use "*" here.

Likely an incorrect or invalid review comment.

src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java (1)

36-40: OPTIONS short-circuit is spot-on

Pre-flight requests now skip JWT validation, unblocking CORS. Nice addition.

src/main/java/com/iemr/helpline104/controller/directory/DirectoryServicesController.java (1)

30-31: Removal of unused @CrossOrigin import looks clean

The stray import was safely eliminated after centralising CORS.

src/main/java/com/iemr/helpline104/controller/secondaryCrmReports/SecondaryCRMReports.java (1)

79-79: Also applies to: 106-106, 133-133, 160-160, 187-187, 214-214, 242-242, 268-268, 296-296, 323-323, 351-351, 379-379, 405-405, 435-435, 460-460

Comment on lines +58 to 60
ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);

return new Gson().toJson(res.getBody());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Gson().toJson(res.getBody()) double-encodes JSON

res.getBody() is already a String; wrapping it in toJson turns it into a JSON string containing JSON (adds extra quotes / escapes). Return the body as-is or deserialize and re-serialize properly.

-return new Gson().toJson(res.getBody());
+return res.getBody();
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);
return new Gson().toJson(res.getBody());
ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);
return res.getBody();
πŸ€– Prompt for AI Agents
In src/main/java/com/iemr/helpline104/service/CTI/CTIService.java around lines
58 to 60, the code calls Gson().toJson on res.getBody(), which is already a JSON
string, causing double encoding and extra quotes. To fix this, return
res.getBody() directly without wrapping it in Gson().toJson, or if
transformation is needed, deserialize the string into an object and then
serialize it back properly.

Comment on lines +43 to +46
private String domain = "http://10.201.13.17";
private String resFormat = "1";
private String ip = "10.208.94.134";

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Externalize domain/IP/format – avoid hard-coding connection details

domain, resFormat, and ip are hard-coded. This prevents environment-specific configuration and makes testing harder.

- private String domain = "http://10.201.13.17";
- private String resFormat = "1";
- private String ip = "10.208.94.134";
+@Value("${cti.domain}")
+private String domain;
+
+@Value("${cti.res-format:1}")
+private String resFormat;
+
+@Value("${cti.ip}")
+private String ip;

Add the three keys to the relevant *.properties files and/or Config Server.

πŸ€– Prompt for AI Agents
In src/main/java/com/iemr/helpline104/service/CTI/CTIService.java around lines
43 to 46, the domain, resFormat, and ip variables are hard-coded, which limits
flexibility and environment-specific configuration. To fix this, remove the
hard-coded values and instead load these properties from external configuration
files such as application.properties or from a Config Server. Add corresponding
keys for domain, resFormat, and ip in the properties files and update the code
to read these values using a configuration management approach like Spring's
@Value annotation or a properties loader.

Comment on lines +55 to 60
String url = domain + "/apps/appsHandler.php?transaction_id=" + transaction_id + "&agent_id=" + agent_id
+ "&ip=" + ip + "&resFormat=" + resFormat;

ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);

return new Gson().toJson(res.getBody());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Build URLs safely and reuse common code

  1. Manual string concatenation risks injection and misses URI-encoding for agent_id.
  2. The same logic exists in all three methods – violates DRY.
-String url = domain + "/apps/appsHandler.php?transaction_id=" + transaction_id +
-             "&agent_id=" + agent_id + "&ip=" + ip + "&resFormat=" + resFormat;
+String url = UriComponentsBuilder.fromHttpUrl(domain + "/apps/appsHandler.php")
+        .queryParam("transaction_id", transaction_id)
+        .queryParam("agent_id", agent_id)
+        .queryParam("ip", ip)
+        .queryParam("resFormat", resFormat)
+        .build()
+        .encode()
+        .toUriString();

Extract this construction into a private helper so loginAgent, logoutAgent, and transferCall call the same utility.

πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
String url = domain + "/apps/appsHandler.php?transaction_id=" + transaction_id + "&agent_id=" + agent_id
+ "&ip=" + ip + "&resFormat=" + resFormat;
ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);
return new Gson().toJson(res.getBody());
String url = UriComponentsBuilder.fromHttpUrl(domain + "/apps/appsHandler.php")
.queryParam("transaction_id", transaction_id)
.queryParam("agent_id", agent_id)
.queryParam("ip", ip)
.queryParam("resFormat", resFormat)
.build()
.encode()
.toUriString();
ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);
return new Gson().toJson(res.getBody());
πŸ€– Prompt for AI Agents
In src/main/java/com/iemr/helpline104/service/CTI/CTIService.java around lines
55 to 60, the URL is built using manual string concatenation which risks
injection and lacks URI encoding for parameters like agent_id. To fix this,
create a private helper method that safely constructs the URL using proper URI
encoding for all parameters. Then refactor the loginAgent, logoutAgent, and
transferCall methods to call this helper, eliminating duplicated URL
construction code and adhering to DRY principles.

Comment on lines +80 to +90

final String transaction_id = "CTI_TRANSFER_AGENT";

InetAddress inetAddress = InetAddress.getLocalHost();
//String ip = inetAddress.getHostAddress();
// String ip = inetAddress.getHostAddress();

RestTemplate restTemplate = new RestTemplate();
String url=domain+"/apps/appsHandler.php?transaction_id="+transaction_id+"&transfer_ from="+transfer_from+"&transfer_to="+transfer_to+"&ip="+ip;

ResponseEntity<String> res= restTemplate.getForEntity(url, String.class);

String url = domain + "/apps/appsHandler.php?transaction_id=" + transaction_id + "&transfer_ from="
+ transfer_from + "&transfer_to=" + transfer_to + "&ip=" + ip;

ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Typo in query parameter – space after underscore breaks request

"&transfer_ from=" contains a space, producing an invalid query param name and 400 responses.

-String url = domain + "/apps/appsHandler.php?transaction_id=" + transaction_id + "&transfer_ from="
+String url = domain + "/apps/appsHandler.php?transaction_id=" + transaction_id + "&transfer_from="
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
final String transaction_id = "CTI_TRANSFER_AGENT";
InetAddress inetAddress = InetAddress.getLocalHost();
//String ip = inetAddress.getHostAddress();
// String ip = inetAddress.getHostAddress();
RestTemplate restTemplate = new RestTemplate();
String url=domain+"/apps/appsHandler.php?transaction_id="+transaction_id+"&transfer_ from="+transfer_from+"&transfer_to="+transfer_to+"&ip="+ip;
ResponseEntity<String> res= restTemplate.getForEntity(url, String.class);
String url = domain + "/apps/appsHandler.php?transaction_id=" + transaction_id + "&transfer_ from="
+ transfer_from + "&transfer_to=" + transfer_to + "&ip=" + ip;
ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);
final String transaction_id = "CTI_TRANSFER_AGENT";
InetAddress inetAddress = InetAddress.getLocalHost();
// String ip = inetAddress.getHostAddress();
RestTemplate restTemplate = new RestTemplate();
String url = domain + "/apps/appsHandler.php?transaction_id=" + transaction_id + "&transfer_from="
+ transfer_from + "&transfer_to=" + transfer_to + "&ip=" + ip;
ResponseEntity<String> res = restTemplate.getForEntity(url, String.class);
πŸ€– Prompt for AI Agents
In src/main/java/com/iemr/helpline104/service/CTI/CTIService.java between lines
80 and 90, the query parameter name "transfer_ from" contains an unintended
space after the underscore, causing an invalid request and 400 errors. Remove
the space so the parameter reads "transfer_from" to correctly format the URL
query string.

Comment on lines +75 to +88
String jwtFromCookie = getJwtTokenFromCookies(request);
String jwtFromHeader = request.getHeader("JwtToken");
String authHeader = request.getHeader("Authorization");

if (jwtFromCookie != null) {
logger.info("Validating JWT token from cookie");
if (jwtAuthenticationUtil.validateUserIdAndJwtToken(jwtFromCookie)) {
AuthorizationHeaderRequestWrapper authorizationHeaderRequestWrapper = new AuthorizationHeaderRequestWrapper(
request, "");
filterChain.doFilter(authorizationHeaderRequestWrapper, servletResponse);
return;
}
} else if (jwtFromHeader != null) {
logger.info("Validating JWT token from header");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Header-name mismatch breaks JWT validation for non-cookie flows

jwtFromHeader is read with "JwtToken" (mixed case) while elsewhere you log "Jwttoken". Because header names are case-insensitive but your clients appear to use exactly Jwttoken, this inconsistency is easy to miss and causes silent auth failures.

-            String jwtFromCookie = getJwtTokenFromCookies(request);
-            String jwtFromHeader = request.getHeader("JwtToken");
+            String jwtFromCookie = getJwtTokenFromCookies(request);
+            // Use the same canonical header everywhere
+            String jwtFromHeader = request.getHeader("Jwttoken");

Consider extracting the header name into a constant to avoid future drift.

πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
String jwtFromCookie = getJwtTokenFromCookies(request);
String jwtFromHeader = request.getHeader("JwtToken");
String authHeader = request.getHeader("Authorization");
if (jwtFromCookie != null) {
logger.info("Validating JWT token from cookie");
if (jwtAuthenticationUtil.validateUserIdAndJwtToken(jwtFromCookie)) {
AuthorizationHeaderRequestWrapper authorizationHeaderRequestWrapper = new AuthorizationHeaderRequestWrapper(
request, "");
filterChain.doFilter(authorizationHeaderRequestWrapper, servletResponse);
return;
}
} else if (jwtFromHeader != null) {
logger.info("Validating JWT token from header");
String jwtFromCookie = getJwtTokenFromCookies(request);
// Use the same canonical header everywhere
String jwtFromHeader = request.getHeader("Jwttoken");
String authHeader = request.getHeader("Authorization");
if (jwtFromCookie != null) {
logger.info("Validating JWT token from cookie");
if (jwtAuthenticationUtil.validateUserIdAndJwtToken(jwtFromCookie)) {
AuthorizationHeaderRequestWrapper authorizationHeaderRequestWrapper = new AuthorizationHeaderRequestWrapper(
request, "");
filterChain.doFilter(authorizationHeaderRequestWrapper, servletResponse);
return;
}
} else if (jwtFromHeader != null) {
logger.info("Validating JWT token from header");
πŸ€– Prompt for AI Agents
In src/main/java/com/iemr/helpline104/utils/JwtUserIdValidationFilter.java
around lines 75 to 88, the header name for retrieving the JWT token is
inconsistently cased as "JwtToken" while clients use "Jwttoken". This mismatch
causes silent authentication failures. To fix this, define a constant for the
JWT header name with the exact casing "Jwttoken" and use this constant
everywhere in the code to read the header, ensuring consistent and correct
header access.

Comment on lines 88 to +93
@PostMapping(value = "update/beneficiaryCallID", headers = "Authorization")
public String updateBeneficiaryIDInCall(
@Parameter(description = "{\"callID\":\"integer\", \"beneficiaryRegID\":\"long\"}") @RequestBody String beneficiaryCall) {
OutputResponse output = new OutputResponse();
Integer startedCall = null;
try {
BeneficiaryCall beneficiarycall = inputMapper.gson().fromJson(beneficiaryCall, BeneficiaryCall.class);
@Parameter(description = "{\"callID\":\"integer\", \"beneficiaryRegID\":\"long\"}") @RequestBody String beneficiaryCall) {

OutputResponse output = new OutputResponse();
Integer startedCall = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing leading slash breaks endpoint mapping

Both paths lose the leading /, producing URLs like
/beneficiaryupdate/beneficiaryCallID instead of /beneficiary/update/….

-    @PostMapping(value = "update/beneficiaryCallID", headers = "Authorization")
+    @PostMapping(value = "/update/beneficiaryCallID", headers = "Authorization")

Do the same for set/callHistory further below:

-    @PostMapping(value = "set/callHistory", produces = MediaType.APPLICATION_JSON, headers = "Authorization")
+    @PostMapping(value = "/set/callHistory", produces = MediaType.APPLICATION_JSON, headers = "Authorization")

Without this fix, clients will receive 404s.

πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@PostMapping(value = "update/beneficiaryCallID", headers = "Authorization")
public String updateBeneficiaryIDInCall(
@Parameter(description = "{\"callID\":\"integer\", \"beneficiaryRegID\":\"long\"}") @RequestBody String beneficiaryCall) {
OutputResponse output = new OutputResponse();
Integer startedCall = null;
try {
BeneficiaryCall beneficiarycall = inputMapper.gson().fromJson(beneficiaryCall, BeneficiaryCall.class);
@Parameter(description = "{\"callID\":\"integer\", \"beneficiaryRegID\":\"long\"}") @RequestBody String beneficiaryCall) {
OutputResponse output = new OutputResponse();
Integer startedCall = null;
@PostMapping(value = "/update/beneficiaryCallID", headers = "Authorization")
public String updateBeneficiaryIDInCall(
@Parameter(description = "{\"callID\":\"integer\", \"beneficiaryRegID\":\"long\"}") @RequestBody String beneficiaryCall) {
OutputResponse output = new OutputResponse();
Integer startedCall = null;
πŸ€– Prompt for AI Agents
In
src/main/java/com/iemr/helpline104/controller/beneficiarycall/BeneficiaryCallController.java
around lines 88 to 93, the @PostMapping path "update/beneficiaryCallID" is
missing a leading slash, causing incorrect endpoint URLs and resulting in 404
errors. Add a leading slash to the path, changing it to
"/update/beneficiaryCallID". Also, apply the same fix to the "set/callHistory"
mapping further down by adding a leading slash to ensure correct URL mapping.

Comment on lines +140 to 143
@Operation(summary = "Fetch drug name list")
@PostMapping(value = "/getDrugDetailList", headers = "Authorization")
public String getDrugNameList(
@Parameter(description = "{\"providerServiceMapID\":\"integer\"}") @RequestBody String request) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Inconsistent naming: path vs. method
The endpoint path /getDrugDetailList and method name getDrugNameList are misaligned. Rename one to unify naming conventions and prevent confusion.

πŸ€– Prompt for AI Agents
In
src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java
around lines 140 to 143, the method name getDrugNameList does not match the
endpoint path /getDrugDetailList, causing inconsistency. Rename either the
method to getDrugDetailList or the endpoint path to /getDrugNameList so that
both names align and clearly represent the same functionality.

Comment on lines +102 to 104
@Operation(summary = "Fetch drug frequency details")
@PostMapping(value = "/get/drugFrequency", headers = "Authorization")
public String getDrugFrequency() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ Refactor suggestion

Use GET for idempotent fetch
getDrugFrequency() does not consume a request bodyβ€”consider switching to @GetMapping("/get/drugFrequency") (and removing the headers restriction) to better reflect REST semantics.

πŸ€– Prompt for AI Agents
In
src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java
around lines 102 to 104, change the annotation from @PostMapping to @GetMapping
for the getDrugFrequency() method, remove the headers="Authorization"
restriction, and update the mapping to @GetMapping("/get/drugFrequency") to
correctly use GET for an idempotent fetch operation without requiring a request
body.

Comment on lines +56 to +59
@Operation(summary = "Fetch drug groups")
@PostMapping(value = "/get/drugGroups", headers = "Authorization")
public String getDrugGroups(@Parameter(description = "{\"serviceProviderID\":\"integer\"}") @RequestBody String request) {
public String getDrugGroups(
@Parameter(description = "{\"serviceProviderID\":\"integer\"}") @RequestBody String request) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ’‘ Verification agent

🧩 Analysis chain

Centralize CORS & specify media types
Removed controller-level @CrossOrigin; ensure your global CorsConfig bean covers /beneficiary/** and properly handles OPTIONS preflight. For better API clarity, add explicit consumes and produces to the @PostMapping.

As a follow-up, verify preflight handling with:


🏁 Script executed:

#!/bin/bash
# Verify CORS preflight on getDrugGroups endpoint
curl -X OPTIONS http://localhost:8080/beneficiary/get/drugGroups \
  -H "Origin: http://example.com" \
  -H "Access-Control-Request-Method: POST" -I

Length of output: 771


🏁 Script executed:

#!/bin/bash
# Display class-level annotations and mappings in DrugGroupController
sed -n '1,200p' src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java

echo "---- @RequestMapping occurrences ----"
rg -n "@RequestMapping" src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java || true

echo "---- Global CORS config classes ----"
rg -n "class CorsConfig" -n src

echo "---- CORS registry patterns ----"
# If a CorsConfig exists, inspect its addMapping lines
rg -n "addMapping" -n src/main/java/com/iemr/helpline104/**/CorsConfig.java || true

Length of output: 6620


Add explicit consumes/produces to your endpoint; global CORS is already covering /beneficiary/**

Our global CorsConfig registers /** (including /beneficiary/**), so you can safely remove any controller-level @CrossOrigin. To improve API clarity and ensure correct content negotiation, please update your @PostMapping annotations to declare the media types:

β€’ File: src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java
Update the mapping on line 56 (and similarly for other endpoints):

-    @PostMapping(value = "/get/drugGroups", headers = "Authorization")
+    @PostMapping(
+        value = "/get/drugGroups",
+        consumes = MediaType.APPLICATION_JSON_VALUE,
+        produces = MediaType.APPLICATION_JSON_VALUE,
+        headers = "Authorization"
+    )

β€’ Don’t forget to add the import:

import org.springframework.http.MediaType;
πŸ€– Prompt for AI Agents
In
src/main/java/com/iemr/helpline104/controller/drugGroup/DrugGroupController.java
around lines 56 to 59, the @PostMapping annotation lacks explicit consumes and
produces attributes for media types. To fix this, remove any controller-level
@CrossOrigin annotations since global CorsConfig covers /beneficiary/**, then
update the @PostMapping to include consumes = MediaType.APPLICATION_JSON_VALUE
and produces = MediaType.APPLICATION_JSON_VALUE. Also, add the import for
org.springframework.http.MediaType to support these attributes.

@PostMapping(value = "/getROSummaryReportByDate", headers = "Authorization")
public ResponseEntity<Object> getROSummaryReportByDate(@RequestBody String jsonRequest)
{
public ResponseEntity<Object> getROSummaryReportByDate(@RequestBody String jsonRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

CORS preflight conflict: remove headers = "Authorization" from mapping.
Restricting the @PostMapping to requests with an Authorization header blocks OPTIONS preflight, breaking CORS. Remove the headers attribute and rely on your global CorsConfig and security filter to enforce auth.

- @PostMapping(value = "/getROSummaryReportByDate", headers = "Authorization")
+ @PostMapping("/getROSummaryReportByDate")

Apply the same removal for all other endpoints.

πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public ResponseEntity<Object> getROSummaryReportByDate(@RequestBody String jsonRequest) {
// Before:
- @PostMapping(value = "/getROSummaryReportByDate", headers = "Authorization")
+ @PostMapping("/getROSummaryReportByDate")
public ResponseEntity<Object> getROSummaryReportByDate(@RequestBody String jsonRequest) {
// ...
}
πŸ€– Prompt for AI Agents
In
src/main/java/com/iemr/helpline104/controller/secondaryCrmReports/SecondaryCRMReports.java
at line 56, remove the headers = "Authorization" attribute from the @PostMapping
annotation on the getROSummaryReportByDate method to avoid blocking CORS
preflight OPTIONS requests. This change should be applied to all other endpoints
in this controller that have the same headers restriction. Rely on the global
CorsConfig and security filters to handle authorization instead.

@vishwab1 vishwab1 closed this Jun 17, 2025
@vishwab1 vishwab1 deleted the corsCheck branch June 17, 2025 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants