Add code to Implement account lockout after configured failed attempts#1194
Add code to Implement account lockout after configured failed attempts#1194Sae126V wants to merge 3 commits into
Conversation
- Improved test cases and code coverage - Added code to revoke suspension and an option for admin to configure whether to disable the users
|
Marking this to draft to improve the naming of env variables. |
- Fix code smell
Sae126V
force-pushed
the
issue-1133-iam-account-lockout
branch
from
75d5a6f to
36530c1
Compare
|
rmiccoli
left a comment
rmiccoli
left a comment
There was a problem hiding this comment.
Fix minor issues and resolve merge conflicts
|
|
||
| private final IamAccountLoginLockoutRepository lockoutRepo; | ||
| private final IamAccountRepository accountRepo; | ||
| private final LoginLockoutProperties properties; |
There was a problem hiding this comment.
I'd call them lockoutProperties
| isPreAuthenticated = extendedAuthenticationToken.isPreAuthenticated(); | ||
| } | ||
|
|
||
| String username = String.valueOf(authentication.getPrincipal()); |
There was a problem hiding this comment.
Maybe String username = authentication.getName());?
There was a problem hiding this comment.
Rename migration number to V122
| @Mock | ||
| private LoginLockoutService lockoutService; | ||
|
|
||
| private AccountLockoutController controller; |
There was a problem hiding this comment.
It's better lockoutController
| } | ||
|
|
||
| @Test | ||
| void disablesAccountAfterMaxSuspensionRounds() { |
There was a problem hiding this comment.
disableAccountAfterMaxSuspensionRounds
| LOG.info("Login blocked: account '{}' is suspended until {}", username, | ||
| lockout.getSuspendedUntil()); | ||
| throw new LockedException( | ||
| "Account is temporarily suspended. Please try again later or contact support for assistance."); |
There was a problem hiding this comment.
Would it be better to give a precise time, e.g. "Please try again in 30 minutes"?
| && Instant.now().isBefore(lockout.get().getSuspendedUntil().toInstant())) { | ||
| return ResponseEntity.ok(Map.of( | ||
| "suspended", true, | ||
| "suspendedUntil", lockout.get().getSuspendedUntil().getTime())); |
There was a problem hiding this comment.
Wouldn't it be better to return a date instead of a timestamp?
3 participants
Resolves #1133
Supersede's #1182