Fix the WASA Issue : IDOR Vulnerability

[vanitha1822]

📋 Description

JIRA ID:

AMM-1922

Use the UserID from the JWT Token, instead of passing it from payload, to prevent this IDOR Vulnerability.

---

✅ Type of Change

• 🛠 Refactor (change that is neither a fix nor a new feature)

---

Summary by CodeRabbit

• Chores

  • Updated project version to 3.6.1

• New Features

  • Implemented JWT token-based authentication validation for telemedicine specialist work list and user service endpoints. These endpoints now require valid authentication tokens and verify that the authenticated user matches the requested user ID. Requests with invalid or mismatched credentials will be rejected with an unauthorized access response.

[coderabbitai]

Walkthrough

Version bumped to 3.6.1 in pom.xml. JWT-based authorization validation added to multiple controller methods via HttpServletRequest inspection and token parsing with JwtUtil and CookieUtil utilities. Each modified endpoint now verifies userId from JWT cookie matches the request userId before proceeding. New getUserIdFromToken public method exposed in JwtUtil for token claim extraction.

Changes

Cohort / File(s)	Summary
Version Update pom.xml	Project version incremented from 3.4.0 to 3.6.1
JWT Authorization in Controllers src/main/java/com/iemr/mmu/controller/common/main/CommonController.java src/main/java/com/iemr/mmu/controller/location/LocationController.java src/main/java/com/iemr/mmu/controller/login/IemrMmuLoginController.java src/main/java/com/iemr/mmu/controller/teleconsultation/TeleConsultationController.java	Added HttpServletRequest parameter to multiple endpoints; added JwtUtil autowired field; implemented JWT token extraction from cookies and userId validation; return 5000 (Unauthorized) status when userId mismatch detected; preserved existing success paths for authorized requests
JWT Utility Enhancement src/main/java/com/iemr/mmu/utils/JwtUtil.java	Added public method getUserIdFromToken(String token) to extract userId claim from JWT token via existing extractAllClaims pathway

Sequence Diagram

sequenceDiagram
    actor Client
    participant Controller
    participant JwtUtil
    participant CookieUtil
    participant Service

    Client->>Controller: POST/GET request with userID
    activate Controller
    Controller->>CookieUtil: Extract JWT from cookies
    activate CookieUtil
    CookieUtil-->>Controller: JWT token
    deactivate CookieUtil
    
    Controller->>JwtUtil: getUserIdFromToken(token)
    activate JwtUtil
    JwtUtil->>JwtUtil: extractAllClaims(token)
    JwtUtil-->>Controller: tokenUserId
    deactivate JwtUtil
    
    alt userId matches
        Controller->>Service: Proceed with business logic
        activate Service
        Service-->>Controller: Response (200)
        deactivate Service
    else userId mismatch
        Controller-->>Client: Unauthorized (5000)
    end
    deactivate Controller

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring extra attention:
  • JWT extraction and userId validation logic consistency across four distinct controllers—verify all follow the same authorization pattern and error handling
  • Exception handling flow in LocationController and IemrMmuLoginController to ensure unauthorized scenarios short-circuit correctly
  • Verify JwtUtil.getUserIdFromToken integrates safely with existing extractAllClaims without introducing side effects
  • Check that HttpServletRequest parameter doesn't conflict with existing Spring routing or parameter binding

Possibly related PRs

• Pass the token from controller to fix the issue in data sync #92: Adds JWT-based authorization at controller layer using HttpServletRequest and modifies TeleConsultationController for token processing, implementing similar auth patterns.

Suggested reviewers

• vishwab1
• ravishanigarapu

Poem

🐰 A token flies in on the cookie breeze,
JwtUtil checks with practiced ease,
"Is your ID true?" the controller cries,
Authorization blooms—no more disguise! 🔐
Version three-point-six hops forward with cheer,
Security fortified, nice and clear! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title directly and accurately describes the main change: implementing IDOR (Insecure Direct Object Reference) vulnerability fixes through JWT-based user validation across multiple controllers.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch nd/vs/wasa_1922

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[drtechie]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (1)

