Fixed bug: CLOUDSTACK-7214 added a config for ldap connection read timeout.

plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapConfiguration.java

@@ -22,13 +22,18 @@ import javax.inject.Inject;
 import javax.naming.directory.SearchControls;
 
 import org.apache.cloudstack.api.command.LdapListConfigurationCmd;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 
 import com.cloud.utils.Pair;
 
-public class LdapConfiguration {
+public class LdapConfiguration implements Configurable{
     private final static String factory = "com.sun.jndi.ldap.LdapCtxFactory";
 
+    private static final ConfigKey<Long> ldapReadTimeout = new ConfigKey<Long>(Long.class, "ldap.read.timeout", "Secure", "1000",
+        "LDAP connection Timeout in milli sec", true, ConfigKey.Scope.Global, 1l);
+
     private final static int scope = SearchControls.SUBTREE_SCOPE;
 
     @Inject
@@ -148,4 +153,18 @@ public class LdapConfiguration {
     public String getCommonNameAttribute() {
         return "cn";
     }
+
+    public Long getReadTimeout() {
+        return ldapReadTimeout.value();
+    }
+
+    @Override
+    public String getConfigComponentName() {
+        return LdapConfiguration.class.getSimpleName();
+    }
+
+    @Override
+    public ConfigKey<?>[] getConfigKeys() {
+        return new ConfigKey<?>[] {ldapReadTimeout};
+    }
 }

plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapContextFactory.java

@@ -82,7 +82,7 @@ public class LdapContextFactory {
 
         environment.put(Context.INITIAL_CONTEXT_FACTORY, factory);
         environment.put(Context.PROVIDER_URL, url);
-        environment.put("com.sun.jndi.ldap.read.timeout", "500");
+        environment.put("com.sun.jndi.ldap.read.timeout", _ldapConfiguration.getReadTimeout().toString());
         environment.put("com.sun.jndi.ldap.connect.pool", "true");
 
         enableSSL(environment);

plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapConfigurationSpec.groovy

@@ -16,241 +16,268 @@
 // under the License.
 package groovy.org.apache.cloudstack.ldap
 
+import org.apache.cloudstack.framework.config.ConfigKey
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import com.cloud.utils.Pair
 import org.apache.cloudstack.api.ServerApiException
+import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl
+import org.apache.cloudstack.framework.config.impl.ConfigurationVO
 import org.apache.cloudstack.ldap.LdapConfiguration
 import org.apache.cloudstack.ldap.LdapConfigurationVO
 import org.apache.cloudstack.ldap.LdapManager
+import org.apache.cxf.common.util.StringUtils
 
 import javax.naming.directory.SearchControls
 
 class LdapConfigurationSpec extends spock.lang.Specification {
     def "Test that getAuthentication returns none"() {
-		given: "We have a ConfigDao, LdapManager and LdapConfiguration"
+        given: "We have a ConfigDao, LdapManager and LdapConfiguration"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get authentication is called"
+        when: "Get authentication is called"
-		String authentication = ldapConfiguration.getAuthentication()
+        String authentication = ldapConfiguration.getAuthentication()
-		then: "none should be returned"
+        then: "none should be returned"
-		authentication == "none"
+        authentication == "none"
     }
 
     def "Test that getAuthentication returns simple"() {
-	given: "We have a configDao, LdapManager and LdapConfiguration with bind principle and password set"
+        given: "We have a configDao, LdapManager and LdapConfiguration with bind principle and password set"
         def configDao = Mock(ConfigurationDao)
         def ldapManager = Mock(LdapManager)
         def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
         configDao.getValue("ldap.bind.password") >> "password"
         configDao.getValue("ldap.bind.principal") >> "cn=rmurphy,dc=cloudstack,dc=org"
-		when: "Get authentication is called"
+        when: "Get authentication is called"
         String authentication = ldapConfiguration.getAuthentication()
-		then: "authentication should be set to simple"
+        then: "authentication should be set to simple"
-		authentication == "simple"
+        authentication == "simple"
     }
 
     def "Test that getBaseDn returns dc=cloudstack,dc=org"() {
-		given: "We have a ConfigDao, LdapManager and ldapConfiguration with a baseDn value set."
+        given: "We have a ConfigDao, LdapManager and ldapConfiguration with a baseDn value set."
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		configDao.getValue("ldap.basedn") >> "dc=cloudstack,dc=org"
+        configDao.getValue("ldap.basedn") >> "dc=cloudstack,dc=org"
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get basedn is called"
+        when: "Get basedn is called"
-		String baseDn = ldapConfiguration.getBaseDn();
+        String baseDn = ldapConfiguration.getBaseDn();
-		then: "The set baseDn should be returned"
+        then: "The set baseDn should be returned"
-		baseDn == "dc=cloudstack,dc=org"
+        baseDn == "dc=cloudstack,dc=org"
     }
 
     def "Test that getEmailAttribute returns mail"() {
-		given: "Given that we have a ConfigDao, LdapManager and LdapConfiguration"
+        given: "Given that we have a ConfigDao, LdapManager and LdapConfiguration"
         def configDao = Mock(ConfigurationDao)
         configDao.getValue("ldap.email.attribute") >> "mail"
         def ldapManager = Mock(LdapManager)
         def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get Email Attribute is called"
+        when: "Get Email Attribute is called"
         String emailAttribute = ldapConfiguration.getEmailAttribute()
-		then: "mail should be returned"
+        then: "mail should be returned"
-		emailAttribute == "mail"
+        emailAttribute == "mail"
     }
 
     def "Test that getFactory returns com.sun.jndi.ldap.LdapCtxFactory"() {
-		given: "We have a ConfigDao, LdapManager and LdapConfiguration"
+        given: "We have a ConfigDao, LdapManager and LdapConfiguration"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get Factory is scalled"
+        when: "Get Factory is scalled"
-		String factory = ldapConfiguration.getFactory();
+        String factory = ldapConfiguration.getFactory();
-		then: "com.sun.jndi.ldap.LdapCtxFactory is returned"
+        then: "com.sun.jndi.ldap.LdapCtxFactory is returned"
-		factory == "com.sun.jndi.ldap.LdapCtxFactory"
+        factory == "com.sun.jndi.ldap.LdapCtxFactory"
     }
 
     def "Test that getFirstnameAttribute returns givenname"() {
-		given: "We have a ConfigDao, LdapManager and LdapConfiguration"
+        given: "We have a ConfigDao, LdapManager and LdapConfiguration"
         def configDao = Mock(ConfigurationDao)
         configDao.getValue("ldap.firstname.attribute") >> "givenname"
         def ldapManager = Mock(LdapManager)
         def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get firstname attribute is called"
+        when: "Get firstname attribute is called"
         String firstname = ldapConfiguration.getFirstnameAttribute()
-		then: "givennam should be returned"
+        then: "givennam should be returned"
         firstname == "givenname"
     }
 
     def "Test that getLastnameAttribute returns givenname"() {
-		given: "We have a ConfigDao, LdapManager and LdapConfiguration"
+        given: "We have a ConfigDao, LdapManager and LdapConfiguration"
         def configDao = Mock(ConfigurationDao)
         configDao.getValue("ldap.lastname.attribute") >> "sn"
         def ldapManager = Mock(LdapManager)
         def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get Lastname Attribute is scalled "
+        when: "Get Lastname Attribute is scalled "
         String lastname = ldapConfiguration.getLastnameAttribute()
-		then: "sn should be returned"
+        then: "sn should be returned"
-		lastname == "sn"
+        lastname == "sn"
     }
 
     def "Test that getReturnAttributes returns the correct data"() {
-		given: "We have a ConfigDao, LdapManager and LdapConfiguration"
+        given: "We have a ConfigDao, LdapManager and LdapConfiguration"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
         configDao.getValue("ldap.firstname.attribute") >> "givenname"
         configDao.getValue("ldap.lastname.attribute") >> "sn"
         configDao.getValue("ldap.username.attribute") >> "uid"
         configDao.getValue("ldap.email.attribute") >> "mail"
         def ldapManager = Mock(LdapManager)
         def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get return attributes is called"
+        when: "Get return attributes is called"
         String[] returnAttributes = ldapConfiguration.getReturnAttributes()
-		then: "An array containing uid, mail, givenname, sn and cn is returned"
+        then: "An array containing uid, mail, givenname, sn and cn is returned"
-	returnAttributes == ["uid", "mail", "givenname", "sn", "cn"]
+        returnAttributes == ["uid", "mail", "givenname", "sn", "cn"]
     }
 
     def "Test that getScope returns SearchControls.SUBTREE_SCOPE"() {
-		given: "We have ConfigDao, LdapManager and LdapConfiguration"
+        given: "We have ConfigDao, LdapManager and LdapConfiguration"
         def configDao = Mock(ConfigurationDao)
         def ldapManager = Mock(LdapManager)
         def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get scope is called"
+        when: "Get scope is called"
         int scope = ldapConfiguration.getScope()
-		then: "SearchControls.SUBTRE_SCOPE should be returned"
+        then: "SearchControls.SUBTRE_SCOPE should be returned"
-		scope == SearchControls.SUBTREE_SCOPE;
+        scope == SearchControls.SUBTREE_SCOPE;
     }
 
     def "Test that getUsernameAttribute returns uid"() {
-		given: "We have ConfigDao, LdapManager and LdapConfiguration"
+        given: "We have ConfigDao, LdapManager and LdapConfiguration"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		configDao.getValue("ldap.username.attribute") >> "uid"
+        configDao.getValue("ldap.username.attribute") >> "uid"
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get Username Attribute is called"
+        when: "Get Username Attribute is called"
-		String usernameAttribute = ldapConfiguration.getUsernameAttribute()
+        String usernameAttribute = ldapConfiguration.getUsernameAttribute()
-		then: "uid should be returned"
+        then: "uid should be returned"
-		usernameAttribute == "uid"
+        usernameAttribute == "uid"
     }
 
     def "Test that getUserObject returns inetOrgPerson"() {
-		given: "We have a ConfigDao, LdapManager and LdapConfiguration"
+        given: "We have a ConfigDao, LdapManager and LdapConfiguration"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		configDao.getValue("ldap.user.object") >> "inetOrgPerson"
+        configDao.getValue("ldap.user.object") >> "inetOrgPerson"
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        def ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
-		when: "Get user object is called"
+        when: "Get user object is called"
-		String userObject = ldapConfiguration.getUserObject()
+        String userObject = ldapConfiguration.getUserObject()
-		then: "inetOrgPerson is returned"
+        then: "inetOrgPerson is returned"
-		userObject == "inetOrgPerson"
+        userObject == "inetOrgPerson"
     }
 
     def "Test that providerUrl successfully returns a URL when a configuration is available"() {
-		given: "We have a ConfigDao, LdapManager, LdapConfiguration"
+        given: "We have a ConfigDao, LdapManager, LdapConfiguration"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		List<LdapConfigurationVO> ldapConfigurationList = new ArrayList()
+        List<LdapConfigurationVO> ldapConfigurationList = new ArrayList()
-		ldapConfigurationList.add(new LdapConfigurationVO("localhost", 389))
+        ldapConfigurationList.add(new LdapConfigurationVO("localhost", 389))
-		Pair<List<LdapConfigurationVO>, Integer> result = new Pair<List<LdapConfigurationVO>, Integer>();
+        Pair<List<LdapConfigurationVO>, Integer> result = new Pair<List<LdapConfigurationVO>, Integer>();
-		result.set(ldapConfigurationList, ldapConfigurationList.size())
+        result.set(ldapConfigurationList, ldapConfigurationList.size())
-		ldapManager.listConfigurations(_) >> result
+        ldapManager.listConfigurations(_) >> result
 
-		LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
 
-		when: "A request is made to get the providerUrl"
+        when: "A request is made to get the providerUrl"
-		String providerUrl = ldapConfiguration.getProviderUrl()
+        String providerUrl = ldapConfiguration.getProviderUrl()
 
-		then: "The providerUrl should be given."
+        then: "The providerUrl should be given."
-		providerUrl == "ldap://localhost:389"
+        providerUrl == "ldap://localhost:389"
     }
 
-	def "Test that get search group principle returns successfully"() {
+    def "Test that get search group principle returns successfully"() {
-		given: "We have a ConfigDao with a value for ldap.search.group.principle and an LdapConfiguration"
+        given: "We have a ConfigDao with a value for ldap.search.group.principle and an LdapConfiguration"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		configDao.getValue("ldap.search.group.principle") >> "cn=cloudstack,cn=users,dc=cloudstack,dc=org"
+        configDao.getValue("ldap.search.group.principle") >> "cn=cloudstack,cn=users,dc=cloudstack,dc=org"
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
 
-		when: "A request is made to get the search group principle"
+        when: "A request is made to get the search group principle"
-		String result = ldapConfiguration.getSearchGroupPrinciple();
+        String result = ldapConfiguration.getSearchGroupPrinciple();
 
-		then: "The result holds the same value configDao did"
+        then: "The result holds the same value configDao did"
-		result == "cn=cloudstack,cn=users,dc=cloudstack,dc=org"
+        result == "cn=cloudstack,cn=users,dc=cloudstack,dc=org"
-	}
+    }
 
-	def "Test that getTrustStorePassword resopnds"() {
+    def "Test that getTrustStorePassword resopnds"() {
-		given: "We have a ConfigDao with a value for truststore password"
+        given: "We have a ConfigDao with a value for truststore password"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		configDao.getValue("ldap.truststore.password") >> "password"
+        configDao.getValue("ldap.truststore.password") >> "password"
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
 
-		when: "A request is made to get the truststore password"
+        when: "A request is made to get the truststore password"
-		String result = ldapConfiguration.getTrustStorePassword()
+        String result = ldapConfiguration.getTrustStorePassword()
 
-		then: "The result is password"
+        then: "The result is password"
-		result == "password";
+        result == "password";
-	}
+    }
 
-	def "Test that getSSLStatus can be true"() {
+    def "Test that getSSLStatus can be true"() {
-		given: "We have a ConfigDao with values for truststore and truststore password set"
+        given: "We have a ConfigDao with values for truststore and truststore password set"
-		def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-		configDao.getValue("ldap.truststore") >> "/tmp/ldap.ts"
+        configDao.getValue("ldap.truststore") >> "/tmp/ldap.ts"
-		configDao.getValue("ldap.truststore.password") >> "password"
+        configDao.getValue("ldap.truststore.password") >> "password"
-		def ldapManager = Mock(LdapManager)
+        def ldapManager = Mock(LdapManager)
-		LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
+        LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManager)
 
-		when: "A request is made to get the status of SSL"
+        when: "A request is made to get the status of SSL"
-		boolean result = ldapConfiguration.getSSLStatus();
+        boolean result = ldapConfiguration.getSSLStatus();
 
-		then: "The response should be true"
+        then: "The response should be true"
-		result == true
+        result == true
-	}
+    }
 
     def "Test getgroupobject"() {
-	given: "We have configdao for ldap group object"
+        given: "We have configdao for ldap group object"
-	def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-	configDao.getValue("ldap.group.object") >> groupObject
+        configDao.getValue("ldap.group.object") >> groupObject
 
-	def ldapManger = Mock(LdapManager)
+        def ldapManger = Mock(LdapManager)
-	LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManger)
+        LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManger)
-	def expectedResult = groupObject == null ? "groupOfUniqueNames" : groupObject
+        def expectedResult = groupObject == null ? "groupOfUniqueNames" : groupObject
 
-	def result = ldapConfiguration.getGroupObject()
+        def result = ldapConfiguration.getGroupObject()
-	expect:
+        expect:
-	result == expectedResult
+        result == expectedResult
-	where:
+        where:
-	groupObject << [null, "", "groupOfUniqueNames"]
+        groupObject << [null, "", "groupOfUniqueNames"]
     }
 
     def "Test getGroupUniqueMemeberAttribute"() {
-	given: "We have configdao for ldap group object"
+        given: "We have configdao for ldap group object"
-	def configDao = Mock(ConfigurationDao)
+        def configDao = Mock(ConfigurationDao)
-	configDao.getValue("ldap.group.user.uniquemember") >> groupObject
+        configDao.getValue("ldap.group.user.uniquemember") >> groupObject
 
-	def ldapManger = Mock(LdapManager)
+        def ldapManger = Mock(LdapManager)
-	LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManger)
+        LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManger)
-	def expectedResult = groupObject == null ? "uniquemember" : groupObject
+        def expectedResult = groupObject == null ? "uniquemember" : groupObject
 
