Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions src/main/java/com/iemr/common/data/users/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ public class User implements Serializable {
@Column(name = "failed_attempt")
private Integer failedAttempt;

@Expose
@Column(name = "lock_timestamp")
private Timestamp lockTimestamp;

@Expose
@Column(name = "dhistoken")
private String dhistoken;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,34 @@

private void checkUserAccountStatus(User user) throws IEMRException {
if (user.getDeleted()) {
throw new IEMRException("Your account is locked or de-activated. Please contact administrator");
// check if account was locked due to failed attempts and lock has expired
if (user.getLockTimestamp() != null) {
java.time.LocalDate lockDate = user.getLockTimestamp().toLocalDateTime().toLocalDate();
java.time.LocalDate today = java.time.LocalDate.now();

if (today.isAfter(lockDate)) {
// auto-unlock: it's a new day after the lock date
user.setDeleted(false);
user.setFailedAttempt(0);
user.setLockTimestamp(null);
iEMRUserRepositoryCustom.save(user);
logger.info("Account auto-unlocked for user {} (locked on {}, unlocked on {})",
user.getUserName(), lockDate, today);
return;
Comment on lines +230 to +238
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't auto-unlock past the inactive-status guard.

If an account is deactivated (statusID > 2) while it still has a historical lockTimestamp, this branch clears deleted and returns before Line 246 runs. The rest of the login flow then proceeds with an inactive account. Let the method continue into the status check after clearing the temporary lock, instead of returning here.

Suggested fix
 		if (user.getDeleted()) {
 			if (user.getLockTimestamp() != null) {
 				java.time.LocalDate lockDate = user.getLockTimestamp().toLocalDateTime().toLocalDate();
 				java.time.LocalDate today = java.time.LocalDate.now();

 				if (today.isAfter(lockDate)) {
 					user.setDeleted(false);
 					user.setFailedAttempt(0);
 					user.setLockTimestamp(null);
 					iEMRUserRepositoryCustom.save(user);
 					logger.info("Account auto-unlocked for user {} (locked on {}, unlocked on {})",
 							user.getUserName(), lockDate, today);
-					return;
 				} else {
 					throw new IEMRException(
 							"Your account has been locked. You can try tomorrow or connect to the administrator.");
 				}
 			}
-			throw new IEMRException(
-					"Your account is locked or de-activated. Please contact administrator");
-		} else if (user.getStatusID() > 2) {
+			if (user.getDeleted()) {
+				throw new IEMRException(
+						"Your account is locked or de-activated. Please contact administrator");
+			}
+		}
+		if (user.getStatusID() > 2) {
 			throw new IEMRException("Your account is not active. Please contact administrator");
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java`
around lines 230 - 238, The auto-unlock block in IEMRAdminUserServiceImpl
currently clears deleted/failedAttempt/lockTimestamp then returns, which
bypasses the subsequent inactive-account status check; remove the premature
return after the iEMRUserRepositoryCustom.save(user) and logger.info(...) so the
method continues into the existing status check logic (so deactivated accounts
with statusID > 2 are handled properly) while keeping the auto-unlock field
resets and save call intact.

} else {
throw new IEMRException(
"Your account has been locked. You can try tomorrow or connect to the administrator.");
}
}
throw new IEMRException(
"Your account is locked or de-activated. Please contact administrator");
} else if (user.getStatusID() > 2) {
throw new IEMRException("Your account is not active. Please contact administrator");
}
}

@Override
public List<User> userAuthenticate(String userName, String password) throws Exception {

Check failure on line 252 in src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=PSMRI_Common-API&issues=AZ2D_eKPgRBI7UuFHzjk&open=AZ2D_eKPgRBI7UuFHzjk&pullRequest=394
List<User> users = iEMRUserRepositoryCustom.findByUserNameNew(userName);
if (users.size() != 1) {
throw new IEMRException("Invalid username or password");
Expand Down Expand Up @@ -265,6 +285,10 @@
checkUserAccountStatus(user);
iEMRUserRepositoryCustom.save(user);
} else if (validatePassword == 0) {
// if already locked/deleted, don't overwrite lock timestamp
if (Boolean.TRUE.equals(user.getDeleted())) {
checkUserAccountStatus(user);
}
if (user.getFailedAttempt() + 1 < failedAttempt) {
user.setFailedAttempt(user.getFailedAttempt() + 1);
user = iEMRUserRepositoryCustom.save(user);
Expand All @@ -273,24 +297,26 @@
} else if (user.getFailedAttempt() + 1 >= failedAttempt) {
user.setFailedAttempt(user.getFailedAttempt() + 1);
user.setDeleted(true);
user.setLockTimestamp(new java.sql.Timestamp(System.currentTimeMillis()));
user = iEMRUserRepositoryCustom.save(user);
logger.warn("User Account has been locked after reaching the limit of {} failed login attempts.",
ConfigProperties.getInteger("failedLoginAttempt"));

throw new IEMRException(
"Invalid username or password. Please contact administrator.");
"Your account has been locked. You can try tomorrow or connect to the administrator.");
} else {
user.setFailedAttempt(user.getFailedAttempt() + 1);
user = iEMRUserRepositoryCustom.save(user);
logger.warn("Failed login attempt {} of {} for a user account.",
user.getFailedAttempt(), ConfigProperties.getInteger("failedLoginAttempt"));
throw new IEMRException(
"Invalid username or password. Please contact administrator.");
"Invalid username or password.");
}
} else {
checkUserAccountStatus(user);
if (user.getFailedAttempt() != 0) {
user.setFailedAttempt(0);
user.setLockTimestamp(null);
user = iEMRUserRepositoryCustom.save(user);
Comment on lines +319 to 320
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== SecurePassword.validatePassword contract =="
fd 'SecurePassword.java' src --exec sed -n '1,260p' {}

echo
echo "== validatePassword call sites in IEMRAdminUserServiceImpl =="
rg -n -C3 '\bvalidatePassword\s*\(' --type=java src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java

Repository: PSMRI/Common-API

Length of output: 6138


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Full code around first reset (lines 250-330) =="
sed -n '250,330p' src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java

echo
echo "== Full code around second reset (lines 340-420) =="
sed -n '340,420p' src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java

Repository: PSMRI/Common-API

Length of output: 6292


Reset the failure state in every successful-auth branch.

failedAttempt and lockTimestamp are only cleared in the final else block of each method. Since SecurePassword.validatePassword(...) returns 1, 2, or 3 for successful password matches—and the code handles each distinctly—users who log in successfully via any of these paths retain stale failure counts. A user with prior failures can authenticate successfully but still be re-locked on the next incorrect password attempt.

Move the reset into all three success branches (validatePassword == 1, 2, 3) in both userAuthenticate() and superUserAuthenticate().

This applies to both methods around the locations indicated.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java`
around lines 319 - 320, In userAuthenticate() and superUserAuthenticate(), the
successful-password branches (where SecurePassword.validatePassword(...) == 1,
== 2, or == 3) must clear the account failure state; currently failedAttempt and
lockTimestamp are only reset in the final else. Update each success branch to
set user.setFailedAttempt(0) and user.setLockTimestamp(null) and persist the
change via iEMRUserRepositoryCustom.save(user) (same persistence call used
elsewhere) so a previously-failed user who now authenticates successfully does
not retain stale failure counts that could re-lock them on a subsequent failure.

}
}
Expand All @@ -317,8 +343,8 @@
* Super Admin login
*/
@Override
public User superUserAuthenticate(String userName, String password) throws Exception {

Check failure on line 346 in src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=PSMRI_Common-API&issues=AZ2D_eKPgRBI7UuFHzjl&open=AZ2D_eKPgRBI7UuFHzjl&pullRequest=394
List<User> users = iEMRUserRepositoryCustom.findByUserName(userName);
List<User> users = iEMRUserRepositoryCustom.findByUserNameNew(userName);

if (users.size() != 1) {
throw new IEMRException("Invalid username or password");
Expand Down Expand Up @@ -351,6 +377,10 @@
iEMRUserRepositoryCustom.save(user);

} else if (validatePassword == 0) {
// if already locked/deleted, don't overwrite lock timestamp
if (Boolean.TRUE.equals(user.getDeleted())) {
checkUserAccountStatus(user);
}
if (user.getFailedAttempt() + 1 < failedAttempt) {
user.setFailedAttempt(user.getFailedAttempt() + 1);
user = iEMRUserRepositoryCustom.save(user);
Expand All @@ -359,24 +389,26 @@
} else if (user.getFailedAttempt() + 1 >= failedAttempt) {
user.setFailedAttempt(user.getFailedAttempt() + 1);
user.setDeleted(true);
user.setLockTimestamp(new java.sql.Timestamp(System.currentTimeMillis()));
user = iEMRUserRepositoryCustom.save(user);
logger.warn("User Account has been locked after reaching the limit of {} failed login attempts.",
ConfigProperties.getInteger("failedLoginAttempt"));

throw new IEMRException(
"Invalid username or password. Please contact administrator.");
"Your account has been locked. You can try tomorrow or connect to the administrator.");
} else {
user.setFailedAttempt(user.getFailedAttempt() + 1);
user = iEMRUserRepositoryCustom.save(user);
logger.warn("Failed login attempt {} of {} for a user account.",
user.getFailedAttempt(), ConfigProperties.getInteger("failedLoginAttempt"));
throw new IEMRException(
"Invalid username or password. Please contact administrator.");
"Invalid username or password.");
}
} else {
checkUserAccountStatus(user);
if (user.getFailedAttempt() != 0) {
user.setFailedAttempt(0);
user.setLockTimestamp(null);
user = iEMRUserRepositoryCustom.save(user);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Add lock_timestamp column to m_user table for time-bound account locking
-- When a user gets locked after 5 failed login attempts, this stores when
-- the lock happened. After 24 hours, the account auto-unlocks on next login.

ALTER TABLE m_user ADD COLUMN lock_timestamp DATETIME NULL DEFAULT NULL;