diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java index 0bfbab60424..afba27238a4 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java @@ -25,6 +25,7 @@ import org.apache.log4j.Logger; import com.cloud.server.auth.DefaultUserAuthenticator; import com.cloud.user.UserAccount; import com.cloud.user.dao.UserAccountDao; +import com.cloud.utils.Pair; public class LdapAuthenticator extends DefaultUserAuthenticator { private static final Logger s_logger = Logger.getLogger(LdapAuthenticator.class.getName()); @@ -45,17 +46,23 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { } @Override - public boolean authenticate(final String username, final String password, final Long domainId, final Map requestParameters) { + public Pair authenticate(final String username, final String password, final Long domainId, final Map requestParameters) { final UserAccount user = _userAccountDao.getUserAccount(username, domainId); if (user == null) { s_logger.debug("Unable to find user with " + username + " in domain " + domainId); - return false; + return new Pair(false, null); } else if (_ldapManager.isLdapEnabled()) { - return _ldapManager.canAuthenticate(username, password); + boolean result = _ldapManager.canAuthenticate(username, password); + ActionOnFailedAuthentication action = null; + if (result == false) { + action = ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT; + } + return new Pair(result, action); + } else { - return false; + return new Pair(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT); } } diff --git a/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java b/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java index f1fe448ffb5..c1415d256bb 100644 --- a/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java +++ b/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java @@ -27,6 +27,7 @@ import org.apache.log4j.Logger; import com.cloud.user.UserAccount; import com.cloud.user.dao.UserAccountDao; +import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; /** @@ -42,21 +43,21 @@ public class MD5UserAuthenticator extends DefaultUserAuthenticator { private UserAccountDao _userAccountDao; @Override - public boolean authenticate(String username, String password, Long domainId, Map requestParameters) { + public Pair authenticate(String username, String password, Long domainId, Map requestParameters) { if (s_logger.isDebugEnabled()) { s_logger.debug("Retrieving user: " + username); } UserAccount user = _userAccountDao.getUserAccount(username, domainId); if (user == null) { s_logger.debug("Unable to find user with " + username + " in domain " + domainId); - return false; + return new Pair(false, null); } if (!user.getPassword().equals(encode(password))) { s_logger.debug("Password does not match"); - return false; + return new Pair(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT); } - return true; + return new Pair(true, null); } @Override diff --git a/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java b/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java index ac03fde1dda..0afbbfc1c95 100644 --- a/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java +++ b/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java @@ -24,6 +24,7 @@ import org.apache.log4j.Logger; import com.cloud.user.UserAccount; import com.cloud.user.dao.UserAccountDao; +import com.cloud.utils.Pair; @Local(value = {UserAuthenticator.class}) public class PlainTextUserAuthenticator extends DefaultUserAuthenticator { @@ -33,7 +34,7 @@ public class PlainTextUserAuthenticator extends DefaultUserAuthenticator { private UserAccountDao _userAccountDao; @Override - public boolean authenticate(String username, String password, Long domainId, Map requestParameters) { + public Pair authenticate(String username, String password, Long domainId, Map requestParameters) { if (s_logger.isDebugEnabled()) { s_logger.debug("Retrieving user: " + username); } @@ -41,14 +42,14 @@ public class PlainTextUserAuthenticator extends DefaultUserAuthenticator { UserAccount user = _userAccountDao.getUserAccount(username, domainId); if (user == null) { s_logger.debug("Unable to find user with " + username + " in domain " + domainId); - return false; + return new Pair(false, null); } if (!user.getPassword().equals(password)) { s_logger.debug("Password does not match"); - return false; + return new Pair(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT); } - return true; + return new Pair(true, null); } @Override diff --git a/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java b/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java index 8dac9c49714..36305f18c99 100644 --- a/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java +++ b/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java @@ -30,6 +30,7 @@ import org.bouncycastle.util.encoders.Base64; import com.cloud.user.UserAccount; import com.cloud.user.dao.UserAccountDao; +import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; @Local(value = {UserAuthenticator.class}) @@ -45,7 +46,7 @@ public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator { * @see com.cloud.server.auth.UserAuthenticator#authenticate(java.lang.String, java.lang.String, java.lang.Long, java.util.Map) */ @Override - public boolean authenticate(String username, String password, Long domainId, Map requestParameters) { + public Pair authenticate(String username, String password, Long domainId, Map requestParameters) { if (s_logger.isDebugEnabled()) { s_logger.debug("Retrieving user: " + username); } @@ -71,7 +72,12 @@ public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator { try { String hashedPassword = encode(password, salt); /* constantTimeEquals comes first in boolean since we need to thwart timing attacks */ - return constantTimeEquals(realPassword, hashedPassword) && realUser; + boolean result = constantTimeEquals(realPassword, hashedPassword) && realUser; + ActionOnFailedAuthentication action = null; + if (!result && realUser) { + action = ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT; + } + return new Pair(result, action); } catch (NoSuchAlgorithmException e) { throw new CloudRuntimeException("Unable to hash password", e); } catch (UnsupportedEncodingException e) { diff --git a/server/src/com/cloud/server/auth/UserAuthenticator.java b/server/src/com/cloud/server/auth/UserAuthenticator.java index 3fccebc4dc7..895c3c06a61 100644 --- a/server/src/com/cloud/server/auth/UserAuthenticator.java +++ b/server/src/com/cloud/server/auth/UserAuthenticator.java @@ -18,6 +18,7 @@ package com.cloud.server.auth; import java.util.Map; +import com.cloud.utils.Pair; import com.cloud.utils.component.Adapter; /** @@ -25,15 +26,18 @@ import com.cloud.utils.component.Adapter; * */ public interface UserAuthenticator extends Adapter { + public enum ActionOnFailedAuthentication { + INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT; + } /** * * @param username * @param password * @param domainId - * @return true if the user has been successfully authenticated, false otherwise + * @return the pair of 2 booleans - first identifies the success of authenciation, the second - whether to increase incorrect login attempts count in case of failed authentication */ - public boolean authenticate(String username, String password, Long domainId, Map requestParameters); + public Pair authenticate(String username, String password, Long domainId, Map requestParameters); /** * @param password diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 52045897619..186cfb2afa2 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -21,6 +21,7 @@ import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -37,9 +38,6 @@ import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.commons.codec.binary.Base64; -import org.apache.log4j.Logger; - import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.acl.SecurityChecker; @@ -55,6 +53,8 @@ import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationSe import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.region.gslb.GlobalLoadBalancerRuleDao; +import org.apache.commons.codec.binary.Base64; +import org.apache.log4j.Logger; import com.cloud.api.ApiDBUtils; import com.cloud.api.query.vo.ControlledViewEntity; @@ -111,6 +111,7 @@ import com.cloud.projects.ProjectVO; import com.cloud.projects.dao.ProjectAccountDao; import com.cloud.projects.dao.ProjectDao; import com.cloud.server.auth.UserAuthenticator; +import com.cloud.server.auth.UserAuthenticator.ActionOnFailedAuthentication; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeApiService; @@ -1965,13 +1966,19 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } boolean authenticated = false; + HashSet actionsOnFailedAuthenticaion = new HashSet(); for (UserAuthenticator authenticator : _userAuthenticators) { - if (authenticator.authenticate(username, password, domainId, requestParameters)) { + Pair result = authenticator.authenticate(username, password, domainId, requestParameters); + if (result.first()) { authenticated = true; break; + } else if (result.second() != null) { + actionsOnFailedAuthenticaion.add(result.second()); } } + boolean updateIncorrectLoginCount = actionsOnFailedAuthenticaion.contains(ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT); + if (authenticated) { UserAccount userAccount = _userAccountDao.getUserAccount(username, domainId); if (userAccount == null) { @@ -2009,12 +2016,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (!isInternalAccount(userAccount.getType())) { // Internal accounts are not disabled int attemptsMade = userAccount.getLoginAttempts() + 1; - 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."); + 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."); + } } } } else {