You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by wi...@apache.org on 2015/07/27 10:43:25 UTC
[37/50] [abbrv] git commit: updated refs/heads/reporter to 3e1816d
CLOUDSTACK-8596 addressed review comments
In LdapUserManagerFactory moved the beans to a map
used a Enum for LdapProvider and made the corresponding changes in
LdapConfiguration and the callers.
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/96cf0325
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/96cf0325
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/96cf0325
Branch: refs/heads/reporter
Commit: 96cf0325e2b9542c18fde6f2c4f7191e6406cdb6
Parents: d42173a
Author: Rajani Karuturi <ra...@gmail.com>
Authored: Thu Jul 23 15:21:59 2015 +0530
Committer: Rajani Karuturi <ra...@gmail.com>
Committed: Thu Jul 23 15:21:59 2015 +0530
----------------------------------------------------------------------
.../cloudstack/ldap/ADLdapUserManagerImpl.java | 15 ++++----
.../cloudstack/ldap/LdapConfiguration.java | 11 +++++-
.../apache/cloudstack/ldap/LdapUserManager.java | 4 +++
.../cloudstack/ldap/LdapUserManagerFactory.java | 37 ++++++++++++--------
.../ldap/LdapConfigurationSpec.groovy | 6 ++--
.../ldap/LdapUserManagerFactorySpec.groovy | 5 +--
6 files changed, 52 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/96cf0325/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
index e5e20c5..50f1fa0 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java
@@ -32,21 +32,22 @@ import org.apache.log4j.Logger;
public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl implements LdapUserManager {
public static final Logger s_logger = Logger.getLogger(ADLdapUserManagerImpl.class.getName());
+ private static final String MICROSOFT_AD_NESTED_MEMBERS_FILTER = "memberOf:1.2.840.113556.1.4.1941";
@Override
public List<LdapUser> getUsersInGroup(String groupName, LdapContext context) throws NamingException {
- final SearchControls searchControls = new SearchControls();
- searchControls.setSearchScope(_ldapConfiguration.getScope());
- searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
+ if (StringUtils.isBlank(groupName)) {
+ throw new IllegalArgumentException("ldap group name cannot be blank");
+ }
String basedn = _ldapConfiguration.getBaseDn();
if (StringUtils.isBlank(basedn)) {
throw new IllegalArgumentException("ldap basedn is not configured");
}
- if (StringUtils.isBlank(groupName)) {
- throw new IllegalArgumentException("ldap group name cannot be blank");
- }
+ final SearchControls searchControls = new SearchControls();
+ searchControls.setSearchScope(_ldapConfiguration.getScope());
+ searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
NamingEnumeration<SearchResult> results = context.search(basedn, generateADGroupSearchFilter(groupName), searchControls);
final List<LdapUser> users = new ArrayList<LdapUser>();
@@ -65,7 +66,7 @@ public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl implements Ld
final StringBuilder memberOfFilter = new StringBuilder();
String groupCnName = _ldapConfiguration.getCommonNameAttribute() + "=" +groupName + "," + _ldapConfiguration.getBaseDn();
- memberOfFilter.append("(memberOf:1.2.840.113556.1.4.1941:=");
+ memberOfFilter.append("(" + MICROSOFT_AD_NESTED_MEMBERS_FILTER + ":=");
memberOfFilter.append(groupCnName);
memberOfFilter.append(")");
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/96cf0325/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 f247f40..a64899a 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
@@ -167,7 +167,16 @@ public class LdapConfiguration implements Configurable{
return ldapPageSize.value();
}
- public String getLdapProvider() { return ldapProvider.value();}
+ public LdapUserManager.Provider getLdapProvider() {
+ LdapUserManager.Provider provider;
+ try {
+ provider = LdapUserManager.Provider.valueOf(ldapProvider.value().toUpperCase());
+ } catch (IllegalArgumentException ex) {
+ //openldap is the default
+ provider = LdapUserManager.Provider.OPENLDAP;
+ }
+ return provider;
+ }
@Override
public String getConfigComponentName() {
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/96cf0325/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 28fc74e..c1bfe74 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
@@ -26,6 +26,10 @@ import javax.naming.ldap.LdapContext;
public interface LdapUserManager {
+ public enum Provider {
+ MICROSOFTAD, OPENLDAP;
+ }
+
public LdapUser getUser(final String username, final LdapContext context) throws NamingException, IOException;
public List<LdapUser> getUsers(final LdapContext context) throws NamingException, IOException;
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/96cf0325/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java
index 35652fb..b7414c7 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManagerFactory.java
@@ -24,28 +24,37 @@ import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
+import java.util.HashMap;
+import java.util.Map;
+
public class LdapUserManagerFactory implements ApplicationContextAware {
+
+
public static final Logger s_logger = Logger.getLogger(LdapUserManagerFactory.class.getName());
- private static LdapUserManager adUserManager;
- private static LdapUserManager openLdapUserManager;
+ private static Map<LdapUserManager.Provider, LdapUserManager> ldapUserManagerMap = new HashMap<>();
static ApplicationContext applicationCtx;
- public LdapUserManager getInstance(String id) {
- if ("microsoftad".equalsIgnoreCase(id)) {
- if (adUserManager == null) {
- adUserManager = new ADLdapUserManagerImpl();
- applicationCtx.getAutowireCapableBeanFactory().autowireBeanProperties(adUserManager, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, true);
+ public LdapUserManager getInstance(LdapUserManager.Provider provider) {
+ LdapUserManager ldapUserManager;
+ if (provider == LdapUserManager.Provider.MICROSOFTAD) {
+ ldapUserManager = ldapUserManagerMap.get(LdapUserManager.Provider.MICROSOFTAD);
+ if (ldapUserManager == null) {
+ ldapUserManager = new ADLdapUserManagerImpl();
+ applicationCtx.getAutowireCapableBeanFactory().autowireBeanProperties(ldapUserManager, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, true);
+ ldapUserManagerMap.put(LdapUserManager.Provider.MICROSOFTAD, ldapUserManager);
+ }
+ } else {
+ //defaults to opendldap
+ ldapUserManager = ldapUserManagerMap.get(LdapUserManager.Provider.OPENLDAP);
+ if (ldapUserManager == null) {
+ ldapUserManager = new OpenLdapUserManagerImpl();
+ applicationCtx.getAutowireCapableBeanFactory().autowireBeanProperties(ldapUserManager, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, true);
+ ldapUserManagerMap.put(LdapUserManager.Provider.OPENLDAP, ldapUserManager);
}
- return adUserManager;
- }
- //defaults to opendldap
- if (openLdapUserManager == null) {
- openLdapUserManager = new OpenLdapUserManagerImpl();
- applicationCtx.getAutowireCapableBeanFactory().autowireBeanProperties(openLdapUserManager, AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE, true);
}
- return openLdapUserManager;
+ return ldapUserManager;
}
@Override
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/96cf0325/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapConfigurationSpec.groovy
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapConfigurationSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapConfigurationSpec.groovy
index a4cf3c2..4f93adf 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapConfigurationSpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapConfigurationSpec.groovy
@@ -25,6 +25,7 @@ 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.cloudstack.ldap.LdapUserManager
import org.apache.cloudstack.ldap.dao.LdapConfigurationDao
import org.apache.cxf.common.util.StringUtils
@@ -295,13 +296,14 @@ class LdapConfigurationSpec extends spock.lang.Specification {
def ldapConfigurationDao = Mock(LdapConfigurationDao)
LdapConfiguration ldapConfiguration = new LdapConfiguration(configDao, ldapConfigurationDao)
- def expected = provider == null ? "openldap" : provider //"openldap" is the default value
+ def expected = provider.equalsIgnoreCase("microsoftad") ? LdapUserManager.Provider.MICROSOFTAD : LdapUserManager.Provider.OPENLDAP //"openldap" is the default value
def result = ldapConfiguration.getLdapProvider()
expect:
+ println "asserting for provider configuration: " + provider
result == expected
where:
- provider << ["openldap", "microsoftad", null, "xyz"]
+ provider << ["openldap", "microsoftad", "", " ", "xyz", "MicrosoftAd", "OpenLdap", "MicrosoftAD"]
}
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/96cf0325/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerFactorySpec.groovy
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerFactorySpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerFactorySpec.groovy
index 7615f9d..ca423d3 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerFactorySpec.groovy
+++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserManagerFactorySpec.groovy
@@ -19,6 +19,7 @@
package groovy.org.apache.cloudstack.ldap
import org.apache.cloudstack.ldap.ADLdapUserManagerImpl
+import org.apache.cloudstack.ldap.LdapUserManager
import org.apache.cloudstack.ldap.LdapUserManagerFactory
import org.apache.cloudstack.ldap.OpenLdapUserManagerImpl
import org.springframework.beans.factory.config.AutowireCapableBeanFactory
@@ -42,7 +43,7 @@ class LdapUserManagerFactorySpec extends spock.lang.Specification {
def result = ldapUserManagerFactory.getInstance(id);
def expected;
- if(id == "microsoftad") {
+ if(id == LdapUserManager.Provider.MICROSOFTAD) {
expected = ADLdapUserManagerImpl.class;
} else {
expected = OpenLdapUserManagerImpl.class;
@@ -51,6 +52,6 @@ class LdapUserManagerFactorySpec extends spock.lang.Specification {
expect:
assert result.class.is(expected)
where:
- id << ["openldap", "microsoftad", null, "xyz"]
+ id << [LdapUserManager.Provider.MICROSOFTAD, LdapUserManager.Provider.OPENLDAP, null]
}
}