src/main/java/com/iemr/mmu/controller/location/LocationController.java (1)

146-168: Consider simplifying the authorization check and logging unauthorized attempts

getLocDetailsBasedOnSpIDAndPsmIDNew correctly blocks requests when the JWT userId is null/missing/mismatched, but the nested structure makes it harder to follow, and unauthorized access is not logged.

You can improve readability and observability by:

• Checking JWT vs request userId up front.
• Logging unauthorized attempts.
• Only then validating spID / spPSMID.

For example:

-            JSONObject obj = new JSONObject(comingRequest);
-            String jwtToken = CookieUtil.getJwtTokenFromCookie(request);
-            String userId = jwtUtil.getUserIdFromToken(jwtToken);
-            if(userId != null && obj.has("userId") && userId.equals(String.valueOf( obj.getInt("userId")))) {
-            if (obj != null && obj.has("spID") && obj.has("spPSMID") && obj.get("spID") != null
-                    && obj.get("spPSMID") != null) {
+            JSONObject obj = new JSONObject(comingRequest);
+            String jwtToken = CookieUtil.getJwtTokenFromCookie(request);
+            String userId = jwtUtil.getUserIdFromToken(jwtToken);
+
+            if (userId == null || !obj.has("userId")
+                    || !userId.equals(String.valueOf(obj.getInt("userId")))) {
+                logger.warn("Unauthorized access in getLocDetailsBasedOnSpIDAndPsmIDNew for userId {}", userId);
+                response.setError(5000, "Unauthorized access");
+                return response.toString();
+            }
+
+            if (obj.has("spID") && obj.has("spPSMID") && obj.get("spID") != null
+                    && obj.get("spPSMID") != null) {
@@
-            }
-            }
-            else {
-                response.setError(5000, "Unauthorized access");
-            }

This keeps the IDOR protection semantics but makes the control flow more obvious and traceable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94e4ddb and fa5bd8f.

📒 Files selected for processing (6)

• pom.xml (1 hunks)
• src/main/java/com/iemr/mmu/controller/common/main/CommonController.java (5 hunks)
• src/main/java/com/iemr/mmu/controller/location/LocationController.java (4 hunks)
• src/main/java/com/iemr/mmu/controller/login/IemrMmuLoginController.java (4 hunks)
• src/main/java/com/iemr/mmu/controller/teleconsultation/TeleConsultationController.java (3 hunks)
• src/main/java/com/iemr/mmu/utils/JwtUtil.java (1 hunks)

🔇 Additional comments (7)

pom.xml (1)

8-8: Version bump aligns with target release

Artifact version updated to 3.6.1; this is consistent with the release-3.6.1 target branch and has no technical issues from this file’s perspective.

src/main/java/com/iemr/mmu/controller/teleconsultation/TeleConsultationController.java (1)

27-55: JwtUtil wiring in TeleConsultationController looks good

The JwtUtil import and autowired field are straightforward and used only where needed; no issues here.

src/main/java/com/iemr/mmu/controller/location/LocationController.java (1)

28-56: JwtUtil and CookieUtil wiring for location endpoints looks fine

The added imports and autowired JwtUtil field are consistent with the new JWT-based authorization flow and don’t introduce functional issues by themselves.

src/main/java/com/iemr/mmu/controller/login/IemrMmuLoginController.java (2)

36-43: JwtUtil and CookieUtil wiring in login controller is correct

The new imports and autowired JwtUtil field are consistent with the JWT-based validation pattern used elsewhere and look fine.

Also applies to: 57-59

---

155-158: Minor catch-block tweak in getVanMaster is fine

The catch block still logs the exception and returns an error response; the additional blank line has no functional impact.

src/main/java/com/iemr/mmu/controller/common/main/CommonController.java (2)

43-43: LGTM: Imports support JWT-based authorization.

The new imports for JwtUtil and CookieUtil are necessary for implementing the IDOR vulnerability fix.

Also applies to: 54-54

---

73-74: LGTM: JwtUtil properly autowired.

The dependency injection follows the existing pattern in this controller.

[sonarqubecloud]

Quality Gate failed

Failed conditions
B Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE