diff --git a/server/src/main/java/com/cloud/configuration/Config.java b/server/src/main/java/com/cloud/configuration/Config.java index 45f6ced7c41..b882e86b682 100644 --- a/server/src/main/java/com/cloud/configuration/Config.java +++ b/server/src/main/java/com/cloud/configuration/Config.java @@ -984,7 +984,7 @@ public enum Config { Integer.class, "incorrect.login.attempts.allowed", "5", - "Incorrect login attempts allowed before the user is disabled", + "Incorrect login attempts allowed before the user is disabled (when value > 0). If value <=0 users are not disabled after failed login attempts", null), // Ovm OvmPublicNetwork("Hidden", ManagementServer.class, String.class, "ovm.public.network.device", null, "Specify the public bridge on host for public network", null), diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index bd51cd419e5..f0028192df5 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -2547,16 +2547,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (userAccount.getState().equalsIgnoreCase(Account.State.enabled.toString())) { if (!isInternalAccount(userAccount.getId())) { // Internal accounts are not disabled - int attemptsMade = userAccount.getLoginAttempts() + 1; - if (updateIncorrectLoginCount) { - if (attemptsMade < _allowedLoginAttempts) { - updateLoginAttempts(userAccount.getId(), attemptsMade, false); - s_logger.warn("Login attempt failed. You have " + (_allowedLoginAttempts - attemptsMade) + " attempt(s) remaining"); - } else { - updateLoginAttempts(userAccount.getId(), _allowedLoginAttempts, true); - s_logger.warn("User " + userAccount.getUsername() + " has been disabled due to multiple failed login attempts." + " Please contact admin."); - } - } + updateLoginAttemptsWhenIncorrectLoginAttemptsEnabled(userAccount, updateIncorrectLoginCount, _allowedLoginAttempts); } } else { s_logger.info("User " + userAccount.getUsername() + " is disabled/locked"); @@ -2565,6 +2556,23 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } } + protected void updateLoginAttemptsWhenIncorrectLoginAttemptsEnabled(UserAccount account, boolean updateIncorrectLoginCount, + int allowedLoginAttempts) { + int attemptsMade = account.getLoginAttempts() + 1; + if (allowedLoginAttempts <= 0 || !updateIncorrectLoginCount) { + return; + } + if (attemptsMade < allowedLoginAttempts) { + updateLoginAttempts(account.getId(), attemptsMade, false); + s_logger.warn("Login attempt failed. You have " + + (allowedLoginAttempts - attemptsMade) + " attempt(s) remaining"); + } else { + updateLoginAttempts(account.getId(), allowedLoginAttempts, true); + s_logger.warn("User " + account.getUsername() + + " has been disabled due to multiple failed login attempts." + " Please contact admin."); + } + } + @Override public Pair findUserByApiKey(String apiKey) { return _accountDao.findUserAccountByApiKey(apiKey); diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index e37507b98e4..96e60cd3664 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -710,4 +710,31 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.verify(authenticatorMock2, Mockito.times(1)).authenticate(username, currentPassword, domainId, null); } + @Test + public void testUpdateLoginAttemptsDisableMechanism() { + accountManagerImpl.updateLoginAttemptsWhenIncorrectLoginAttemptsEnabled(userAccountVO, true, 0); + Mockito.verify(accountManagerImpl, Mockito.never()).updateLoginAttempts(Mockito.anyLong(), Mockito.anyInt(), Mockito.anyBoolean()); + } + + @Test + public void testUpdateLoginAttemptsEnableMechanismAttemptsLeft() { + int attempts = 2; + int allowedAttempts = 5; + Long accountId = 1L; + Mockito.when(userAccountVO.getLoginAttempts()).thenReturn(attempts); + Mockito.when(userAccountVO.getId()).thenReturn(accountId); + accountManagerImpl.updateLoginAttemptsWhenIncorrectLoginAttemptsEnabled(userAccountVO, true, allowedAttempts); + Mockito.verify(accountManagerImpl).updateLoginAttempts(Mockito.eq(accountId), Mockito.eq(attempts + 1), Mockito.eq(false)); + } + + @Test + public void testUpdateLoginAttemptsEnableMechanismNoAttemptsLeft() { + int attempts = 5; + int allowedAttempts = 5; + Long accountId = 1L; + Mockito.when(userAccountVO.getLoginAttempts()).thenReturn(attempts); + Mockito.when(userAccountVO.getId()).thenReturn(accountId); + accountManagerImpl.updateLoginAttemptsWhenIncorrectLoginAttemptsEnabled(userAccountVO, true, allowedAttempts); + Mockito.verify(accountManagerImpl).updateLoginAttempts(Mockito.eq(accountId), Mockito.eq(allowedAttempts), Mockito.eq(true)); + } }