-	def result = ldapConfiguration.getGroupUniqueMemeberAttribute()
+        def result = ldapConfiguration.getGroupUniqueMemeberAttribute()
-	expect:
+        expect:
-	result == expectedResult
+        result == expectedResult
-	where:
+        where:
-	groupObject << [null, "", "uniquemember"]
+        groupObject << [null, "", "uniquemember"]
+    }
+
+    def "Test getReadTimeout"() {
+        given: "We have configdao for ldap group object"
+        def configDao = Mock(ConfigurationDao)
+        ConfigurationVO configurationVo = new ConfigurationVO("ldap.read.timeout", LdapConfiguration.ldapReadTimeout);
+        configurationVo.setValue(timeout)
+        configDao.findById("ldap.read.timeout") >> configurationVo
+
+        def configDepotImpl = Mock(ConfigDepotImpl)
+        configDepotImpl.global() >> configDao
+        ConfigKey.init(configDepotImpl)
+
+        def ldapManger = Mock(LdapManager)
+        LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapManger)
+
+        def expected = timeout == null ? 1000 : timeout.toLong() //1000 is the default value
+
+        def result = ldapConfiguration.getReadTimeout()
+        expect:
+        result == expected
+        where:
+        timeout << ["1000000", "1000", null]
     }
 
 }