Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMM-1048 | Assam Pre Prod in MO Save and Frequency is not working #77

Merged
merged 7 commits into from
Nov 20, 2024
Merged

AMM-1048 | Assam Pre Prod in MO Save and Frequency is not working #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6f448ac
Merge pull request #1 from PSMRI/develop
srishtigrp78 Sep 18, 2024
1b7f584
Merge pull request #4 from PSMRI/develop
srishtigrp78 Nov 14, 2024
576bcdd
adding changes for getByBenRegIdList API
Nov 19, 2024
fef5c2d
added changes suggested by sonar cloud analysis
Nov 19, 2024
866ec9f
removing commented code
Nov 19, 2024
2be7f43
making sure to not log user controlled data
Nov 19, 2024
4edcf18
adding changes related to class Casting and removing new method
Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 22 additions & 18 deletions src/main/java/com/iemr/common/identity/controller/IdentityController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,26 +697,30 @@ public String getPartialBeneficiariesByBenRegIds(
@Operation(summary = "Get beneficiaries by beneficiary registration id")
@PostMapping(path = "/getByBenRegIdList", headers = "Authorization")
public String getBeneficiariesByBenRegIds(
@Param(value = " {\"beneficiaryRegID\": \"Long\"}") @RequestBody String benRegIds) {
logger.info("IdentityController.getBeneficiariesByBenRegIds - start. benRegIdList = " + benRegIds);
BigInteger[] benRegIdarray = null;
JsonElement json = JsonParser.parseString(benRegIds);

if (json instanceof JsonNull) {
return getErrorResponseString("Null/Empty Phone Number.", 200, "success", "");
}

benRegIdarray = InputMapper.getInstance().gson().fromJson(json, BigInteger[].class);

List<BeneficiariesDTO> list = svc.getBeneficiariesDeatilsByBenRegIdList(Arrays.asList(benRegIdarray));
list.removeIf(Objects::isNull);
Collections.sort(list);
String response = getSuccessResponseString(list, 200, "success", "getBeneficiariesByBenRegIds");

logger.info("IdentityController.getBeneficiariesByBenRegIds - end : ");
return response;
@RequestBody Long[] benRegIds) { // Accepting an array of Longs directly
logger.info("IdentityController.getBeneficiariesByBenRegIds - start. benRegIdList = " + benRegIds.length);

Comment on lines +701 to +702
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security: Fix log injection vulnerability

The current logging implementation directly interpolates user input into the log message, which could be exploited for log injection attacks.

Apply this diff to sanitize the log message:

-logger.info("IdentityController.getBeneficiariesByBenRegIds - start. benRegIdList = " + benRegIds.length);
+logger.info("IdentityController.getBeneficiariesByBenRegIds - start. benRegIdList size = {}", benRegIds.length);
📝 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
logger.info("IdentityController.getBeneficiariesByBenRegIds - start. benRegIdList = " + benRegIds.length);
logger.info("IdentityController.getBeneficiariesByBenRegIds - start. benRegIdList size = {}", benRegIds.length);
🧰 Tools
🪛 GitHub Check: SonarCloud

[notice] 702-702: Logging should not be vulnerable to injection attacks

Change this code to not log user-controlled data.

See more on SonarQube Cloud

// If benRegIds is null or empty, return an error response
if (benRegIds == null || benRegIds.length == 0) {
return getErrorResponseString("No beneficiary registration IDs provided", 400, "error", "Array is empty");
}

// Convert the Long[] to BigInteger[] for further processing
BigInteger[] benRegIdArray = Arrays.stream(benRegIds)
.map(BigInteger::valueOf)
.toArray(BigInteger[]::new);

List<BeneficiariesDTO> list = svc.getBeneficiariesDeatilsByBenRegIdList(Arrays.asList(benRegIdArray));
list.removeIf(Objects::isNull);
Collections.sort(list);

String response = getSuccessResponseString(list, 200, "success", "getBeneficiariesByBenRegIds");

logger.info("IdentityController.getBeneficiariesByBenRegIds - end : ");
return response;
Comment on lines +713 to +720
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding error handling for service layer exceptions

The service call and response handling could benefit from more robust error handling.

Apply this diff to improve error handling:

-    List<BeneficiariesDTO> list = svc.getBeneficiariesDeatilsByBenRegIdList(Arrays.asList(benRegIdArray));
-    list.removeIf(Objects::isNull);
-    Collections.sort(list);
-
-    String response = getSuccessResponseString(list, 200, "success", "getBeneficiariesByBenRegIds");
-
-    logger.info("IdentityController.getBeneficiariesByBenRegIds - end : ");
-    return response;
+    try {
+        List<BeneficiariesDTO> list = svc.getBeneficiariesDeatilsByBenRegIdList(Arrays.asList(benRegIdArray));
+        if (list == null) {
+            return getErrorResponseString("No beneficiaries found", 404, "not_found", "getBeneficiariesByBenRegIds");
+        }
+        list.removeIf(Objects::isNull);
+        Collections.sort(list);
+
+        String response = getSuccessResponseString(list, 200, "success", "getBeneficiariesByBenRegIds");
+        logger.info("IdentityController.getBeneficiariesByBenRegIds - end");
+        return response;
+    } catch (Exception e) {
+        logger.error("Error retrieving beneficiaries: {}", e.getMessage(), e);
+        return getErrorResponseString("Error retrieving beneficiaries", 500, "error", "getBeneficiariesByBenRegIds");
+    }
📝 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
List<BeneficiariesDTO> list = svc.getBeneficiariesDeatilsByBenRegIdList(Arrays.asList(benRegIdArray));
list.removeIf(Objects::isNull);
Collections.sort(list);
String response = getSuccessResponseString(list, 200, "success", "getBeneficiariesByBenRegIds");
logger.info("IdentityController.getBeneficiariesByBenRegIds - end : ");
return response;
try {
List<BeneficiariesDTO> list = svc.getBeneficiariesDeatilsByBenRegIdList(Arrays.asList(benRegIdArray));
if (list == null) {
return getErrorResponseString("No beneficiaries found", 404, "not_found", "getBeneficiariesByBenRegIds");
}
list.removeIf(Objects::isNull);
Collections.sort(list);
String response = getSuccessResponseString(list, 200, "success", "getBeneficiariesByBenRegIds");
logger.info("IdentityController.getBeneficiariesByBenRegIds - end");
return response;
} catch (Exception e) {
logger.error("Error retrieving beneficiaries: {}", e.getMessage(), e);
return getErrorResponseString("Error retrieving beneficiaries", 500, "error", "getBeneficiariesByBenRegIds");
}

}


/**
* Overloaded method with string
*
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/com/iemr/common/identity/service/IdentityService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ public List<BeneficiariesDTO> searhBeneficiaryByGovIdentity(String identity)
}
return beneficiaryList;
}

private MBeneficiarymapping getBeneficiariesDTONew(Object[] benMapArr) {
MBeneficiarymapping benMapOBJ = new MBeneficiarymapping();
if (benMapArr != null && benMapArr.length == 12 && benMapArr[8] != null && benMapArr[9] != null) {
Expand All @@ -579,8 +579,11 @@ private MBeneficiarymapping getBeneficiariesDTONew(Object[] benMapArr) {
benMapOBJ.setCreatedDate((Timestamp) benMapArr[11]);
benMapOBJ = mappingRepo.getMapping(getBigIntegerValueFromObject(benMapArr[9]), (Integer) benMapArr[8]);


BigInteger benRegId = new BigInteger(benMapArr[5].toString());
RMNCHBeneficiaryDetailsRmnch obj = rMNCHBeneficiaryDetailsRmnchRepo
.getByRegID(((BigInteger) benMapArr[5]));
.getByRegID(benRegId);

if (obj != null) {
if (obj.getHouseoldId() != null)
benMapOBJ.setHouseHoldID(obj.getHouseoldId());
Expand Down
Loading