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 04:56:20 UTC

git commit: updated refs/heads/4.3 to d5e0dcd

Updated Branches:
  refs/heads/4.3 361aaca79 -> d5e0dcd2a


Revert "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"

This reverts commit 7884bb8aafe46b880607fda3602c317b2def8e9d.


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

Branch: refs/heads/4.3
Commit: d5e0dcd2a7339d529731b729817eb8f80a6d53c3
Parents: 361aaca
Author: Alena Prokharchyk <al...@citrix.com>
Authored: Tue Jan 21 19:50:27 2014 -0800
Committer: Alena Prokharchyk <al...@citrix.com>
Committed: Tue Jan 21 19:50:46 2014 -0800

----------------------------------------------------------------------
 .../cloudstack/ldap/LdapAuthenticator.java      | 31 ++++------
 .../cloud/server/auth/MD5UserAuthenticator.java | 64 ++++++++++----------
 .../server/auth/PlainTextUserAuthenticator.java | 42 ++++++-------
 .../auth/SHA256SaltedUserAuthenticator.java     | 13 ++--
 .../cloud/server/auth/UserAuthenticator.java    | 35 +++++------
 .../src/com/cloud/user/AccountManagerImpl.java  | 25 +++-----
 6 files changed, 92 insertions(+), 118 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d5e0dcd2/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 dac917b..559a979 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,7 +25,6 @@ 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
@@ -47,27 +46,23 @@ public class LdapAuthenticator extends DefaultUserAuthenticator {
 		_userAccountDao = userAccountDao;
 	}
 
-    @Override
-    public Pair<Boolean, ActionOnFailedAuthentication> authenticate(final String username, final String password, final Long domainId, final Map<String, Object[]> requestParameters) {
+	@Override
+	public boolean 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 new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
-        } else if (_ldapManager.isLdapEnabled()) {
-            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 new Pair<Boolean, ActionOnFailedAuthentication>(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
-        }
-    }
+		if (user == null) {
+			s_logger.debug("Unable to find user with " + username
+					+ " in domain " + domainId);
+			return false;
+		} else if (_ldapManager.isLdapEnabled()) {
+			return _ldapManager.canAuthenticate(username, password);
+		} else {
+			return false;
+		}
+	}
 
 	@Override
 	public String encode(final String password) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d5e0dcd2/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 f8f75ed..63583af 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,7 +27,6 @@ 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;
 
 /**
@@ -35,48 +34,47 @@ import com.cloud.utils.exception.CloudRuntimeException;
  * comparing it against the local database.
  * 
  */
-@Local(value = {UserAuthenticator.class})
+@Local(value={UserAuthenticator.class})
 public class MD5UserAuthenticator extends DefaultUserAuthenticator {
-    public static final Logger s_logger = Logger.getLogger(MD5UserAuthenticator.class);
-
-    @Inject
-    private UserAccountDao _userAccountDao;
-
-    @Override
-    public Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, Map<String, Object[]> requestParameters) {
-        if (s_logger.isDebugEnabled()) {
+	public static final Logger s_logger = Logger.getLogger(MD5UserAuthenticator.class);
+	
+	@Inject private UserAccountDao _userAccountDao;
+	
+	@Override
+	public boolean 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 new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
+            return false;
         }
-
+        
         if (!user.getPassword().equals(encode(password))) {
             s_logger.debug("Password does not match");
-            return new Pair<Boolean, ActionOnFailedAuthentication>(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
+            return false;
         }
-        return new Pair<Boolean, ActionOnFailedAuthentication>(true, null);
-    }
+		return true;
+	}
 
