cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From raj...@apache.org
Subject git commit: updated refs/heads/master to 14f3ad5
Date Wed, 20 Aug 2014 10:28:42 GMT
Repository: cloudstack
Updated Branches:
  refs/heads/master 28ad34e31 -> 14f3ad55e


Fixed CLOUDSTACK-7374: added PaginationControl while querying ldap users


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

Branch: refs/heads/master
Commit: 14f3ad55ecd5aff36b9b95bf0be5fdd27f5f1e8a
Parents: 28ad34e
Author: Rajani Karuturi <rajanikaruturi@gmail.com>
Authored: Wed Aug 20 15:56:36 2014 +0530
Committer: Rajani Karuturi <rajanikaruturi@gmail.com>
Committed: Wed Aug 20 15:58:08 2014 +0530

----------------------------------------------------------------------
 .../api/command/LdapCreateAccountCmd.java       |   5 +-
 .../cloudstack/ldap/LdapConfiguration.java      |   9 +-
 .../cloudstack/ldap/LdapContextFactory.java     |  28 +-
 .../org/apache/cloudstack/ldap/LdapManager.java |   4 +-
 .../apache/cloudstack/ldap/LdapManagerImpl.java |  38 +-
 .../apache/cloudstack/ldap/LdapUserManager.java |  73 ++-
 .../ldap/LdapAuthenticatorSpec.groovy           |   8 +-
 .../ldap/LdapContextFactorySpec.groovy          |  11 +-
 .../ldap/LdapCreateAccountCmdSpec.groovy        |   5 +-
 .../cloudstack/ldap/LdapManagerImplSpec.groovy  | 498 ++++++++++---------
 .../cloudstack/ldap/LdapUserManagerSpec.groovy  |   9 +-
 11 files changed, 356 insertions(+), 332 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
index b753952..d1beffa 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java
@@ -21,8 +21,6 @@ import java.security.SecureRandom;
 import java.util.Map;
 
 import javax.inject.Inject;
-import javax.naming.NamingException;
-
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
@@ -35,6 +33,7 @@ import org.apache.cloudstack.api.response.DomainResponse;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.ldap.LdapManager;
 import org.apache.cloudstack.ldap.LdapUser;
+import org.apache.cloudstack.ldap.NoLdapUserMatchingQueryException;
 import org.apache.log4j.Logger;
 import org.bouncycastle.util.encoders.Base64;
 
@@ -134,7 +133,7 @@ public class LdapCreateAccountCmd extends BaseCmd {
             } else {
                 throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create a user account");
             }
