Mark LDAP user query timeout as incorrect login instead of disabling user immediately (#11220)

sureshanaparti · web-flow · commit ed6ee6b704ff · 2025-07-25T19:31:43.000+05:30
* Mark LDAP user query timeout as incorrect login instead of disabling user immediately

* code improvements
diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
@@ -140,10 +140,11 @@ public void execute() throws ServerApiException {
         try {
             final List<LdapUser> users = _ldapManager.getUsers(domainId);
             ldapResponses = createLdapUserResponse(users);
-//            now filter and annotate
+            // now filter and annotate
             ldapResponses = applyUserFilter(ldapResponses);
         } catch (final NoLdapUserMatchingQueryException ex) {
-            // ok, we'll make do with the empty list ldapResponses = new ArrayList<LdapUserResponse>();
+            logger.debug(ex.getMessage());
+            // ok, we'll make do with the empty list
         } finally {
             response.setResponses(ldapResponses);
             response.setResponseName(getCommandName());
diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapAuthenticator.java
@@ -45,6 +45,8 @@ public class LdapAuthenticator extends AdapterBase implements UserAuthenticator
     @Inject
     private AccountManager _accountManager;
 
+    private static final String LDAP_READ_TIMED_OUT_MESSAGE = "LDAP response read timed out";
+
     public LdapAuthenticator() {
         super();
     }
@@ -74,8 +76,8 @@ public Pair<Boolean, ActionOnFailedAuthentication> authenticate(final String use
                     return rc;
                 }
                 List<LdapTrustMapVO> ldapTrustMapVOs = getLdapTrustMapVOS(domainId);
-                if(ldapTrustMapVOs != null && ldapTrustMapVOs.size() > 0) {
-                    if(ldapTrustMapVOs.size() == 1 && ldapTrustMapVOs.get(0).getAccountId() == 0) {
+                if (ldapTrustMapVOs != null && ldapTrustMapVOs.size() > 0) {
+                    if (ldapTrustMapVOs.size() == 1 && ldapTrustMapVOs.get(0).getAccountId() == 0) {
                         if (logger.isTraceEnabled()) {
                             logger.trace("We have a single mapping of a domain to an ldap group or ou");
                         }
@@ -125,24 +127,24 @@ Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String
             mappedGroups.retainAll(memberships);
             tracelist("actual groups for " + username, mappedGroups);
             // check membership, there must be only one match in this domain
-            if(ldapUser.isDisabled()) {
+            if (ldapUser.isDisabled()) {
                 logAndDisable(userAccount, "attempt to log on using disabled ldap user " + userAccount.getUsername(), false);
-            } else if(mappedGroups.size() > 1) {
+            } else if (mappedGroups.size() > 1) {
                 logAndDisable(userAccount, "user '" + username + "' is mapped to more then one account in domain and will be disabled.", false);
-            } else if(mappedGroups.size() < 1) {
+            } else if (mappedGroups.size() < 1) {
                 logAndDisable(userAccount, "user '" + username + "' is not mapped to an account in domain and will be removed.", true);
             } else {
                 // a valid ldap configured user exists
                 LdapTrustMapVO mapping = _ldapManager.getLinkedLdapGroup(domainId,mappedGroups.get(0));
                 // we could now assert that ldapTrustMapVOs.contains(mapping);
                 // createUser in Account can only be done by account name not by account id;
                 Account account = _accountManager.getAccount(mapping.getAccountId());
-                if(null == account) {
+                if (null == account) {
                     throw new CloudRuntimeException(String.format("account for user (%s) not found by id %d", username, mapping.getAccountId()));
                 }
                 String accountName = account.getAccountName();
                 rc.first(_ldapManager.canAuthenticate(ldapUser.getPrincipal(), password, domainId));
-                if (! rc.first()) {
+                if (!rc.first()) {
                     rc.second(ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
                 }
                 // for security reasons we keep processing on faulty login attempt to not give a way information on userid existence
@@ -162,7 +164,7 @@ Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String
                     userAccount = _accountManager.getUserAccountById(user.getId());
                 } else {
                     // not a new user, check if mapped group has changed
-                    if(userAccount.getAccountId() != mapping.getAccountId()) {
+                    if (userAccount.getAccountId() != mapping.getAccountId()) {
                         final Account mappedAccount = _accountManager.getAccount(mapping.getAccountId());
                         if (mappedAccount == null || mappedAccount.getRemoved() != null) {
                             throw new CloudRuntimeException("Mapped account for users does not exist. Please contact your administrator.");
@@ -174,12 +176,21 @@ Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String
             }
         } catch (NoLdapUserMatchingQueryException e) {
             logger.debug(e.getMessage());
-            disableUserInCloudStack(userAccount);
+            processLdapUserErrorMessage(userAccount, e.getMessage(), rc);
         }
 
         return rc;
     }
 
+    private void processLdapUserErrorMessage(UserAccount user, String errorMessage, Pair<Boolean, ActionOnFailedAuthentication> rc) {
+        if (StringUtils.isNotEmpty(errorMessage) && errorMessage.contains(LDAP_READ_TIMED_OUT_MESSAGE) && !rc.first()) {
+            rc.second(ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
+        } else {
+            // no user in ldap ==>> disable user in cloudstack
+            disableUserInCloudStack(user);
+        }
+    }
+
     private void tracelist(String msg, List<String> listToTrace) {
         if (logger.isTraceEnabled()) {
             StringBuilder logMsg = new StringBuilder();
@@ -197,7 +208,7 @@ private void logAndDisable(UserAccount userAccount, String msg, boolean remove)
         if (logger.isInfoEnabled()) {
             logger.info(msg);
         }
-        if(remove) {
+        if (remove) {
             removeUserInCloudStack(userAccount);
         } else {
             disableUserInCloudStack(userAccount);
@@ -229,23 +240,22 @@ private Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username
             processLdapUser(password, domainId, user, rc, ldapUser, accountType);
         } catch (NoLdapUserMatchingQueryException e) {
             logger.debug(e.getMessage());
-            // no user in ldap ==>> disable user in cloudstack
-            disableUserInCloudStack(user);
+            processLdapUserErrorMessage(user, e.getMessage(), rc);
         }
         return rc;
     }
 
     private void processLdapUser(String password, Long domainId, UserAccount user, Pair<Boolean, ActionOnFailedAuthentication> rc, LdapUser ldapUser, Account.Type accountType) {
-        if(!ldapUser.isDisabled()) {
+        if (!ldapUser.isDisabled()) {
             rc.first(_ldapManager.canAuthenticate(ldapUser.getPrincipal(), password, domainId));
-            if(rc.first()) {
-                if(user == null) {
+            if (rc.first()) {
+                if (user == null) {
                     // import user to cloudstack
                     createCloudStackUserAccount(ldapUser, domainId, accountType);
                 } else {
                     enableUserInCloudStack(user);
                 }
-            } else if(user != null) {
+            } else if (user != null) {
                 rc.second(ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
             }
         } else {
@@ -264,30 +274,34 @@ private void processLdapUser(String password, Long domainId, UserAccount user, P
      */
     Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, UserAccount user) {
         boolean result = false;
+        boolean timedOut = false;
 
-        if(user != null ) {
+        if (user != null ) {
             try {
                 LdapUser ldapUser = _ldapManager.getUser(username, domainId);
-                if(!ldapUser.isDisabled()) {
+                if (!ldapUser.isDisabled()) {
                     result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password, domainId);
                 } else {
                     logger.debug("user with principal "+ ldapUser.getPrincipal() + " is disabled in ldap");
                 }
             } catch (NoLdapUserMatchingQueryException e) {
                 logger.debug(e.getMessage());
+                if (e.getMessage().contains(LDAP_READ_TIMED_OUT_MESSAGE)) {
+                    timedOut = true;
+                }
             }
         }
-        return processResultAndAction(user, result);
+        return processResultAndAction(user, result, timedOut);
     }
 
-    private Pair<Boolean, ActionOnFailedAuthentication> processResultAndAction(UserAccount user, boolean result) {
-        return (!result && user != null) ?
+    private Pair<Boolean, ActionOnFailedAuthentication> processResultAndAction(UserAccount user, boolean result, boolean timedOut) {
+        return (!result && (user != null || timedOut)) ?
                 new Pair<Boolean, ActionOnFailedAuthentication>(result, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT):
                 new Pair<Boolean, ActionOnFailedAuthentication>(result, null);
     }
 
     private void enableUserInCloudStack(UserAccount user) {
-        if(user != null && (user.getState().equalsIgnoreCase(Account.State.DISABLED.toString()))) {
+        if (user != null && (user.getState().equalsIgnoreCase(Account.State.DISABLED.toString()))) {
             _accountManager.enableUser(user.getId());
         }
     }
diff --git a/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java b/plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/LdapManagerImpl.java
@@ -166,7 +166,7 @@ public LdapConfigurationResponse addConfiguration(final String hostname, int por
 
     private LdapConfigurationResponse addConfigurationInternal(final String hostname, int port, final Long domainId) throws InvalidParameterValueException {
         // TODO evaluate what the right default should be
-        if(port <= 0) {
+        if (port <= 0) {
             port = 389;
         }
 
@@ -210,7 +210,7 @@ public boolean canAuthenticate(final String principal, final String password, fi
             // TODO return the right account for this user
             final LdapContext context = _ldapContextFactory.createUserContext(principal, password, domainId);
             closeContext(context);
-            if(logger.isTraceEnabled()) {
+            if (logger.isTraceEnabled()) {
                 logger.trace(String.format("User(%s) authenticated for domain(%s)", principal, domainId));
             }
             return true;
@@ -234,7 +234,7 @@ private void closeContext(final LdapContext context) {
     @Override
     public LdapConfigurationResponse createLdapConfigurationResponse(final LdapConfigurationVO configuration) {
         String domainUuid = null;
-        if(configuration.getDomainId() != null) {
+        if (configuration.getDomainId() != null) {
             DomainVO domain = domainDao.findById(configuration.getDomainId());
             if (domain != null) {
                 domainUuid = domain.getUuid();
@@ -303,8 +303,8 @@ public LdapUser getUser(final String username, Long domainId) throws NoLdapUserM
             return _ldapUserManagerFactory.getInstance(_ldapConfiguration.getLdapProvider(null)).getUser(escapedUsername, context, domainId);
 
         } catch (NamingException | IOException e) {
-            logger.debug("ldap Exception: ",e);
-            throw new NoLdapUserMatchingQueryException("No Ldap User found for username: "+username);
+            logger.debug("LDAP Exception: ", e);
+            throw new NoLdapUserMatchingQueryException("Unable to find LDAP User for username: " + username + ", due to " + e.getMessage());
         } finally {
             closeContext(context);
         }
@@ -324,8 +324,8 @@ public LdapUser getUser(final String username, final String type, final String n
             LdapUserManager userManagerFactory = _ldapUserManagerFactory.getInstance(ldapProvider);
             return userManagerFactory.getUser(escapedUsername, type, name, context, domainId);
         } catch (NamingException | IOException e) {
-            logger.debug("ldap Exception: ",e);
-            throw new NoLdapUserMatchingQueryException("No Ldap User found for username: "+username + " in group: " + name + " of type: " + type);
+            logger.debug("LDAP Exception: ", e);
+            throw new NoLdapUserMatchingQueryException("Unable to find LDAP User for username: " + username + " in group: " + name + " of type: " + type + ", due to " + e.getMessage());
         } finally {
             closeContext(context);
         }
@@ -338,7 +338,7 @@ public List<LdapUser> getUsers(Long domainId) throws NoLdapUserMatchingQueryExce
             context = _ldapContextFactory.createBindContext(domainId);
             return _ldapUserManagerFactory.getInstance(_ldapConfiguration.getLdapProvider(domainId)).getUsers(context, domainId);
         } catch (NamingException | IOException e) {
-            logger.debug("ldap Exception: ",e);
+            logger.debug("LDAP Exception: ", e);
             throw new NoLdapUserMatchingQueryException("*");
         } finally {
             closeContext(context);
@@ -352,7 +352,7 @@ public List<LdapUser> getUsersInGroup(String groupName, Long domainId) throws No
             context = _ldapContextFactory.createBindContext(domainId);
             return _ldapUserManagerFactory.getInstance(_ldapConfiguration.getLdapProvider(domainId)).getUsersInGroup(groupName, context, domainId);
         } catch (NamingException | IOException e) {
-            logger.debug("ldap NamingException: ",e);
+            logger.debug("LDAP Exception: ", e);
             throw new NoLdapUserMatchingQueryException("groupName=" + groupName);
         } finally {
             closeContext(context);
@@ -390,7 +390,7 @@ public List<LdapUser> searchUsers(final String username) throws NoLdapUserMatchi
             final String escapedUsername = LdapUtils.escapeLDAPSearchFilter(username);
             return _ldapUserManagerFactory.getInstance(_ldapConfiguration.getLdapProvider(null)).getUsers("*" + escapedUsername + "*", context, null);
         } catch (NamingException | IOException e) {
-            logger.debug("ldap Exception: ",e);
+            logger.debug("LDAP Exception: ",e);
             throw new NoLdapUserMatchingQueryException(username);
         } finally {
             closeContext(context);
@@ -481,7 +481,7 @@ public LinkAccountToLdapResponse linkAccountToLdap(LinkAccountToLdapCmd cmd) {
     private void clearOldAccountMapping(LinkAccountToLdapCmd cmd) {
         //        first find if exists log warning and update
         LdapTrustMapVO oldVo = _ldapTrustMapDao.findGroupInDomain(cmd.getDomainId(), cmd.getLdapDomain());
-        if(oldVo != null) {
+        if (oldVo != null) {
             // deal with edge cases, i.e. check if the old account is indeed deleted etc.
             if (oldVo.getAccountId() != 0l) {
                 AccountVO oldAcount = accountDao.findByIdIncludingRemoved(oldVo.getAccountId());
