This is an automated email from the ASF dual-hosted git repository.
sureshanaparti pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new ed6ee6b704f Mark LDAP user query timeout as incorrect login instead of
disabling user immediately (#11220)
ed6ee6b704f is described below
commit ed6ee6b704ff84eb0efcec92fbcec518a90d30d0
Author: Suresh Kumar Anaparti <[email protected]>
AuthorDate: Fri Jul 25 19:31:43 2025 +0530
Mark LDAP user query timeout as incorrect login instead of disabling user
immediately (#11220)
* Mark LDAP user query timeout as incorrect login instead of disabling user
immediately
* code improvements
---
.../cloudstack/api/command/LdapListUsersCmd.java | 5 +-
.../apache/cloudstack/ldap/LdapAuthenticator.java | 58 ++++++++++++++--------
.../apache/cloudstack/ldap/LdapManagerImpl.java | 22 ++++----
3 files changed, 50 insertions(+), 35 deletions(-)
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
index e5d434d3810..6bfc70ea139 100644
---
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 class LdapListUsersCmd extends BaseListCmd {
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
index b8509881594..36c663566cb 100644
---
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 class LdapAuthenticator extends AdapterBase implements
UserAuthenticator
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,11 +127,11 @@ public class LdapAuthenticator extends AdapterBase
implements UserAuthenticator
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
@@ -137,12 +139,12 @@ public class LdapAuthenticator extends AdapterBase
implements UserAuthenticator
// 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 @@ public class LdapAuthenticator extends AdapterBase
implements UserAuthenticator
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 @@ public class LdapAuthenticator extends AdapterBase
implements UserAuthenticator
}
} 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 @@ public class LdapAuthenticator extends AdapterBase
implements UserAuthenticator
if (logger.isInfoEnabled()) {
logger.info(msg);
}
- if(remove) {
+ if (remove) {
removeUserInCloudStack(userAccount);
} else {
disableUserInCloudStack(userAccount);
@@ -229,23 +240,22 @@ public class LdapAuthenticator extends AdapterBase
implements UserAuthenticator
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 @@ public class LdapAuthenticator extends AdapterBase
implements UserAuthenticator
*/
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
index 2394c0b3d1e..05b8578bb42 100644
---
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 class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
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 class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
// 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 @@ public class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
@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 class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
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 class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
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 class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
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 class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
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 class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
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 class LdapManagerImpl extends ComponentLifecycleBase
implements LdapManag
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());
