cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From raj...@apache.org
Subject [4/5] git commit: updated refs/heads/master to 7febdb5
Date Fri, 24 Jul 2015 05:29:56 GMT
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/master
Commit: 96cf0325e2b9542c18fde6f2c4f7191e6406cdb6
Parents: d42173a
Author: Rajani Karuturi <rajanikaruturi@gmail.com>
Authored: Thu Jul 23 15:21:59 2015 +0530
Committer: Rajani Karuturi <rajanikaruturi@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]
     }
 }


Mime
View raw message