-        } catch (final NamingException e) {
+        } catch (NoLdapUserMatchingQueryException e) {
             throw new ServerApiException(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, "No LDAP user exists with the username of " + username);
         }
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java
index baf0de8..c171ebf 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java
@@ -34,6 +34,9 @@ public class LdapConfiguration implements Configurable{
     private static final ConfigKey<Long> ldapReadTimeout = new ConfigKey<Long>(Long.class, "ldap.read.timeout", "Advanced", "1000",
         "LDAP connection Timeout in milli sec", true, ConfigKey.Scope.Global, 1l);
 
+    private static final ConfigKey<Integer> ldapPageSize = new ConfigKey<Integer>(Integer.class, "ldap.request.page.size", "Advanced", "1000",
+                                                                               "page size sent to ldap server on each request to get user", true, ConfigKey.Scope.Global, 1);
+
     private final static int scope = SearchControls.SUBTREE_SCOPE;
 
     @Inject
@@ -158,6 +161,10 @@ public class LdapConfiguration implements Configurable{
         return ldapReadTimeout.value();
     }
 
+    public Integer getLdapPageSize() {
+        return ldapPageSize.value();
+    }
+
     @Override
     public String getConfigComponentName() {
         return LdapConfiguration.class.getSimpleName();
@@ -165,6 +172,6 @@ public class LdapConfiguration implements Configurable{
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {ldapReadTimeout};
+        return new ConfigKey<?>[] {ldapReadTimeout, ldapPageSize};
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
index dc53578..9e27fff 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java
@@ -16,13 +16,14 @@
 // under the License.
 package org.apache.cloudstack.ldap;
 
+import java.io.IOException;
 import java.util.Hashtable;
 
 import javax.inject.Inject;
 import javax.naming.Context;
 import javax.naming.NamingException;
-import javax.naming.directory.DirContext;
-import javax.naming.directory.InitialDirContext;
+import javax.naming.ldap.InitialLdapContext;
+import javax.naming.ldap.LdapContext;
 
 import org.apache.log4j.Logger;
 
@@ -39,27 +40,28 @@ public class LdapContextFactory {
         _ldapConfiguration = ldapConfiguration;
     }
 
-    public DirContext createBindContext() throws NamingException {
+    public LdapContext createBindContext() throws NamingException, IOException {
         return createBindContext(null);
     }
 
-    public DirContext createBindContext(final String providerUrl) throws NamingException {
+    public LdapContext createBindContext(final String providerUrl) throws NamingException, IOException {
         final String bindPrincipal = _ldapConfiguration.getBindPrincipal();
         final String bindPassword = _ldapConfiguration.getBindPassword();
         return createInitialDirContext(bindPrincipal, bindPassword, providerUrl, true);
     }
 
-    private DirContext createInitialDirContext(final String principal, final String password, final boolean isSystemContext) throws NamingException {
+    private LdapContext createInitialDirContext(final String principal, final String password, final boolean isSystemContext) throws NamingException, IOException {
         return createInitialDirContext(principal, password, null, isSystemContext);
     }
 
-    private DirContext createInitialDirContext(final String principal, final String password, final String providerUrl, final boolean isSystemContext) throws NamingException {
+    private LdapContext createInitialDirContext(final String principal, final String password, final String providerUrl, final boolean isSystemContext)
+        throws NamingException, IOException {
         Hashtable<String, String> environment = getEnvironment(principal, password, providerUrl, isSystemContext);
         s_logger.debug("initializing ldap with provider url: " + environment.get(Context.PROVIDER_URL));
-        return new InitialDirContext(environment);
+        return new InitialLdapContext(environment, null);
     }
 
-    public DirContext createUserContext(final String principal, final String password) throws NamingException {
+    public LdapContext createUserContext(final String principal, final String password) throws NamingException, IOException {
         return createInitialDirContext(principal, password, false);
     }
 
@@ -109,14 +111,4 @@ public class LdapContextFactory {
         }
     }
 
-    public void testConnection(final String providerUrl) throws NamingException {
-        try {
-            createBindContext(providerUrl);
-            s_logger.info("LDAP Connection was successful");
-        } catch (final NamingException e) {
-            s_logger.warn("LDAP Connection failed");
-            s_logger.error(e.getMessage(), e);
-            throw e;
-        }
-    }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java
index 3da372b..31205c4 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java
@@ -18,8 +18,6 @@ package org.apache.cloudstack.ldap;
 
 import java.util.List;
 
-import javax.naming.NamingException;
-
 import org.apache.cloudstack.api.command.LdapListConfigurationCmd;
 import org.apache.cloudstack.api.response.LdapConfigurationResponse;
 import org.apache.cloudstack.api.response.LdapUserResponse;
@@ -40,7 +38,7 @@ public interface LdapManager extends PluggableService {
 
     LdapConfigurationResponse deleteConfiguration(String hostname) throws InvalidParameterValueException;
 
-    LdapUser getUser(final String username) throws NamingException;
+    LdapUser getUser(final String username) throws NoLdapUserMatchingQueryException;
 
     List<LdapUser> getUsers() throws NoLdapUserMatchingQueryException;
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
index 0e3bde8..87c0443 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
@@ -16,13 +16,14 @@
 // under the License.
 package org.apache.cloudstack.ldap;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
 import javax.ejb.Local;
 import javax.inject.Inject;
 import javax.naming.NamingException;
-import javax.naming.directory.DirContext;
+import javax.naming.ldap.LdapContext;
 
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -80,7 +81,7 @@ public class LdapManagerImpl implements LdapManager, LdapValidator {
                 _ldapConfigurationDao.persist(configuration);
                 s_logger.info("Added new ldap server with hostname: " + hostname);
                 return new LdapConfigurationResponse(hostname, port);
-            } catch (final NamingException e) {
+            } catch (NamingException | IOException e) {
                 s_logger.debug("NamingException while doing an LDAP bind", e);
                 throw new InvalidParameterValueException("Unable to bind to the given LDAP server");
             }
@@ -95,17 +96,17 @@ public class LdapManagerImpl implements LdapManager, LdapValidator {
         try {
             final LdapUser user = getUser(escapedUsername);
             final String principal = user.getPrincipal();
-            final DirContext context = _ldapContextFactory.createUserContext(principal, password);
+            final LdapContext context = _ldapContextFactory.createUserContext(principal, password);
             closeContext(context);
             return true;
-        } catch (final NamingException e) {
-            s_logger.debug("NamingException: while doing an LDAP bind for user "+" "+username, e);
+        } catch (NamingException | IOException | NoLdapUserMatchingQueryException e) {
+            s_logger.debug("Exception while doing an LDAP bind for user "+" "+username, e);
             s_logger.info("Failed to authenticate user: " + username + ". incorrect password.");
             return false;
         }
     }
 
-    private void closeContext(final DirContext context) {
+    private void closeContext(final LdapContext context) {
         try {
             if (context != null) {
                 context.close();
@@ -163,16 +164,17 @@ public class LdapManagerImpl implements LdapManager, LdapValidator {
     }
 
     @Override
-    public LdapUser getUser(final String username) throws NamingException {
-        DirContext context = null;
+    public LdapUser getUser(final String username) throws NoLdapUserMatchingQueryException {
+        LdapContext context = null;
         try {
             context = _ldapContextFactory.createBindContext();
 
             final String escapedUsername = LdapUtils.escapeLDAPSearchFilter(username);
             return _ldapUserManager.getUser(escapedUsername, context);
 
-        } catch (final NamingException e) {
-            throw e;
+        } catch (NamingException | IOException e) {
+            s_logger.debug("ldap Exception: ",e);
+            throw new NoLdapUserMatchingQueryException("No Ldap User found for username: "+username);
         } finally {
             closeContext(context);
         }
@@ -180,12 +182,12 @@ public class LdapManagerImpl implements LdapManager, LdapValidator {
 
     @Override
     public List<LdapUser> getUsers() throws NoLdapUserMatchingQueryException {
-        DirContext context = null;
+        LdapContext context = null;
         try {
             context = _ldapContextFactory.createBindContext();
             return _ldapUserManager.getUsers(context);
-        } catch (final NamingException e) {
-            s_logger.debug("ldap NamingException: ",e);
+        } catch (NamingException | IOException e) {
+            s_logger.debug("ldap Exception: ",e);
             throw new NoLdapUserMatchingQueryException("*");
         } finally {
             closeContext(context);
@@ -194,11 +196,11 @@ public class LdapManagerImpl implements LdapManager, LdapValidator {
 
     @Override
     public List<LdapUser> getUsersInGroup(String groupName) throws NoLdapUserMatchingQueryException {
-        DirContext context = null;
+        LdapContext context = null;
         try {
             context = _ldapContextFactory.createBindContext();
             return _ldapUserManager.getUsersInGroup(groupName, context);
-        } catch (final NamingException e) {
+        } catch (NamingException | IOException e) {
             s_logger.debug("ldap NamingException: ",e);
             throw new NoLdapUserMatchingQueryException("groupName=" + groupName);
         } finally {
@@ -221,13 +223,13 @@ public class LdapManagerImpl implements LdapManager, LdapValidator {
 
     @Override
     public List<LdapUser> searchUsers(final String username) throws NoLdapUserMatchingQueryException {
-        DirContext context = null;
+        LdapContext context = null;
         try {
             context = _ldapContextFactory.createBindContext();
             final String escapedUsername = LdapUtils.escapeLDAPSearchFilter(username);
             return _ldapUserManager.getUsers("*" + escapedUsername + "*", context);
-        } catch (final NamingException e) {
-            s_logger.debug("ldap NamingException: ",e);
+        } catch (NamingException | IOException e) {
+            s_logger.debug("ldap Exception: ",e);
             throw new NoLdapUserMatchingQueryException(username);
         } finally {
             closeContext(context);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManager.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManager.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManager.java
index beff9db..654a601 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManager.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManager.java
@@ -16,6 +16,7 @@
 // under the License.
 package org.apache.cloudstack.ldap;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -25,10 +26,14 @@ import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
 import javax.naming.directory.Attribute;
 import javax.naming.directory.Attributes;
-import javax.naming.directory.DirContext;
 import javax.naming.directory.SearchControls;
 import javax.naming.directory.SearchResult;
+import javax.naming.ldap.Control;
+import javax.naming.ldap.LdapContext;
+import javax.naming.ldap.PagedResultsControl;
+import javax.naming.ldap.PagedResultsResponseControl;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 
@@ -113,35 +118,29 @@ public class LdapUserManager {
         return result.toString();
     }
 
-    public LdapUser getUser(final String username, final DirContext context) throws NamingException {
-        final NamingEnumeration<SearchResult> result = searchUsers(username, context);
-        if (result.hasMoreElements()) {
-            return createUser(result.nextElement());
+    public LdapUser getUser(final String username, final LdapContext context) throws NamingException, IOException {
+        List<LdapUser> result = searchUsers(username, context);
+        if (result!= null && result.size() == 1) {
+            return result.get(0);
         } else {
             throw new NamingException("No user found for username " + username);
         }
     }
 
-    public List<LdapUser> getUsers(final DirContext context) throws NamingException {
+    public List<LdapUser> getUsers(final LdapContext context) throws NamingException, IOException {
         return getUsers(null, context);
     }
 
-    public List<LdapUser> getUsers(final String username, final DirContext context) throws NamingException {
-        final NamingEnumeration<SearchResult> results = searchUsers(username, context);
+    public List<LdapUser> getUsers(final String username, final LdapContext context) throws NamingException, IOException {
+        List<LdapUser> users = searchUsers(username, context);
 
-        final List<LdapUser> users = new ArrayList<LdapUser>();
-
-        while (results.hasMoreElements()) {
-            final SearchResult result = results.nextElement();
-            users.add(createUser(result));
+        if (CollectionUtils.isNotEmpty(users)) {
+            Collections.sort(users);
         }
-
-        Collections.sort(users);
-
         return users;
     }
 
-    public List<LdapUser> getUsersInGroup(String groupName, DirContext context) throws NamingException {
+    public List<LdapUser> getUsersInGroup(String groupName, LdapContext context) throws NamingException {
         String attributeName = _ldapConfiguration.getGroupUniqueMemeberAttribute();
         final SearchControls controls = new SearchControls();
         controls.setSearchScope(_ldapConfiguration.getScope());
@@ -170,7 +169,7 @@ public class LdapUserManager {
         return users;
     }
 
-    private LdapUser getUserForDn(String userdn, DirContext context) throws NamingException {
+    private LdapUser getUserForDn(String userdn, LdapContext context) throws NamingException {
         final SearchControls controls = new SearchControls();
         controls.setSearchScope(_ldapConfiguration.getScope());
         controls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
@@ -183,20 +182,46 @@ public class LdapUserManager {
         }
     }
 
-    public NamingEnumeration<SearchResult> searchUsers(final DirContext context) throws NamingException {
+    public List<LdapUser> searchUsers(final LdapContext context) throws NamingException, IOException {
         return searchUsers(null, context);
     }
 
-    public NamingEnumeration<SearchResult> searchUsers(final String username, final DirContext context) throws NamingException {
-        final SearchControls controls = new SearchControls();
+    public List<LdapUser> searchUsers(final String username, final LdapContext context) throws NamingException, IOException {
 
-        controls.setSearchScope(_ldapConfiguration.getScope());
-        controls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
+        final SearchControls searchControls = new SearchControls();
+
+        searchControls.setSearchScope(_ldapConfiguration.getScope());
+        searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
 
         String basedn = _ldapConfiguration.getBaseDn();
         if (StringUtils.isBlank(basedn)) {
             throw new IllegalArgumentException("ldap basedn is not configured");
         }
-        return context.search(basedn, generateSearchFilter(username), controls);
+        byte[] cookie = null;
+        int pageSize = _ldapConfiguration.getLdapPageSize();
+        context.setRequestControls(new Control[]{new PagedResultsControl(pageSize, Control.NONCRITICAL)});
+        final List<LdapUser> users = new ArrayList<LdapUser>();
+        NamingEnumeration<SearchResult> results;
+        do {
+            results = context.search(basedn, generateSearchFilter(username), searchControls);
+            while (results.hasMoreElements()) {
+                final SearchResult result = results.nextElement();
+                users.add(createUser(result));
+            }
+            Control[] contextControls = context.getResponseControls();
+            if (contextControls != null) {
+                for (Control control : contextControls) {
+                    if (control instanceof PagedResultsResponseControl) {
+                        PagedResultsResponseControl prrc = (PagedResultsResponseControl) control;
+                        cookie = prrc.getCookie();
+                    }
+                }
+            } else {
+                s_logger.info("No controls were sent from the ldap server");
+            }
+            context.setRequestControls(new Control[] {new PagedResultsControl(pageSize, cookie, Control.CRITICAL)});
+        } while (cookie != null);
+
+        return users;
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy
index 416c133..51f8e84 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy
@@ -34,7 +34,7 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
 		when: "A user authentications"
         def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null)
 		then: "their authentication fails"
-		result == false
+		result.first() == false
     }
 
     def "Test failed authentication due to ldap bind being unsuccessful"() {
@@ -51,7 +51,7 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
 		def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null)
 
 		then: "their authentication fails"
-		result == false
+		result.first() == false
     }
 
     def "Test failed authentication due to ldap not being configured"() {
@@ -66,7 +66,7 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
 		when: "The user authenticates"
 		def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null)
 		then: "their authentication fails"
-		result == false
+		result.first() == false
     }
 
 	def "Test successful authentication"() {
@@ -83,7 +83,7 @@ class LdapAuthenticatorSpec extends spock.lang.Specification {
 		def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null)
 
 		then: "their authentication passes"
-		result == true
+		result.first() == true
 	}
 
     def "Test that encode doesn't change the input"() {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapContextFactorySpec.groovy
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapContextFactorySpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapContextFactorySpec.groovy
index 0b8f284..2a66528 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapContextFactorySpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapContextFactorySpec.groovy
@@ -53,6 +53,8 @@ class LdapContextFactorySpec extends spock.lang.Specification {
 		ldapConfiguration.getSSLStatus() >> true
 		ldapConfiguration.getTrustStore() >> "/tmp/ldap.ts"
 		ldapConfiguration.getTrustStorePassword() >> "password"
+        ldapConfiguration.getReadTimeout() >> 1000
+        ldapConfiguration.getLdapPageSize() >> 1
 
         username = "rmurphy"
         principal = "cn=" + username + "," + ldapConfiguration.getBaseDn()
@@ -68,15 +70,6 @@ class LdapContextFactorySpec extends spock.lang.Specification {
 		thrown NamingException
     }
 
-    def "Test successful failed connection"() {
-		given: "We have a LdapContextFactory"
-		def ldapContextFactory = Spy(LdapContextFactory, constructorArgs: [ldapConfiguration])
-		when: "Test connection is executed"
-		ldapContextFactory.testConnection(ldapConfiguration.getProviderUrl())
-		then: "An exception is thrown"
-		thrown NamingException
-    }
-
     def "Test successfully binding as a user"() {
 		given: "We have a LdapContextFactory"
 		def ldapContextFactory = new LdapContextFactory(ldapConfiguration)

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy
index b316a80..5805e0b 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy
@@ -29,7 +29,8 @@ import org.apache.cloudstack.context.CallContext;
 
 import com.cloud.user.AccountService;
 import com.cloud.user.UserAccount;
-import com.cloud.user.UserAccountVO;
+import com.cloud.user.UserAccountVO
+import org.apache.cloudstack.ldap.NoLdapUserMatchingQueryException;
 
 import javax.naming.NamingException
 
@@ -38,7 +39,7 @@ class LdapCreateAccountCmdSpec extends spock.lang.Specification {
     def "Test failure to retrive LDAP user"() {
 		given: "We have an LdapManager, AccountService and LdapCreateAccountCmd and LDAP user that doesn't exist"
 		LdapManager ldapManager = Mock(LdapManager)
-		ldapManager.getUser(_) >> { throw new NamingException() }
+		ldapManager.getUser(_) >> { throw new NoLdapUserMatchingQueryException() }
 		AccountService accountService = Mock(AccountService)
 		def ldapCreateAccountCmd = Spy(LdapCreateAccountCmd, constructorArgs: [ldapManager, accountService])
 		ldapCreateAccountCmd.getCurrentContext() >> Mock(CallContext)

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
index 42988e0..ee31720 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy
@@ -16,6 +16,8 @@
 // under the License.
 package groovy.org.apache.cloudstack.ldap
 
+import org.apache.cloudstack.api.command.LDAPConfigCmd
+import org.apache.cloudstack.api.command.LDAPRemoveCmd
 import org.apache.cloudstack.api.command.LdapAddConfigurationCmd
 import org.apache.cloudstack.api.command.LdapCreateAccountCmd
 import org.apache.cloudstack.api.command.LdapDeleteConfigurationCmd
@@ -35,296 +37,298 @@ import com.cloud.utils.Pair
 
 class LdapManagerImplSpec extends spock.lang.Specification {
     def "Test failing of getUser due to bind issue"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapContextFactory.createBindContext() >> { throw new NamingException() }
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "We search for a user but there is a bind issue"
-		ldapManager.getUser("rmurphy")
-		then: "an exception is thrown"
-		thrown NamingException
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext() >> { throw new NoLdapUserMatchingQueryException() }
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "We search for a user but there is a bind issue"
+        ldapManager.getUser("rmurphy")
+        then: "an exception is thrown"
+        thrown NoLdapUserMatchingQueryException
     }
 
     def "Test failing of getUsers due to bind issue"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapContextFactory.createBindContext() >> { throw new NamingException() }
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "We search for a group of users but there is a bind issue"
-		ldapManager.getUsers()
-		then: "An exception is thrown"
-		thrown NoLdapUserMatchingQueryException
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext() >> { throw new NamingException() }
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "We search for a group of users but there is a bind issue"
+        ldapManager.getUsers()
+        then: "An exception is thrown"
+        thrown NoLdapUserMatchingQueryException
     }
 
     def "Test failing of searchUsers due to a failure to bind"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapContextFactory.createBindContext() >> { throw new NamingException() }
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "We search for users"
-		ldapManager.searchUsers("rmurphy")
-		then: "An exception is thrown"
-		thrown NoLdapUserMatchingQueryException
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext() >> { throw new NamingException() }
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "We search for users"
+        ldapManager.searchUsers("rmurphy")
+        then: "An exception is thrown"
+        thrown NoLdapUserMatchingQueryException
     }
 
     def "Test LdapConfigurationResponse generation"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "A ldap configuration response is generated"
-		def result = ldapManager.createLdapConfigurationResponse(new LdapConfigurationVO("localhost", 389))
-		then: "the result of the response should match the given LdapConfigurationVO"
-		result.hostname == "localhost"
-		result.port == 389
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "A ldap configuration response is generated"
+        def result = ldapManager.createLdapConfigurationResponse(new LdapConfigurationVO("localhost", 389))
+        then: "the result of the response should match the given LdapConfigurationVO"
+        result.hostname == "localhost"
+        result.port == 389
     }
 
     def "Test LdapUserResponse generation"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "A ldap user response is generated"
-		def result = ldapManager.createLdapUserResponse(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org",
-		"engineering"))
-		then: "The result of the response should match the given ldap user"
-		result.username == "rmurphy"
-		result.email == "rmurphy@test.com"
-		result.firstname == "Ryan"
-		result.lastname == "Murphy"
-		result.principal == "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org"
-	result.domain == "engineering"
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "A ldap user response is generated"
+        def result = ldapManager.createLdapUserResponse(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org",
+                "engineering"))
+        then: "The result of the response should match the given ldap user"
+        result.username == "rmurphy"
+        result.email == "rmurphy@test.com"
+        result.firstname == "Ryan"
+        result.lastname == "Murphy"
+        result.principal == "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org"
+        result.domain == "engineering"
     }
 
     def "Test success getUsers"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapContextFactory.createBindContext() >> null
-		List<LdapUser> users = new ArrayList<>();
-		users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null))
-		ldapUserManager.getUsers(_) >> users;
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "We search for a group of users"
-		def result = ldapManager.getUsers()
-		then: "A list greater than 0 is returned"
-		result.size() > 0;
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext() >> null
+        List<LdapUser> users = new ArrayList<>();
+        users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null))
+        ldapUserManager.getUsers(_) >> users;
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "We search for a group of users"
+        def result = ldapManager.getUsers()
+        then: "A list greater than 0 is returned"
+        result.size() > 0;
     }
 
     def "Test success of getUser"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapContextFactory.createBindContext() >> null
-		ldapUserManager.getUser(_, _) >> new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null)
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "We search for a user"
-		def result = ldapManager.getUser("rmurphy")
-		then: "The user is returned"
-		result.username == "rmurphy"
-		result.email == "rmurphy@test.com"
-		result.firstname == "Ryan"
-		result.lastname == "Murphy"
-		result.principal == "cn=rmurphy,dc=cloudstack,dc=org"
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext() >> null
+        ldapUserManager.getUser(_, _) >> new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null)
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "We search for a user"
+        def result = ldapManager.getUser("rmurphy")
+        then: "The user is returned"
+        result.username == "rmurphy"
+        result.email == "rmurphy@test.com"
+        result.firstname == "Ryan"
+        result.lastname == "Murphy"
+        result.principal == "cn=rmurphy,dc=cloudstack,dc=org"
     }
 
     def "Test successful closing of context"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "The context is closed"
-		def context = Mock(InitialLdapContext)
-		ldapManager.closeContext(context)
-		then: "The context is null"
-		context.defaultInitCtx == null
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "The context is closed"
+        def context = Mock(InitialLdapContext)
+        ldapManager.closeContext(context)
+        then: "The context is null"
+        context.defaultInitCtx == null
     }
 
     def "Test successful failed result from canAuthenticate due to bad password"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
         def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
         def ldapContextFactory = Mock(LdapContextFactory)
         ldapContextFactory.createUserContext(_, _) >> { throw new NamingException() }
         def ldapUserManager = Mock(LdapUserManager)
         def ldapManager = Spy(LdapManagerImpl, constructorArgs: [ldapConfigurationDao, ldapContextFactory, ldapUserManager])
-	ldapManager.getUser(_) >> { new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) }
-		when: "The user attempts to authenticate with a bad password"
+        ldapManager.getUser(_) >> { new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) }
+        when: "The user attempts to authenticate with a bad password"
         def result = ldapManager.canAuthenticate("rmurphy", "password")
-		then: "The authentication fails"
-		result == false
+        then: "The authentication fails"
+        result == false
     }
 
     def "Test successful failed result from canAuthenticate due to user not found"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		def ldapManager = Spy(LdapManagerImpl, constructorArgs: [ldapConfigurationDao, ldapContextFactory, ldapUserManager])
-		ldapManager.getUser(_) >> { throw new NamingException() }
-		when: "The user attempts to authenticate and the user is not found"
-		def result = ldapManager.canAuthenticate("rmurphy", "password")
-		then: "the authentication fails"
-		result == false
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        def ldapManager = Spy(LdapManagerImpl, constructorArgs: [ldapConfigurationDao, ldapContextFactory, ldapUserManager])
+        ldapManager.getUser(_) >> { throw new NamingException() }
+        when: "The user attempts to authenticate and the user is not found"
+        def result = ldapManager.canAuthenticate("rmurphy", "password")
+        then: "the authentication fails"
+        result == false
     }
 
     def "Test successful failed result from deleteConfiguration due to configuration not existing"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapConfigurationDao.findByHostname(_) >> null
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "A ldap configuration that doesn't exist is deleted"
-		ldapManager.deleteConfiguration("localhost")
-		then: "A exception is thrown"
-		thrown InvalidParameterValueException
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapConfigurationDao.findByHostname(_) >> null
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "A ldap configuration that doesn't exist is deleted"
+        ldapManager.deleteConfiguration("localhost")
+        then: "A exception is thrown"
+        thrown InvalidParameterValueException
     }
 
     def "Test successful failing to close of context"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
         def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
         def ldapContextFactory = Mock(LdapContextFactory)
         def ldapUserManager = Mock(LdapUserManager)
         def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "The context is closed"
+        when: "The context is closed"
         def context = Mock(InitialLdapContext)
         context.close() >> { throw new NamingException() }
         ldapManager.closeContext(context)
-		then: "An exception is thrown"
-		context.defaultInitCtx == null
+        then: "An exception is thrown"
+        context.defaultInitCtx == null
     }
 
     def "Test successful result from canAuthenticate"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		ldapContextFactory.createUserContext(_, _) >> null
-		def ldapUserManager = Mock(LdapUserManager)
-		def ldapManager = Spy(LdapManagerImpl, constructorArgs: [ldapConfigurationDao, ldapContextFactory, ldapUserManager])
-		ldapManager.getUser(_) >> { new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) }
-		when: "A user authenticates"
-		def result = ldapManager.canAuthenticate("rmurphy", "password")
-		then: "The result is true"
-		result == true
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        ldapContextFactory.createUserContext(_, _) >> null
+        def ldapUserManager = Mock(LdapUserManager)
+        def ldapManager = Spy(LdapManagerImpl, constructorArgs: [ldapConfigurationDao, ldapContextFactory, ldapUserManager])
+        ldapManager.getUser(_) >> { new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) }
+        when: "A user authenticates"
+        def result = ldapManager.canAuthenticate("rmurphy", "password")
+        then: "The result is true"
+        result == true
     }
 
     def "Test successful result from deleteConfiguration"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapConfigurationDao.findByHostname(_) >> {
-		    def configuration = new LdapConfigurationVO("localhost", 389)
-		    configuration.setId(0);
-		    return configuration;
-		}
-		ldapConfigurationDao.remove(_) >> null
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "A ldap configuration is deleted"
-		def result = ldapManager.deleteConfiguration("localhost")
-		then: "The deleted configuration is returned"
-		result.hostname == "localhost"
-		result.port == 389
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapConfigurationDao.findByHostname(_) >> {
+            def configuration = new LdapConfigurationVO("localhost", 389)
+            configuration.setId(0);
+            return configuration;
+        }
+        ldapConfigurationDao.remove(_) >> null
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "A ldap configuration is deleted"
+        def result = ldapManager.deleteConfiguration("localhost")
+        then: "The deleted configuration is returned"
+        result.hostname == "localhost"
+        result.port == 389
     }
 
     def "Test successful result from searchUsers"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapContextFactory.createBindContext() >> null;
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext() >> null;
 
-		List<LdapUser> users = new ArrayList<LdapUser>();
-		users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering"))
-		ldapUserManager.getUsers(_, _) >> users;
+        List<LdapUser> users = new ArrayList<LdapUser>();
+        users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering"))
+        ldapUserManager.getUsers(_, _) >> users;
 
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "We search for users"
-		def result = ldapManager.searchUsers("rmurphy");
-		then: "A list of atleast 1 is returned"
-		result.size() > 0;
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "We search for users"
+        def result = ldapManager.searchUsers("rmurphy");
+        then: "A list of atleast 1 is returned"
+        result.size() > 0;
     }
 
     def "Test successfully addConfiguration"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapContextFactory.createBindContext(_) >> null
-		ldapConfigurationDao.persist(_) >> null
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "A ldap configuration is added"
-		def result = ldapManager.addConfiguration("localhost", 389)
-		then: "the resulting object contain the given hostname and port"
-		result.hostname == "localhost"
-		result.port == 389
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext(_) >> null
+        ldapConfigurationDao.persist(_) >> null
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "A ldap configuration is added"
+        def result = ldapManager.addConfiguration("localhost", 389)
+        then: "the resulting object contain the given hostname and port"
+        result.hostname == "localhost"
+        result.port == 389
     }
 
     def "Test that addConfiguration fails when a binding fails"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapContextFactory.createBindContext(_) >> { throw new NamingException() }
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "A configuration is added that can not be binded"
-		ldapManager.addConfiguration("localhost", 389)
-		then: "An exception is thrown"
-		thrown InvalidParameterValueException
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext(_) >> { throw new NamingException() }
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "A configuration is added that can not be binded"
+        ldapManager.addConfiguration("localhost", 389)
+        then: "An exception is thrown"
+        thrown InvalidParameterValueException
     }
 
     def "Test that addConfiguration fails when a duplicate configuration exists"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		ldapConfigurationDao.findByHostname(_) >> new LdapConfigurationVO("localhost", 389)
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "a configuration that already exists is added"
-		ldapManager.addConfiguration("localhost", 389)
-		then: "An exception is thrown"
-		thrown InvalidParameterValueException
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapConfigurationDao.findByHostname(_) >> new LdapConfigurationVO("localhost", 389)
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "a configuration that already exists is added"
+        ldapManager.addConfiguration("localhost", 389)
+        then: "An exception is thrown"
+        thrown InvalidParameterValueException
     }
 
     def supportedLdapCommands() {
-	List<Class<?>> cmdList = new ArrayList<Class<?>>();
-	cmdList.add(LdapUserSearchCmd.class);
-	cmdList.add(LdapListUsersCmd.class);
-	cmdList.add(LdapAddConfigurationCmd.class);
-	cmdList.add(LdapDeleteConfigurationCmd.class);
-	cmdList.add(LdapListConfigurationCmd.class);
-	cmdList.add(LdapCreateAccountCmd.class);
-	cmdList.add(LdapImportUsersCmd.class);
-	return cmdList
+        List<Class<?>> cmdList = new ArrayList<Class<?>>();
+        cmdList.add(LdapUserSearchCmd.class);
+        cmdList.add(LdapListUsersCmd.class);
+        cmdList.add(LdapAddConfigurationCmd.class);
+        cmdList.add(LdapDeleteConfigurationCmd.class);
+        cmdList.add(LdapListConfigurationCmd.class);
+        cmdList.add(LdapCreateAccountCmd.class);
+        cmdList.add(LdapImportUsersCmd.class);
+        cmdList.add(LDAPConfigCmd.class);
+        cmdList.add(LDAPRemoveCmd.class);
+        return cmdList
     }
 
     def "Test that getCommands isn't empty"() {
-		given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-	final List<Class<?>> cmdList = supportedLdapCommands()
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "Get commands is called"
-		def result = ldapManager.getCommands()
-		then: "it must return all the commands"
-		result.size() > 0
-	result == cmdList
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        final List<Class<?>> cmdList = supportedLdapCommands()
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "Get commands is called"
+        def result = ldapManager.getCommands()
+        then: "it must return all the commands"
+        result.size() > 0
+        result == cmdList
     }
 
     def "Testing of listConfigurations"() {
-	given:  "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
         def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
         def ldapContextFactory = Mock(LdapContextFactory)
         def ldapUserManager = Mock(LdapUserManager)
@@ -334,42 +338,42 @@ class LdapManagerImplSpec extends spock.lang.Specification {
         configurations.set(ldapConfigurationList, ldapConfigurationList.size())
         ldapConfigurationDao.searchConfigurations(_, _) >> configurations
         def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-	when: "A request for configurations is made"
+        when: "A request for configurations is made"
         def result = ldapManager.listConfigurations(new LdapListConfigurationCmd())
-		then: "Then atleast 1 ldap configuration is returned"
-		result.second() > 0
+        then: "Then atleast 1 ldap configuration is returned"
+        result.second() > 0
     }
 
-	def "Testing of isLdapEnabled"() {
-		given:  "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-		def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-		def ldapContextFactory = Mock(LdapContextFactory)
-		def ldapUserManager = Mock(LdapUserManager)
-		List<LdapConfigurationVO> ldapConfigurationList = new ArrayList()
-		ldapConfigurationList.add(new LdapConfigurationVO("localhost", 389))
-		Pair<List<LdapConfigurationVO>, Integer> configurations = new Pair<List<LdapConfigurationVO>, Integer>();
-		configurations.set(ldapConfigurationList, ldapConfigurationList.size())
-		ldapConfigurationDao.searchConfigurations(_, _) >> configurations
-		def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-		when: "A request to find out is ldap enabled"
-		def result = ldapManager.isLdapEnabled();
-		then: "true is returned because a configuration was found"
-		result == true;
-	}
+    def "Testing of isLdapEnabled"() {
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        List<LdapConfigurationVO> ldapConfigurationList = new ArrayList()
+        ldapConfigurationList.add(new LdapConfigurationVO("localhost", 389))
+        Pair<List<LdapConfigurationVO>, Integer> configurations = new Pair<List<LdapConfigurationVO>, Integer>();
+        configurations.set(ldapConfigurationList, ldapConfigurationList.size())
+        ldapConfigurationDao.searchConfigurations(_, _) >> configurations
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "A request to find out is ldap enabled"
+        def result = ldapManager.isLdapEnabled();
+        then: "true is returned because a configuration was found"
+        result == true;
+    }
 
     def "Test success getUsersInGroup"() {
-	given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
-	def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
-	def ldapContextFactory = Mock(LdapContextFactory)
-	def ldapUserManager = Mock(LdapUserManager)
-	ldapContextFactory.createBindContext() >> null
-	List<LdapUser> users = new ArrayList<>();
-	users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", "engineering"))
-	ldapUserManager.getUsersInGroup("engineering", _) >> users;
-	def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
-	when: "We search for a group of users"
-	def result = ldapManager.getUsersInGroup("engineering")
-	then: "A list greater of size one is returned"
-	result.size() == 1;
+        given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager"
+        def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl)
+        def ldapContextFactory = Mock(LdapContextFactory)
+        def ldapUserManager = Mock(LdapUserManager)
+        ldapContextFactory.createBindContext() >> null
+        List<LdapUser> users = new ArrayList<>();
+        users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", "engineering"))
+        ldapUserManager.getUsersInGroup("engineering", _) >> users;
+        def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManager)
+        when: "We search for a group of users"
+        def result = ldapManager.getUsersInGroup("engineering")
+        then: "A list greater of size one is returned"
+        result.size() == 1;
     }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/14f3ad55/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerSpec.groovy
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerSpec.groovy
index c8edb3f..d1f3667 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerSpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerSpec.groovy
@@ -26,6 +26,7 @@ import javax.naming.directory.Attributes
 import javax.naming.directory.InitialDirContext
 import javax.naming.directory.SearchControls
 import javax.naming.directory.SearchResult
+import javax.naming.ldap.InitialLdapContext
 import javax.naming.ldap.LdapContext
 
 class LdapUserManagerSpec extends spock.lang.Specification {
@@ -169,6 +170,8 @@ class LdapUserManagerSpec extends spock.lang.Specification {
         ldapConfiguration.getCommonNameAttribute() >> "cn"
         ldapConfiguration.getGroupObject() >> "groupOfUniqueNames"
         ldapConfiguration.getGroupUniqueMemeberAttribute() >> "uniquemember"
+        ldapConfiguration.getLdapPageSize() >> 1
+        ldapConfiguration.getReadTimeout() >> 1000
 
         username = "rmurphy"
         email = "rmurphy@test.com"
@@ -217,7 +220,7 @@ class LdapUserManagerSpec extends spock.lang.Specification {
         result.size() == 1
     }
 
-    def "Test successfully returning a NamingEnumeration from searchUsers"() {
+    def "Test successfully returning a ldap user from searchUsers"() {
         given: "We have a LdapUserManager"
         def userManager = new LdapUserManager(ldapConfiguration)
 
@@ -225,7 +228,7 @@ class LdapUserManagerSpec extends spock.lang.Specification {
         def result = userManager.searchUsers(createContext())
 
         then: "A list of users are returned."
-        result.next().getName() + "," + ldapConfiguration.getBaseDn() == principal
+        result.first().getPrincipal() == principal
     }
 
     def "Test successfully returning an Ldap user from a get user request"() {
@@ -324,7 +327,7 @@ class LdapUserManagerSpec extends spock.lang.Specification {
         def ldapUserManager = new LdapUserManager(ldapconfig)
 
         when: "A request for search users is made"
-        def result = ldapUserManager.searchUsers(new InitialDirContext())
+        def result = ldapUserManager.searchUsers(new InitialLdapContext())
 
         then: "An exception with no basedn defined is returned"
         def e = thrown(IllegalArgumentException)


Mime
View raw message