You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by al...@apache.org on 2014/01/22 03:13:39 UTC

git commit: updated refs/heads/master to ab627bc

Updated Branches:
  refs/heads/master a0197006e -> ab627bc76


Changed "authenticate" method to return both - result of authentication, and action to perform when authentication failed - to the accountManagerImpl. Only if authenicators request INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT, the incorrect_login_attempts parameter will be increased

Signed-off-by: Alena Prokharchyk <al...@citrix.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/ab627bc7
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/ab627bc7
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/ab627bc7

Branch: refs/heads/master
Commit: ab627bc76764829fb9fb5d6bd9735b9d18519a77
Parents: a019700
Author: Alena Prokharchyk <al...@citrix.com>
Authored: Tue Jan 7 12:15:37 2014 -0800
Committer: Alena Prokharchyk <al...@citrix.com>
Committed: Tue Jan 21 17:45:53 2014 -0800

----------------------------------------------------------------------
 .../cloudstack/ldap/LdapAuthenticator.java      | 15 +++++++---
 .../cloud/server/auth/MD5UserAuthenticator.java |  9 +++---
 .../server/auth/PlainTextUserAuthenticator.java |  9 +++---
 .../auth/SHA256SaltedUserAuthenticator.java     | 10 +++++--
 .../cloud/server/auth/UserAuthenticator.java    |  8 ++++--
 .../src/com/cloud/user/AccountManagerImpl.java  | 29 +++++++++++++-------
 6 files changed, 54 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ab627bc7/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java
----------------------------------------------------------------------
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 0bfbab6..afba272 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<String, Object[]> requestParameters) {
+    public Pair<Boolean, ActionOnFailedAuthentication> authenticate(final String username, final String password, final Long domainId, final Map<String, Object[]> 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<Boolean, ActionOnFailedAuthentication>(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<Boolean, ActionOnFailedAuthentication>(result, action);
+
         } else {
-            return false;
+            return new Pair<Boolean, ActionOnFailedAuthentication>(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ab627bc7/plugins/user-authenticators/md5/src/com/cloud/server/auth/MD5UserAuthenticator.java
----------------------------------------------------------------------
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 f1fe448..c1415d2 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<String, Object[]> requestParameters) {
+    public Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, Map<String, Object[]> 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<Boolean, ActionOnFailedAuthentication>(false, null);
         }
 
         if (!user.getPassword().equals(encode(password))) {
             s_logger.debug("Password does not match");
-            return false;
+            return new Pair<Boolean, ActionOnFailedAuthentication>(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
         }
-        return true;
+        return new Pair<Boolean, ActionOnFailedAuthentication>(true, null);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ab627bc7/plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
----------------------------------------------------------------------
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 ac03fde..0afbbfc 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<String, Object[]> requestParameters) {
+    public Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, Map<String, Object[]> 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<Boolean, ActionOnFailedAuthentication>(false, null);
         }
 
         if (!user.getPassword().equals(password)) {
             s_logger.debug("Password does not match");
-            return false;
+            return new Pair<Boolean, ActionOnFailedAuthentication>(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
         }
-        return true;
+        return new Pair<Boolean, ActionOnFailedAuthentication>(true, null);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ab627bc7/plugins/user-authenticators/sha256salted/src/com/cloud/server/auth/SHA256SaltedUserAuthenticator.java
----------------------------------------------------------------------
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 8dac9c4..36305f1 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<String, Object[]> requestParameters) {
+    public Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, Map<String, Object[]> 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<Boolean, ActionOnFailedAuthentication>(result, action);
         } catch (NoSuchAlgorithmException e) {
             throw new CloudRuntimeException("Unable to hash password", e);
         } catch (UnsupportedEncodingException e) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ab627bc7/server/src/com/cloud/server/auth/UserAuthenticator.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/server/auth/UserAuthenticator.java b/server/src/com/cloud/server/auth/UserAuthenticator.java
index 3fccebc..895c3c0 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<String, Object[]> requestParameters);
+    public Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, Map<String, Object[]> requestParameters);
 
     /**
      * @param password

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/ab627bc7/server/src/com/cloud/user/AccountManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java
index 5204589..186cfb2 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<ActionOnFailedAuthentication> actionsOnFailedAuthenticaion = new HashSet<ActionOnFailedAuthentication>();
         for (UserAuthenticator authenticator : _userAuthenticators) {
-            if (authenticator.authenticate(username, password, domainId, requestParameters)) {
+            Pair<Boolean, ActionOnFailedAuthentication> 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 {