-
Notifications
You must be signed in to change notification settings - Fork 49
AMM-118: Add time-based account lockout with auto-unlock #332
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.
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
base: main
Are you sure you want to change the base?
AMM-118: Add time-based account lockout with auto-unlock #332
Conversation
📝 WalkthroughWalkthroughAdds account lock timestamp to User, implements configurable time-based auto-unlock and centralized authentication/lock logic, and exposes admin endpoints to query lock status and manually unlock user accounts. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin/Client
participant Controller as IEMRAdminController
participant Service as IEMRAdminUserServiceImpl
participant DB as User Repository/Database
Admin->>Controller: POST /unlockUserAccount { userId }
Controller->>Controller: parseUserIdFromRequest(), getAuthenticatedUserId()
Controller->>Service: hasAdminPrivileges(authUserId)
alt admin
Controller->>Service: unlockUserAccount(userId)
Service->>DB: find user by id
DB-->>Service: User
Service->>Service: clear lockTimestamp, reset attempts
Service->>DB: save(user)
Service-->>Controller: success
else not admin
Service-->>Controller: permission error
end
Controller-->>Admin: success/failure response
sequenceDiagram
participant Client as User/Client
participant AuthCtrl as Auth Controller
participant Service as IEMRAdminUserServiceImpl
participant DB as User Repository/Database
Client->>AuthCtrl: Login(credentials)
AuthCtrl->>Service: userAuthenticate(credentials)
Service->>DB: findByUserId(userId)
DB-->>Service: User
Service->>Service: checkUserAccountStatus()
alt Locked and within lock duration
Service-->>AuthCtrl: lockout message (remaining time)
else Locked but duration expired
Service->>Service: auto-unlock (clear lockTimestamp, reset attempts)
Service->>Service: validate password
alt Password valid
Service->>Service: reset failed attempts
Service-->>AuthCtrl: auth success
else Password invalid
Service->>Service: handle failed attempt (increment/lock)
Service-->>AuthCtrl: invalid/locked response
end
else Not locked
Service->>Service: validate password
alt Password valid
Service->>Service: reset failed attempts
Service-->>AuthCtrl: auth success
else Password invalid
Service->>Service: handle failed attempt (increment/lock)
Service-->>AuthCtrl: invalid/locked response
end
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly Related Issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java:
- Around line 1263-1294: In unlockUserAccount, remove the redundant
iEMRUserRepositoryCustom.findByUserID(userId) call and use the same single
lookup pattern as getUserLockStatus by calling
iEMRUserRepositoryCustom.findById(userId).orElse(null) to fetch the User once;
then adjust the subsequent null check and logic to operate on that single user
instance and delete the fallback branch so no duplicate DB query occurs.
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
1215-1237: Replace deprecated JsonParser constructor.The
new JsonParser()constructor is deprecated. UseJsonParser.parseString(request)instead for better compatibility with newer Gson versions.♻️ Recommended refactor
- JsonObject requestObj = new JsonParser().parse(request).getAsJsonObject(); + JsonObject requestObj = JsonParser.parseString(request).getAsJsonObject();
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
binsrc/main/java/com/iemr/common/controller/users/IEMRAdminController.javasrc/main/java/com/iemr/common/data/users/User.javasrc/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserService.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.javasrc/main/resources/application.properties
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-18T08:53:22.725Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
🔇 Additional comments (9)
src/main/resources/application.properties (1)
172-173: Configuration property looks good.The new property follows Spring Boot conventions (kebab-case), is logically grouped with authentication settings, uses a reasonable default (24 hours), and includes a clear explanatory comment. Well done.
bin (1)
1-1: Submodule pointer—unable to review functional changes.This file is a git submodule reference and contains no executable code to review. The PR description indicates that functional changes exist in files like
IEMRAdminController.javaandIEMRAdminUserServiceImpl.java, but those files were not provided for review.Please confirm whether the source code files containing the account lockout implementation should be included in this review. A review of the submodule pointer alone cannot validate the PR objectives (login flow modifications, auto-unlock logic, new endpoints, configuration, and error handling).
src/main/java/com/iemr/common/data/users/User.java (1)
211-213: LGTM! New lock timestamp field properly added.The
lockTimestampfield is correctly annotated with JPA and JSON serialization annotations, and Lombok's@Datawill automatically generate the necessary getters and setters for lock management functionality.src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java (1)
124-126: LGTM! Interface methods for account lock management.The new method signatures are well-defined and follow the existing code patterns in this interface. The return types and exception declarations are appropriate for their respective use cases.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (5)
132-133: LGTM! Configuration property for lock duration.The
accountLockDurationHoursconfiguration field is properly defined with a sensible default value of 24 hours, allowing flexibility for different deployment environments.
225-249: LGTM! Auto-unlock logic correctly implemented.The account status check properly implements auto-unlock behavior:
- Correctly calculates lock duration using the configured hours
- Resets all lock-related fields when unlocking
- Provides clear error messages for different lock states
- Includes appropriate logging for audit trails
288-303: LGTM! Improved login failure handling with null-safety.The updated failure handling properly:
- Uses null-safe checks for
failedAttemptwith a sensible default- Captures lock timestamp when the threshold is reached
- Leverages the helper method for consistent error messaging
- Maintains existing attempt tracking behavior
331-360: LGTM! Well-structured error message generation.The helper method correctly:
- Handles null timestamps with a fallback message
- Calculates remaining lockout time accurately
- Formats the duration in a user-friendly format
- Provides actionable messages for both active locks and expired locks
1296-1312: LGTM! Clean implementation of lock status retrieval.The method correctly fetches the user entity and handles the not-found case appropriately. The implementation is straightforward and follows good error handling practices.
9c6b5c8 to
ffdf91b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java:
- Around line 1268-1296: The unlock endpoint lacks an admin-only check: add
role-based authorization to the controller method that exposes unlockUserAccount
by annotating the endpoint method in IEMRAdminController with
@PreAuthorize("hasRole('ADMIN')") (or equivalent), ensure the necessary import
for PreAuthorize is added, and confirm method security is enabled (e.g.,
@EnableGlobalMethodSecurity(prePostEnabled = true) or the appropriate
configuration) so only users with the ADMIN role can call the controller method
that invokes IEMRAdminUserServiceImpl.unlockUserAccount.
🧹 Nitpick comments (2)
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (2)
132-133: Add validation for the account lock duration configuration.The
accountLockDurationHoursfield lacks validation. If configured with zero or negative values, it would cause incorrect lock duration calculations throughout the code.✅ Recommended validation approach
Add a
@PostConstructmethod to validate the configuration:@PostConstruct public void validateConfiguration() { if (accountLockDurationHours <= 0) { throw new IllegalStateException("account.lock.duration.hours must be a positive integer, got: " + accountLockDurationHours); } }
225-249: Consider extracting the lock duration calculation to reduce duplication.The calculation
(long) accountLockDurationHours * 60 * 60 * 1000appears in multiple methods (lines 230, 338, 1319). Extracting this to a helper method or constant would improve maintainability.♻️ Suggested refactor
Add a helper method at the class level:
private long getLockDurationMillis() { return (long) accountLockDurationHours * 60 * 60 * 1000; }Then use it consistently:
- long lockDurationMillis = (long) accountLockDurationHours * 60 * 60 * 1000; + long lockDurationMillis = getLockDurationMillis();
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
binsrc/main/java/com/iemr/common/controller/users/IEMRAdminController.javasrc/main/java/com/iemr/common/data/users/User.javasrc/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserService.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.javasrc/main/resources/application.properties
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
- bin
- src/main/java/com/iemr/common/data/users/User.java
- src/main/resources/application.properties
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-18T08:53:22.725Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
🔇 Additional comments (6)
src/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.java (1)
78-78: LGTM!This formatting change improves readability by adding visual separation between the method declarations. No functional impact.
src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java (1)
126-128: LGTM!The new method signatures are well-defined and follow the existing conventions in the interface. The return types and exception declarations are appropriate for admin account management operations.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (4)
288-303: LGTM!The null-safe handling of
failedAttemptprevents potential NPEs, and the lock logic correctly sets the timestamp when the threshold is reached. The use ofgenerateLockoutErrorMessageprovides clear feedback to users about when they can retry.
306-310: LGTM!The reset logic properly clears both the failed attempt counter and lock timestamp after successful authentication, ensuring a clean state.
331-360: LGTM!The helper method comprehensively handles lockout message generation with proper null checks, time calculations, and user-friendly formatting for various time remaining scenarios.
400-421: LGTM!The changes to
superUserAuthenticatemirror the updates inuserAuthenticate, maintaining consistency in lock handling across both authentication flows.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
Show resolved
Hide resolved
ffdf91b to
a2b1e2a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java:
- Around line 340-345: getFailedAttemptThreshold currently calls
Integer.parseInt(failedLoginAttempt) directly which can throw
NumberFormatException; wrap the parse in a try-catch inside
getFailedAttemptThreshold, catch NumberFormatException, log a warning (using the
class logger) that the configured failedLoginAttempt value is invalid, and
return the default 5 on error or when failedLoginAttempt is null/blank.
- Around line 255-281: handlePasswordValidationAndLocking currently treats
validatePassword() results 1,2,3 as successful logins but never calls
resetFailedAttemptsIfNeeded(), so prior failedAttempt counts remain; update the
method to call resetFailedAttemptsIfNeeded(user) in the successful branches
(cases 1,2,3) — ensure you still call checkUserAccountStatus(user) and
iEMRUserRepositoryCustom.save(user) after resetting, and for case 1 (old format)
keep generateUpgradedPassword(password) before save; leave
handleFailedLoginAttempt(...) for case 0 unchanged.
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (1)
285-294: Increase PBKDF2 iteration count significantly for better security.The iteration count of 1001 is far below current OWASP recommendations. For PBKDF2-HMAC-SHA512, OWASP recommends 210,000 iterations (or tune to achieve 50–150 ms computation time on production hardware). While this matches existing patterns in the codebase, consider planning a migration to stronger parameters.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.javasrc/main/java/com/iemr/common/data/users/User.javasrc/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserService.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.javasrc/main/resources/application.properties
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/resources/application.properties
- src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
- src/main/java/com/iemr/common/data/users/User.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-07T19:32:42.660Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
📚 Learning: 2024-12-18T08:53:22.725Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
🔇 Additional comments (8)
src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java (1)
126-128: LGTM!The new method declarations for
unlockUserAccountandgetUserLockStatusJsonare well-designed. They follow the existing interface patterns, use appropriate return types, and declare the standardIEMRExceptionfor error handling.src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (7)
132-133: LGTM!The configuration field with a sensible default of 24 hours is well implemented. Externalizing the lock duration allows for environment-specific tuning without code changes.
225-249: LGTM!The auto-unlock logic is well-implemented. The method correctly distinguishes between time-based locks (with
lockTimestamp) and admin deactivations (withoutlockTimestamp), providing appropriate messaging for each case. The explicit cast tolongon line 230 appropriately prevents potential integer overflow for larger lock durations.
296-313: LGTM!The failed login attempt handling logic correctly increments the counter and locks the account when the threshold is reached. The null check for
getFailedAttempt()prevents potential NPEs.
347-368: LGTM!The lockout messaging utilities are well-designed with appropriate handling for null timestamps, expired locks, and active locks. The remaining time calculation is correct.
1227-1255: LGTM!The
unlockUserAccountmethod correctly distinguishes between time-based account locks (unlockable via this endpoint) and admin deactivations (requiring user management). The error handling with specific messages for each scenario is helpful for administrators.
1257-1301: LGTM!The
getUserLockStatusJsonmethod provides comprehensive lock status information appropriate for administrative use. The distinction between time-based locks and other deactivations is maintained, and the calculated fields (remaining time, unlock time) are useful for support operations.
1303-1309: LGTM!The time formatting utility handles the common cases well. For edge cases with less than a minute remaining, "0 minutes" is displayed, which is acceptable for user-facing messages.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
Show resolved
Hide resolved
a2b1e2a to
3395eb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java:
- Around line 225-249: checkUserAccountStatus currently calls user.getDeleted()
without null-safe handling which can NPE; update the boolean checks to mirror
unlockUserAccount by guarding against null (e.g., use user.getDeleted() != null
&& user.getDeleted()) when testing deleted state and adjust logic paths
accordingly (including the branch that resets deleted to
false/failedAttempt/lockTimestamp and the else throws) so all uses of
getDeleted() are null-safe; ensure you update any related conditional like the
initial if(user.getDeleted()) and any subsequent flows that assume non-null
Deleted.
🧹 Nitpick comments (2)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (2)
1233-1262: Consider adding request/response logging for consistency.Other endpoints in this controller log both request and response at INFO level. These new endpoints only log errors.
Suggested enhancement
@Operation(summary = "Unlock user account locked due to failed login attempts") @RequestMapping(value = "/unlockUserAccount", method = RequestMethod.POST, produces = MediaType.APPLICATION_JSON, headers = "Authorization") public String unlockUserAccount(@RequestBody String request) { OutputResponse response = new OutputResponse(); + logger.info("unlockUserAccount request: " + request); try { Long userId = parseUserIdFromRequest(request); boolean unlocked = iemrAdminUserServiceImpl.unlockUserAccount(userId); response.setResponse(unlocked ? "User account successfully unlocked" : "User account was not locked"); } catch (Exception e) { logger.error("Error unlocking user account: " + e.getMessage(), e); response.setError(e); } + logger.info("unlockUserAccount response: " + response.toString()); return response.toString(); } @Operation(summary = "Get user account lock status") @RequestMapping(value = "/getUserLockStatus", method = RequestMethod.POST, produces = MediaType.APPLICATION_JSON, headers = "Authorization") public String getUserLockStatus(@RequestBody String request) { OutputResponse response = new OutputResponse(); + logger.info("getUserLockStatus request: " + request); try { Long userId = parseUserIdFromRequest(request); String lockStatusJson = iemrAdminUserServiceImpl.getUserLockStatusJson(userId); response.setResponse(lockStatusJson); } catch (Exception e) { logger.error("Error getting user lock status: " + e.getMessage(), e); response.setError(e); } + logger.info("getUserLockStatus response: " + response.toString()); return response.toString(); }
1264-1270: Consider handling malformeduserIdvalues.If
userIdis provided as a non-numeric string (e.g.,"userId": "abc"),getAsLong()will throw aNumberFormatExceptionwith a less user-friendly message. Consider adding explicit validation.Suggested enhancement
private Long parseUserIdFromRequest(String request) throws IEMRException { - JsonObject requestObj = JsonParser.parseString(request).getAsJsonObject(); - if (!requestObj.has("userId") || requestObj.get("userId").isJsonNull()) { - throw new IEMRException("userId is required"); + try { + JsonObject requestObj = JsonParser.parseString(request).getAsJsonObject(); + if (!requestObj.has("userId") || requestObj.get("userId").isJsonNull()) { + throw new IEMRException("userId is required"); + } + return requestObj.get("userId").getAsLong(); + } catch (NumberFormatException e) { + throw new IEMRException("Invalid userId format"); + } catch (IllegalStateException | com.google.gson.JsonSyntaxException e) { + throw new IEMRException("Invalid request format"); } - return requestObj.get("userId").getAsLong(); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.javasrc/main/java/com/iemr/common/data/users/User.javasrc/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserService.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.javasrc/main/resources/application.properties
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/resources/application.properties
- src/main/java/com/iemr/common/data/users/User.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-07T19:32:42.660Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
📚 Learning: 2024-12-18T08:53:22.725Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
🧬 Code graph analysis (1)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (3)
src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java (1)
RequestMapping(47-161)src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java (1)
RequestMapping(18-48)src/main/java/com/iemr/common/controller/email/EmailController.java (1)
RequestMapping(43-105)
🔇 Additional comments (8)
src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java (1)
126-128: LGTM!The new interface method declarations for
unlockUserAccountandgetUserLockStatusJsonare well-defined and consistent with the existing interface patterns.src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (7)
132-133: LGTM!The configuration field with a sensible default of 24 hours is properly wired and follows Spring conventions.
251-285: LGTM!The refactored password validation and locking flow is well-structured with clear separation of concerns. The switch-case handles all validation outcomes appropriately.
287-331: LGTM!The helper methods are well-implemented with proper null handling in
handleFailedLoginAttempt. The differentiation between 1001 iterations for upgraded passwords vs 1000 for existing passwords is noted as intentional.
333-400: LGTM!Good refactoring to extract common authentication logic into
handlePasswordValidationAndLockingandgetFailedAttemptThreshold, reducing code duplication betweenuserAuthenticateandsuperUserAuthenticate.
361-381: LGTM!The lockout error message generation with remaining time provides clear user feedback. The handling of null timestamps and expired locks is appropriate.
1240-1268: LGTM!The manual unlock implementation correctly differentiates between time-based locks (from failed login attempts) and admin deactivations, providing appropriate responses for each case.
1270-1321: LGTM!The lock status JSON provides comprehensive information for administrators. The
formatRemainingTimehelper handles most cases well.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
Show resolved
Hide resolved
848f506 to
29a1bb0
Compare
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/common/controller/users/IEMRAdminController.java:
- Around line 1276-1301: getAuthenticatedUserId currently only reads the JWT
from cookies so header-supplied tokens are ignored; update
getAuthenticatedUserId(HttpServletRequest) to first check
HttpServletRequest.getHeader("Jwttoken") and use that token if present
(non-null/non-empty), falling back to the existing cookie iteration for
"Jwttoken" otherwise, then validate via jwtUtil.getUserIdFromToken and parse
with Long.parseLong as before, preserving the same exception handling paths
(IEMRException on missing/invalid token and NumberFormatException handling).
- Around line 1268-1274: The parseUserIdFromRequest method can throw uncaught
JsonSyntaxException or NumberFormatException for malformed JSON or non-numeric
userId; wrap the parsing and getAsLong calls in a try/catch that catches
JsonSyntaxException, IllegalStateException, and NumberFormatException (or
Exception) and rethrow a user-friendly IEMRException (e.g., "Invalid request
body" or "userId must be a number") including the original exception as the
cause; also validate that requestObj.has("userId") &&
requestObj.get("userId").isJsonPrimitive() &&
requestObj.get("userId").getAsJsonPrimitive().isNumber() before calling
getAsLong to provide a clearer error path in parseUserIdFromRequest.
In @src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java:
- Around line 225-249: checkUserAccountStatus may NPE on user.getDeleted(); make
the deleted check null-safe by using a Boolean-safe comparison (e.g.,
Boolean.TRUE.equals(user.getDeleted()) or explicit null check) before treating
it as a boolean; update the conditional in checkUserAccountStatus to handle null
deleted values consistently with unlockUserAccount and preserve existing
behavior (auto-unlock, exception paths) when deleted is true, false, or null.
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (1)
1315-1321: Edge case: Very short remaining times display as "0 minutes".When
remainingMillisis less than 60 seconds (e.g., 30 seconds), this method returns "0 minutes", which could confuse users who still cannot log in. Consider showing seconds for sub-minute durations or displaying "less than 1 minute".♻️ Suggested improvement
private String formatRemainingTime(long remainingMillis) { long hours = remainingMillis / (60 * 60 * 1000); long minutes = (remainingMillis % (60 * 60 * 1000)) / (60 * 1000); if (hours > 0 && minutes > 0) return String.format("%d hours %d minutes", hours, minutes); if (hours > 0) return String.format("%d hours", hours); + if (minutes <= 0) return "less than 1 minute"; return String.format("%d minutes", minutes); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.javasrc/main/java/com/iemr/common/data/users/User.javasrc/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserService.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.javasrc/main/resources/application.properties
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/resources/application.properties
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-07T19:32:42.660Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
📚 Learning: 2024-12-18T08:53:22.725Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
🧬 Code graph analysis (1)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (3)
src/main/java/com/iemr/common/controller/covid/CovidVaccinationController.java (1)
RequestMapping(47-161)src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java (1)
RequestMapping(18-48)src/main/java/com/iemr/common/controller/email/EmailController.java (1)
RequestMapping(43-105)
🔇 Additional comments (12)
src/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.java (1)
78-78: No concerns with this formatting change.This is a trivial whitespace addition (blank line) between method declarations with no functional impact.
src/main/java/com/iemr/common/data/users/User.java (1)
216-218: LGTM!The
lockTimestampfield is appropriately added with correct annotations (@Exposefor JSON serialization,@Columnfor JPA mapping). The naming and type are consistent with other timestamp fields in the entity.src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java (1)
126-130: LGTM!The new method signatures are well-defined with appropriate return types and consistent exception handling. They complement the existing service contract for the account lockout feature.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (6)
132-133: LGTM!Good use of externalized configuration with a sensible default value. The
@Valueannotation correctly provides fallback to 24 hours if the property is not configured.
251-285: Good centralization of authentication and locking logic.The helper method cleanly consolidates password validation and account locking logic used by both
userAuthenticate()andsuperUserAuthenticate(). The switch cases handle all validation outcomes appropriately.Minor observation: Cases 2/3 and the default case both call
clearFailedAttemptState/resetFailedAttemptsIfNeededfollowed by save, which is slightly redundant but ensures consistency.
307-324: LGTM!The failed login handling correctly increments the counter, locks the account at threshold, and records the lock timestamp for accurate remaining time calculations.
1240-1268: LGTM!The unlock logic correctly distinguishes between accounts locked due to failed attempts (unlockable) versus accounts deactivated by administrators (requires user management). Proper null-safety checks and logging are in place.
1270-1313: LGTM!The lock status JSON response provides comprehensive information for administrators including failed attempt count, lock state, and remaining time. The distinction between login-lockout and admin-deactivation states is clearly communicated.
1323-1344: Loose role name matching could allow unintended roles to be flagged as admin.The
contains()check for "admin", "supervisor", and "provideradmin" relies on substring matching. While the three keywords are reasonably specific and no problematic role names are evident in the codebase, this pattern is inherently fragile. Consider either adding test cases for edge case role names (e.g., "Administrator", "NonAdminReviewer") to ensure they behave as expected, or refactoring to use exact matches or an enum-based approach for more maintainability.src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (3)
1234-1249: LGTM!The endpoint correctly implements authentication and authorization checks before allowing the admin unlock operation. Error handling and logging are properly implemented.
1251-1266: LGTM!The lock status endpoint follows the same secure pattern as the unlock endpoint with proper authentication and authorization checks.
1303-1308: LGTM!The authorization helper is clean and includes appropriate audit logging for unauthorized access attempts.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
Show resolved
Hide resolved
29a1bb0 to
b657431
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/common/controller/users/IEMRAdminController.java:
- Around line 1286-1315: In getAuthenticatedUserId, call
jwtUtil.validateToken(jwtToken) before jwtUtil.getUserIdFromToken(jwtToken)
(same pattern used in refreshToken) and throw an IEMRException if validateToken
returns false or throws, so denylisted/revoked tokens are rejected; then proceed
to call getUserIdFromToken and parse the userId as before, preserving existing
NumberFormatException and generic exception handling.
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (1)
255-285: Minor inefficiency: redundant database save operations.Cases 2 and 3 (lines 271-277) always call
iEMRUserRepositoryCustom.save(user)even when no state has changed (failed attempts were already 0 and lock timestamp was already null). Consider checking whether a save is actually needed:case 2: case 3: // Valid password checkUserAccountStatus(user); - clearFailedAttemptState(user); - iEMRUserRepositoryCustom.save(user); + if (user.getFailedAttempt() != null && user.getFailedAttempt() != 0) { + clearFailedAttemptState(user); + iEMRUserRepositoryCustom.save(user); + } break;This is similar to what
resetFailedAttemptsIfNeededalready does in the default case.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.javasrc/main/java/com/iemr/common/data/users/User.javasrc/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserService.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.javasrc/main/resources/application.properties
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/iemr/common/data/users/User.java
- src/main/resources/application.properties
- src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-12-07T19:32:42.660Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
📚 Learning: 2024-12-18T08:53:22.725Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
📚 Learning: 2025-07-11T18:00:56.920Z
Learnt from: Zapper9982
Repo: PSMRI/Common-API PR: 258
File: src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java:704-704
Timestamp: 2025-07-11T18:00:56.920Z
Learning: In the IEMRAdminController (and likely other controllers in this codebase), error handling is incomplete - NullPointerExceptions are being allowed to propagate to clients instead of being caught and converted to proper error responses. Tests expect error messages like "Cannot invoke \"org.json.JSONObject.toString()\" because \"responseObj\" is null", indicating a systemic issue with input validation and null handling in controller methods.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
📚 Learning: 2025-02-03T12:42:38.278Z
Learnt from: indraniBan
Repo: PSMRI/Common-API PR: 138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
📚 Learning: 2025-02-21T07:43:03.828Z
Learnt from: sandipkarmakar3
Repo: PSMRI/Common-API PR: 162
File: src/main/java/com/iemr/common/utils/CookieUtil.java:52-66
Timestamp: 2025-02-21T07:43:03.828Z
Learning: In the Common-API project's CookieUtil class, the current implementation of addJwtTokenToCookie using both response.addCookie() and manual Set-Cookie header has been tested and confirmed to work as required for their use case. URL encoding of the JWT token is not necessary.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
🔇 Additional comments (12)
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (8)
132-133: LGTM!Good use of configuration-driven lock duration with a sensible default value. This allows flexibility without code changes.
225-249: LGTM!The auto-unlock logic correctly distinguishes between time-based lockouts (with
lockTimestamp) and admin-initiated deactivations (withoutlockTimestamp). The duration check and state reset are properly implemented.
292-294: LGTM!The cast to
longis correctly placed at the beginning of the expression, ensuring the entire multiplication is performed in long arithmetic to avoid overflow.
307-324: LGTM!The failed login attempt handling correctly locks the account on the Nth failed attempt (where N equals the threshold) and properly sets the lock timestamp for time-based auto-unlock.
1315-1321: LGTM!The time formatting logic correctly handles all cases. Callers already guard against negative values.
1240-1268: LGTM!Good implementation that correctly distinguishes between time-based lockouts (unlockable via this endpoint) and admin-initiated deactivations (requiring user management). The error messaging is clear and appropriate.
1270-1313: LGTM!The lock status endpoint provides comprehensive information for administrators to diagnose account states. The response correctly distinguishes between different lock scenarios and provides actionable information like remaining time and expected unlock time.
1323-1344: Verify admin role names in the m_role database table match the expected patterns.The
hasAdminPrivilegesmethod relies on substring matching for role names containing "admin", "supervisor", or "provideradmin". Since role names are populated in the external m_role database table (not defined in this codebase), confirm that actual admin roles follow these naming conventions. If they don't, the privilege check may fail to identify legitimate admins or incorrectly identify non-admin roles.src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (4)
1234-1249: LGTM!The endpoint properly enforces authentication and admin authorization before allowing account unlock operations. The separation of concerns using helper methods improves maintainability.
1251-1266: LGTM!Consistent implementation with the unlock endpoint. Both endpoints share the same authentication and authorization pattern.
1268-1284: LGTM!Robust input validation that checks for presence, type, and format of the userId parameter. The exception handling properly wraps parsing errors.
1317-1322: LGTM!Good security practice to log unauthorized access attempts for audit purposes. The error message is clear without revealing internal details.
710f70f to
2984d5e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java`:
- Around line 1328-1335: The authorization loop in IEMRAdminUserServiceImpl uses
role.getRoleName().toLowerCase().contains(...), which can falsely match
unintended names; replace the contains checks with exact matching (e.g., use
role.getRoleName().trim().equalsIgnoreCase("Admin"),
equalsIgnoreCase("Supervisor"), equalsIgnoreCase("ProviderAdmin") or compare
against a Set of allowed role names) so only exact role names grant privileges;
update the logic in the loop that iterates UserServiceRoleMapping -> Role ->
getRoleName accordingly.
🧹 Nitpick comments (2)
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (2)
276-281: Default case appears to be unreachable dead code.Based on the
securePassword.validatePassword()implementation (which returns 0, 1, 2, or 3), this default case will never execute. Consider removing it or converting it to throw an exception for unexpected return values:♻️ Proposed fix
default: - // Successful validation - reset failed attempts if needed - checkUserAccountStatus(user); - resetFailedAttemptsIfNeeded(user); - break; + // Unexpected return value - should never happen + throw new IEMRException("Unexpected password validation result: " + validatePassword);
1313-1319: Edge case: remaining time less than 1 minute shows "0 minutes".When the lock is about to expire (less than 1 minute remaining), this will display "0 minutes" which may confuse users. Consider handling seconds or displaying a more informative message:
♻️ Proposed improvement
private String formatRemainingTime(long remainingMillis) { long hours = remainingMillis / (60 * 60 * 1000); long minutes = (remainingMillis % (60 * 60 * 1000)) / (60 * 1000); if (hours > 0 && minutes > 0) return String.format("%d hours %d minutes", hours, minutes); if (hours > 0) return String.format("%d hours", hours); + if (minutes <= 0) return "less than a minute"; return String.format("%d minutes", minutes); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.javasrc/main/java/com/iemr/common/data/users/User.javasrc/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserService.javasrc/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.javasrc/main/resources/application.properties
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/iemr/common/repository/users/IEMRUserRepositoryCustom.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/iemr/common/data/users/User.java
- src/main/resources/application.properties
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-12-07T19:32:42.660Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 123
File: src/main/java/com/iemr/common/controller/abdmfacility/AbdmFacilityController.java:31-31
Timestamp: 2024-12-07T19:32:42.660Z
Learning: In this codebase, for Java Spring Boot API controllers like `AbdmFacilityController.java`, it's a common pattern to include the `Authorization` parameter in method signatures, even if it's not used in the implementation, to maintain consistency across APIs.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
📚 Learning: 2024-12-18T08:53:22.725Z
Learnt from: helenKaryamsetty
Repo: PSMRI/Common-API PR: 145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
Applied to files:
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
📚 Learning: 2025-07-11T18:00:56.920Z
Learnt from: Zapper9982
Repo: PSMRI/Common-API PR: 258
File: src/test/java/com/iemr/common/controller/users/IEMRAdminControllerTest.java:704-704
Timestamp: 2025-07-11T18:00:56.920Z
Learning: In the IEMRAdminController (and likely other controllers in this codebase), error handling is incomplete - NullPointerExceptions are being allowed to propagate to clients instead of being caught and converted to proper error responses. Tests expect error messages like "Cannot invoke \"org.json.JSONObject.toString()\" because \"responseObj\" is null", indicating a systemic issue with input validation and null handling in controller methods.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
📚 Learning: 2025-02-03T12:42:38.278Z
Learnt from: indraniBan
Repo: PSMRI/Common-API PR: 138
File: src/main/java/com/iemr/common/utils/JwtAuthenticationUtil.java:0-0
Timestamp: 2025-02-03T12:42:38.278Z
Learning: In the JwtAuthenticationUtil class, catching a generic Exception in the validateUserIdAndJwtToken method is acceptable as per the team's preference.
Applied to files:
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java
🔇 Additional comments (10)
src/main/java/com/iemr/common/service/users/IEMRAdminUserService.java (1)
126-131: LGTM!The new method declarations follow the existing interface patterns and have appropriate return types and exception handling. The interface provides a clean contract for account lock management operations.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (4)
132-133: LGTM!Configuration-driven lock duration with a sensible default of 24 hours is a good approach for flexibility.
225-248: LGTM!The auto-unlock logic is well-implemented with proper null-safety checks and clear logging. The time-based unlock calculation correctly uses the configured lock duration.
1238-1266: LGTM!The unlock logic correctly distinguishes between accounts locked due to failed attempts versus those deactivated by an administrator, providing appropriate handling for each scenario. The audit logging for admin unlock actions is a good practice.
1268-1311: LGTM!The lock status JSON provides comprehensive information including lock state, timestamps, and calculated remaining time. The conditional logic properly handles the different scenarios (locked due to failed attempts vs. admin deactivation vs. not locked).
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (5)
80-80: LGTM!Good practice to use a constant for the field name to avoid magic strings.
587-592: LGTM!Good addition of JWT token validation before extracting the user ID, ensuring that expired or denylisted tokens are properly rejected.
1242-1274: LGTM!Both endpoints follow a secure and consistent pattern: authenticate the caller, validate admin privileges, then perform the operation. The error handling and logging are appropriate.
1276-1293: LGTM!Comprehensive input validation with proper handling of malformed JSON, missing fields, and type validation. The error messages are user-friendly without leaking implementation details.
1295-1323: LGTM!The authentication helper correctly uses the session-based approach consistent with the codebase patterns. The admin privilege validation provides clear access control with appropriate logging of unauthorized access attempts.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java
Show resolved
Hide resolved
Signed-off-by: Varun Deep Saini <varun.23bcs10048@ms.sst.scaler.com>
2984d5e to
e57bce4
Compare
|



Part of PSMRI/AMRIT#118
📋 Description
Implements 24-hour time-based auto-unlock for user accounts locked due to failed login attempts. Previously, locked accounts required manual admin intervention. Now accounts automatically unlock after 24 hours, with clear error messages showing remaining lockout time. Also adds admin endpoints to manually unlock accounts and check lock status.
✅ Type of Change
ℹ️ Additional Information
Changes:
lock_timestampfield to User entity/unlockUserAccountendpoint for manual admin unlock/getUserLockStatusendpoint to check account statusaccount.lock.duration.hoursconfig property (default 24)Testing:
Summary by CodeRabbit
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.