TASK-6013 - Extend User Management capabilities #2432
Conversation
| // We can only lock the account if it is not the root user | ||
| int failedAttempts = userOpenCGAResult.first().getInternal().getFailedAttempts(); | ||
| ObjectMap updateParams = new ObjectMap(UserDBAdaptor.QueryParams.INTERNAL_FAILED_ATTEMPTS.key(), failedAttempts + 1); | ||
| if (failedAttempts >= 4) { |
There was a problem hiding this comment.
The number of maxFailedAttemps should be configurable
There was a problem hiding this comment.
We are not aware of any software that let's you decide the maximum number of login attempts before blocking an account, so that is going to be the number of times a user is allowed to make a login attempt before blocking the account. Obviously, we will review this decision depending on how users react.
There was a problem hiding this comment.
Should be configurable so it can be disabled 🤷♂️
There was a problem hiding this comment.
Also, check:
pwdMaxFailure
https://www.openldap.org/doc/admin24/guide.html
There was a problem hiding this comment.
Or...
deny– used to define the number of attempts (3 in this case), after which the user account should be locked.
| @@ -768,6 +801,10 @@ public AuthenticationResponse login(String organizationId, String username, Stri | |||
| throw CatalogAuthenticationException.incorrectUserOrPassword(); | |||
| } | |||
|
|
|||
| // Reset login failed attempts counter | |||
| ObjectMap updateParams = new ObjectMap(UserDBAdaptor.QueryParams.INTERNAL_FAILED_ATTEMPTS.key(), 0); | |||
There was a problem hiding this comment.
Worth checking if the current value is already 0 or not? Worth skipping a mongo update on each login when non needed, which might be the general scenario
| @@ -130,7 +130,14 @@ public OpenCGAResult<User> create(User user, String password, String token) thro | |||
| } | |||
| user.setAccount(ParamUtils.defaultObject(user.getAccount(), Account::new)); | |||
| user.getAccount().setCreationDate(TimeUtils.getTime()); | |||
| user.getAccount().setExpirationDate(ParamUtils.defaultString(user.getAccount().getExpirationDate(), "")); | |||
| if (StringUtils.isEmpty(user.getAccount().getExpirationDate())) { | |||
| // By default, user accounts will be valid for 1 year when they are created. | |||
There was a problem hiding this comment.
This is harsh...
I'd make this configurable.
Also, what if the user is "sync" from LDAP? Does it expire? Do we want these to expire?
There was a problem hiding this comment.
These things will only apply to local users. It will not apply to existing LDAP, AzureAD or SSO users.
opencga-app/src/main/java/org/opencb/opencga/app/migrations/v3_1_0/UserBanMigration.java
Outdated
Show resolved
Hide resolved
opencga-catalog/src/main/java/org/opencb/opencga/catalog/managers/UserManager.java
Show resolved
Hide resolved
https://app.clickup.com/t/36631768/TASK-6013