From 5fa2d1c7ca26d199f0dcd499db8db245c2593a5b Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Wed, 30 Jul 2014 11:56:25 +0530 Subject: [PATCH] Fixed Bug: CLOUDSTACK-7200 [LDAP] importUsersCmd for a group fails incase any member of a group is not an user --- .../cloudstack/ldap/LdapUserManager.java | 8 +++- .../ldap/LdapUserManagerSpec.groovy | 45 +++++++++++++++++-- .../com/cloud/user/AccountManagerImpl.java | 7 +-- 3 files changed, 52 insertions(+), 8 deletions(-) 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 afdf97540e5..beff9db7927 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 @@ -30,8 +30,10 @@ import javax.naming.directory.SearchControls; import javax.naming.directory.SearchResult; import org.apache.commons.lang.StringUtils; +import org.apache.log4j.Logger; public class LdapUserManager { + private static final Logger s_logger = Logger.getLogger(LdapUserManager.class.getName()); @Inject private LdapConfiguration _ldapConfiguration; @@ -155,7 +157,11 @@ public class LdapUserManager { while (values.hasMoreElements()) { String userdn = String.valueOf(values.nextElement()); - users.add(getUserForDn(userdn, context)); + try{ + users.add(getUserForDn(userdn, context)); + } catch (NamingException e){ + s_logger.info("Userdn: " + userdn + " Not Found:: Exception message: " + e.getMessage()); + } } } 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 9fbc81f6230..c8edb3f22f7 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 @@ -48,7 +48,7 @@ class LdapUserManagerSpec extends spock.lang.Specification { @Shared private def principal - private def createGroupSearchContext() { + private def createGroupSearchContextOneUser() { def umSearchResult = Mock(SearchResult) umSearchResult.getName() >> principal; @@ -75,7 +75,34 @@ class LdapUserManagerSpec extends spock.lang.Specification { searchUsersResults.add(userSearchResult); def context = Mock(LdapContext) - context.search(_, _, _) >>> [searchGroupResults, searchUsersResults]; + context.search(_, _, _) >>> [searchGroupResults, searchUsersResults, searchGroupResults, new BasicNamingEnumerationImpl()]; + + return context + } + + private def createGroupSearchContextNoUser() { + + def umSearchResult = Mock(SearchResult) + umSearchResult.getName() >> principal; + umSearchResult.getAttributes() >> principal + + def uniqueMembers = new BasicNamingEnumerationImpl() + uniqueMembers.add(umSearchResult); + def attributes = Mock(Attributes) + def uniqueMemberAttribute = Mock(Attribute) + uniqueMemberAttribute.getId() >> "uniquemember" + uniqueMemberAttribute.getAll() >> uniqueMembers + attributes.get("uniquemember") >> uniqueMemberAttribute + + def groupSearchResult = Mock(SearchResult) + groupSearchResult.getName() >> principal; + groupSearchResult.getAttributes() >> attributes + + def searchGroupResults = new BasicNamingEnumerationImpl() + searchGroupResults.add(groupSearchResult); + + def context = Mock(LdapContext) + context.search(_, _, _) >>> [searchGroupResults, new BasicNamingEnumerationImpl()]; return context } @@ -254,16 +281,26 @@ class LdapUserManagerSpec extends spock.lang.Specification { varGroupName << ["", null, "Murphy"] } - def "test successful getUsersInGroup"() { + def "test successful getUsersInGroup one user"() { given: "ldap user manager and ldap config" def ldapUserManager = new LdapUserManager(ldapConfiguration) when: "A request for users is made" - def result = ldapUserManager.getUsersInGroup("engineering", createGroupSearchContext()) + def result = ldapUserManager.getUsersInGroup("engineering", createGroupSearchContextOneUser()) then: "one user is returned" result.size() == 1 } + def "test successful getUsersInGroup no user"() { + given: "ldap user manager and ldap config" + def ldapUserManager = new LdapUserManager(ldapConfiguration) + + when: "A request for users is made" + def result = ldapUserManager.getUsersInGroup("engineering", createGroupSearchContextNoUser()) + then: "no user is returned" + result.size() == 0 + } + def "test successful getUserForDn"() { given: "ldap user manager and ldap config" def ldapUserManager = new LdapUserManager(ldapConfiguration) diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 75a97bc29f9..11876fab418 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -39,6 +39,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; import org.apache.commons.codec.binary.Base64; +import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.ControlledEntity; @@ -989,15 +990,15 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M domainId = Domain.ROOT_DOMAIN; } - if (userName.isEmpty()) { + if (StringUtils.isEmpty(userName)) { throw new InvalidParameterValueException("Username is empty"); } - if (firstName.isEmpty()) { + if (StringUtils.isEmpty(firstName)) { throw new InvalidParameterValueException("Firstname is empty"); } - if (lastName.isEmpty()) { + if (StringUtils.isEmpty(lastName)) { throw new InvalidParameterValueException("Lastname is empty"); }