-    public String encode(String password) {
-        MessageDigest md5 = null;
-        try {
-            md5 = MessageDigest.getInstance("MD5");
-        } catch (NoSuchAlgorithmException e) {
-            throw new CloudRuntimeException("Unable to hash password", e);
-        }
+	public String encode(String password) {
+		MessageDigest md5 = null;
+		try {
+			md5 = MessageDigest.getInstance("MD5");
+		} catch (NoSuchAlgorithmException e) {
+			throw new CloudRuntimeException("Unable to hash password", e);
+		}
 
-        md5.reset();
-        BigInteger pwInt = new BigInteger(1, md5.digest(password.getBytes()));
-        String pwStr = pwInt.toString(16);
-        int padding = 32 - pwStr.length();
-        StringBuffer sb = new StringBuffer();
-        for (int i = 0; i < padding; i++) {
-            sb.append('0'); // make sure the MD5 password is 32 digits long
-        }
-        sb.append(pwStr);
-        return sb.toString();
-    }
+		md5.reset();
+		BigInteger pwInt = new BigInteger(1, md5.digest(password.getBytes()));
+		String pwStr = pwInt.toString(16);
+		int padding = 32 - pwStr.length();
+		StringBuffer sb = new StringBuffer();
+		for (int i = 0; i < padding; i++) {
+		    sb.append('0'); // make sure the MD5 password is 32 digits long
+		}
+		sb.append(pwStr);
+		return sb.toString();
+	}
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d5e0dcd2/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 6a5f415..849e82e 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,38 +24,36 @@ 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 {
-    public static final Logger s_logger = Logger.getLogger(PlainTextUserAuthenticator.class);
-
-    @Inject
-    private UserAccountDao _userAccountDao;
 
-    @Override
-    public Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, Map<String, Object[]> requestParameters) {
-        if (s_logger.isDebugEnabled()) {
+@Local(value={UserAuthenticator.class})
+public class PlainTextUserAuthenticator extends DefaultUserAuthenticator {
+	public static final Logger s_logger = Logger.getLogger(PlainTextUserAuthenticator.class);
+	
+	@Inject private UserAccountDao _userAccountDao;
+	
+	@Override
+	public boolean 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 new Pair<Boolean, ActionOnFailedAuthentication>(false, null);
+            return false;
         }
-
+        
         if (!user.getPassword().equals(password)) {
             s_logger.debug("Password does not match");
-            return new Pair<Boolean, ActionOnFailedAuthentication>(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT);
+            return false;
         }
-
-        return new Pair<Boolean, ActionOnFailedAuthentication>(true, null);
-    }
-
-    @Override
-    public String encode(String password) {
-        // Plaintext so no encoding at all
-        return password;
-    }
+		return true;
+	}
+
+	@Override
+	public String encode(String password) {
+		// Plaintext so no encoding at all
+		return password; 
+	}
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d5e0dcd2/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 36305f1..3592ddc 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,10 +30,9 @@ 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})
+@Local(value={UserAuthenticator.class})
 public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator {
     public static final Logger s_logger = Logger.getLogger(SHA256SaltedUserAuthenticator.class);
     private static final String s_defaultPassword = "000000000000000000000000000=";
@@ -46,7 +45,8 @@ 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 Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, Map<String, Object[]> requestParameters) {
+    public boolean authenticate(String username, String password,
+            Long domainId, Map<String, Object[]> requestParameters) {
         if (s_logger.isDebugEnabled()) {
             s_logger.debug("Retrieving user: " + username);
         }
@@ -72,12 +72,7 @@ public class SHA256SaltedUserAuthenticator extends DefaultUserAuthenticator {
         try {
             String hashedPassword = encode(password, salt);
             /* constantTimeEquals comes first in boolean since we need to thwart timing attacks */
-            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);
+            return constantTimeEquals(realPassword, hashedPassword) && realUser;
         } catch (NoSuchAlgorithmException e) {
             throw new CloudRuntimeException("Unable to hash password", e);
         } catch (UnsupportedEncodingException e) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d5e0dcd2/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 cc527b8..95c4f0e 100644
--- a/server/src/com/cloud/server/auth/UserAuthenticator.java
+++ b/server/src/com/cloud/server/auth/UserAuthenticator.java
@@ -18,29 +18,26 @@ package com.cloud.server.auth;
 
 import java.util.Map;
 
-import com.cloud.utils.Pair;
 import com.cloud.utils.component.Adapter;
 
 /**
  * which UserAuthenticator to user in components.xml.
+ * 
  */
 public interface UserAuthenticator extends Adapter {
-    public enum ActionOnFailedAuthentication {
-        INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT;
-    }
-
-    /**
-     *
-     * @param username
-     * @param password
-     * @param domainId
-     * @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 Pair<Boolean, ActionOnFailedAuthentication> authenticate(String username, String password, Long domainId, Map<String, Object[]> requestParameters);
-
-    /**
-     * @param password
-     * @return the encoded password
-     */
-    public String encode(String password);
+	
+	/**
+	 * 
+	 * @param username
+	 * @param password
+	 * @param domainId
+	 * @return true if the user has been successfully authenticated, false otherwise
+	 */
+	public boolean authenticate(String username, String password, Long domainId, Map<String, Object[]> requestParameters);
+	
+	/**
+	 * @param password
+	 * @return the encoded password
+	 */
+	public String encode(String password);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d5e0dcd2/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 d367653..0655f8c 100755
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -21,7 +21,6 @@ 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;
@@ -111,7 +110,6 @@ 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;
@@ -1962,19 +1960,13 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
         }
 
         boolean authenticated = false;
-        HashSet<ActionOnFailedAuthentication> actionsOnFailedAuthenticaion = new HashSet<ActionOnFailedAuthentication>();
         for (UserAuthenticator authenticator : _userAuthenticators) {
-            Pair<Boolean, ActionOnFailedAuthentication> result = authenticator.authenticate(username, password, domainId, requestParameters);
-            if (result.first()) {
+            if (authenticator.authenticate(username, password, domainId, requestParameters)) {
                 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) {
@@ -2011,14 +2003,13 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
                     if (!isInternalAccount(userAccount.getType())) {
                         // 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.");
-                            }
+                        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 {