Fix: Allow disabling the login attempts mechanism for disabling users (#6254)

* Fix: Allow disabling the login attempts mechanism for disabling users

* Refactor

Author: nvazquez

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),

server/src/main/java/com/cloud/user/AccountManagerImpl.java

@@ -2547,16 +2547,7 @@ private UserAccount getUserAccount(String username, String password, Long domain
             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 @@ private UserAccount getUserAccount(String username, String password, Long domain
         }
     }
 
+    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<User, Account> findUserByApiKey(String apiKey) {
         return _accountDao.findUserAccountByApiKey(apiKey);

server/src/test/java/com/cloud/user/AccountManagerImplTest.java

@@ -710,4 +710,31 @@ public void validateCurrentPasswordTestUserAuthenticatedWithProvidedCurrentPassw
         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));
+    }
 }