Skip to content

Add Security Check to Prevent User Enumeration #242

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

Merged
vishwab1 merged 1 commit into from
Jul 1, 2025
Merged

Add Security Check to Prevent User Enumeration #242

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 10 additions & 10 deletions src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ public String superUserAuthenticate(
response.setResponse(responseObj.toString());
} catch (Exception e) {
logger.error("userAuthenticate failed with error " + e.getMessage(), e);
response.setError(5000, "Authentication failed. Please try again later."); // Generic fallback
response.setError(e);
}
logger.info("userAuthenticate response " + response.toString());
return response.toString();
Expand Down Expand Up @@ -620,10 +620,10 @@ public String forgetPassword(

if (mUsers == null || mUsers.size() <= 0) {
logger.error("User not found");
throw new IEMRException("Request failed, please try again later");
throw new IEMRException("If the username is valid, you will be asked a security question");
} else if (mUsers.size() > 1) {
logger.error("More than 1 user found");
throw new IEMRException("Request failed. Please retry again");
throw new IEMRException("If the username is valid, you will be asked a security question");

} else if (mUsers.size() == 1) {
List<Map<String, String>> quesAnsList = new ArrayList<>();
Expand All @@ -645,7 +645,7 @@ public String forgetPassword(
}
} catch (Exception e) {
logger.error("forgetPassword failed with error " + e.getMessage(), e);
response.setError(5000, "ForgetPassword failed.");
response.setError(e);
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security inconsistency: Generic error exposure undermines user enumeration prevention.

While the specific error cases above use proper generic messages, this catch-all exception handler now exposes detailed error information, which could still aid user enumeration attacks or expose sensitive system details.

Consider using a generic error message:

-			response.setError(e);
+			response.setError(5000, "Unable to process request. Please try again later.");
🤖 Prompt for AI Agents 
In src/main/java/com/iemr/common/controller/users/IEMRAdminController.java at
line 648, the catch-all exception handler currently exposes detailed error
information via response.setError(e), which risks user enumeration and leaks
sensitive details. Replace this with a generic error message that does not
reveal specifics, ensuring consistent use of generic messages for all error
cases to enhance security.

}
logger.info("forgetPassword response " + response.toString());
return response.toString();
Expand Down Expand Up @@ -734,7 +734,7 @@ public String changePassword(
response.setResponse(changeReqResult);
} catch (Exception e) {
logger.error("changePassword failed with error " + e.getMessage(), e);
response.setError(5000, "Password change failed. Please try again later.");
response.setError(e);
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security concern: Password operations should use generic error messages.

Exposing detailed exception information in password change operations could reveal sensitive system details or validation logic to attackers.

Consider using a generic error message:

-			response.setError(e);
+			response.setError(5000, "Unable to change password. Please try again later.");
🤖 Prompt for AI Agents 
In src/main/java/com/iemr/common/controller/users/IEMRAdminController.java at
line 737, replace the detailed exception being set in response.setError(e) with
a generic error message string to avoid exposing sensitive system details during
password operations. This involves catching the exception but setting a
non-specific error message like "An error occurred while changing the password"
instead of the exception object.

}
logger.info("changePassword response " + response.toString());
return response.toString();
Expand All @@ -752,7 +752,7 @@ public String saveUserSecurityQuesAns(
response.setResponse(responseData);
} catch (Exception e) {
logger.error("saveUserSecurityQuesAns failed with error " + e.getMessage(), e);
response.setError(5000, "Failed to save security questions. Please try again later.");
response.setError(e);
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security concern: Security question operations should use generic error messages.

Exposing detailed exception information in security question operations could reveal sensitive validation logic or system details.

Consider using a generic error message:

-			response.setError(e);
+			response.setError(5000, "Unable to save security questions. Please try again later.");
📝 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
response.setError(e);
response.setError(5000, "Unable to save security questions. Please try again later.");
🤖 Prompt for AI Agents 
In src/main/java/com/iemr/common/controller/users/IEMRAdminController.java at
line 755, replace the detailed exception being set in response.setError(e) with
a generic error message string to avoid exposing sensitive system or validation
details. This means instead of passing the exception object, set a fixed,
non-specific error message like "An error occurred while processing your
request" to enhance security.

}
logger.info("saveUserSecurityQuesAns response " + response.toString());
return response.toString();
Expand Down Expand Up @@ -1058,7 +1058,7 @@ public String userAuthenticateByEncryption(
response.setResponse(responseObj.toString());
} catch (Exception e) {
logger.error("userAuthenticateByEncryption failed with error " + e.getMessage(), e);
response.setError(5000, "Request failed. Please try again.");
response.setError(e);
}
logger.info("userAuthenticateByEncryption response " + response.toString());
return response.toString();
Expand All @@ -1076,7 +1076,7 @@ public String getrolewrapuptime(@PathVariable("roleID") Integer roleID) {
}
response.setResponse(test.toString());
} catch (Exception e) {
response.setError(5000, "Request failed. Please try again.");
response.setError(e);
}
return response.toString();
}
Expand Down Expand Up @@ -1104,7 +1104,7 @@ public String validateSecurityQuestionAndAnswer(
throw new IEMRException("Invalid Request");
} catch (Exception e) {
logger.error("validateSecurityQuestionAndAnswer failed: {}", e.toString());
response.setError(5000, "Request failed. Please try again.");
response.setError(5000, e.getMessage());
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security concern: Exposing exception messages in security validation.

While this retains the error code, exposing exception messages in security question validation could still reveal sensitive validation logic.

Consider using a completely generic message:

-			response.setError(5000, e.getMessage());
+			response.setError(5000, "Security validation failed. Please try again.");
📝 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
response.setError(5000, e.getMessage());
response.setError(5000, "Security validation failed. Please try again.");
🤖 Prompt for AI Agents 
In src/main/java/com/iemr/common/controller/users/IEMRAdminController.java at
line 1107, avoid exposing the exception message in the response to prevent
leaking sensitive validation details. Replace e.getMessage() with a generic
error message string that does not reveal internal logic, while keeping the
error code 5000 unchanged.

}
logger.info("validateSecurityQuestionAndAnswer API response" + response.toString());
return response.toString();
Expand Down Expand Up @@ -1160,7 +1160,7 @@ public String userAuthenticateBhavya(
response.setResponse(responseObj.toString());
} catch (Exception e) {
logger.error("userAuthenticate failed with error " + e.getMessage(), e);
response.setError(5000, "Authentication failed. Please try again.");
response.setError(e);
}
logger.info("userAuthenticate response " + response.toString());
return response.toString();
Expand Down
Loading