Conversation
airajena
force-pushed
the
FINERACT-2004/limit-login-retries
branch
4 times, most recently
from
b44c9df to
2ba7017
Compare
|
Hi @adamsaghy , i checked the failed check |
airajena
force-pushed
the
FINERACT-2004/limit-login-retries
branch
2 times, most recently
from
9474e14 to
5ac562f
Compare
|
@airajena Please rebase this PR with latest |
|
|
||
| @Getter | ||
| @Column(name = "failed_login_attempts", nullable = false) | ||
| private int failedLoginAttempts; |
There was a problem hiding this comment.
Please make sure to introduce a new flag which controls whether this functionality is allowed on a particular account or not.
For example, the system account should NEVER be locked! Probably same applies to accounts which are used for API communication!
adamsaghy
left a comment
adamsaghy
left a comment
There was a problem hiding this comment.
Kindly see my concerns!
airajena
force-pushed
the
FINERACT-2004/limit-login-retries
branch
2 times, most recently
from
baefdaa to
226afac
Compare
|
@airajena Please rebase. |
airajena
force-pushed
the
FINERACT-2004/limit-login-retries
branch
from
226afac to
428c924
Compare
done, resolved the merge conflicts |
| <column name="failed_login_attempts" type="INT" defaultValueNumeric="0"> | ||
| <constraints nullable="false"/> | ||
| </column> | ||
| <column name="is_login_retries_enabled" type="BOOLEAN" defaultValueBoolean="true"> |
There was a problem hiding this comment.
Please dont enable it by default!
Let it be false automatically, and which ever accounts they want to enable, they can do it manually. Existing deployments might already using users for different situations where limiting is not needed.
|
|
||
| @Getter | ||
| @Column(name = "is_login_retries_enabled", nullable = false) | ||
| private boolean loginRetriesEnabled; |
There was a problem hiding this comment.
Name is misleading. This is a flag to control whether login retries are limited or not. Please make sure it reflects it.
| } | ||
|
|
||
| private static boolean isDefaultLoginRetriesExemptUser(final String username, final Boolean cannotChangePassword) { | ||
| return AppUserConstants.SYSTEM_USER_NAME.equalsIgnoreCase(username) || Boolean.TRUE.equals(cannotChangePassword); |
There was a problem hiding this comment.
The system user part i got, but the cannot change password is why needed?
adamsaghy
left a comment
adamsaghy
left a comment
There was a problem hiding this comment.
Kindly see my concerns!
2 participants
Description
Adds a configurable global setting to limit login retries and lock accounts after exceeding the configured threshold. Failed attempts are tracked per user, and a successful login resets the counter. This aligns with FINERACT-2004 and provides safer, configurable account protection without changing public APIs.
Key changes
max-login-retry-attempts(enabled flag + value).failed_login_attemptsonm_appuserand lock the user when the threshold is reached.Testing
./gradlew :fineract-core:spotlessApply :fineract-core:spotbugsMain :fineract-core:spotbugsTest :fineract-core:checkstyleMain :fineract-core:checkstyleTest./gradlew :fineract-provider:spotlessApply :fineract-provider:spotbugsMain :fineract-provider:spotbugsTest :fineract-provider:checkstyleMain :fineract-provider:checkstyleTest./gradlew :integration-tests:spotlessApply :integration-tests:spotbugsMain :integration-tests:spotbugsTest :integration-tests:checkstyleMain :integration-tests:checkstyleTestChecklist