CLOUDSTACK-6210: LDAP:listLdapUsers api throws exception when we click on "Add
LDAP Account" This occurs when ldap basedn is not configured. Throwing an IAE
and a proper message is returned from the api call
Signed-off-by: Ian Duffy <i...@ianduffy.ie>
(cherry picked from commit 4552ec632201e7432afae7770f5854aaa244267c)
Signed-off-by: Rohit Yadav <rohit.ya...@shapeblue.com>
Conflicts:
plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUserManager.java
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/c2c6ecf8
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/c2c6ecf8
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/c2c6ecf8
Branch: refs/heads/4.3
Commit: c2c6ecf828d85e3a5085ea28baed0ed9e1ba5d9e
Parents: 2534b54
Author: Rajani Karuturi <rajanikarut...@gmail.com>
Authored: Fri Mar 7 11:13:35 2014 +0530
Committer: Rohit Yadav <rohit.ya...@shapeblue.com>
Committed: Wed Nov 26 18:19:54 2014 +0530
----------------------------------------------------------------------
.../apache/cloudstack/ldap/LdapUserManager.java | 10 +-
.../cloudstack/ldap/LdapUserManagerSpec.groovy | 263 ++++++++++---------
2 files changed, 146 insertions(+), 127 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c2c6ecf8/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 59a41de..f80142f 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
@@ -25,6 +25,8 @@ import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
import javax.naming.directory.*;
+import org.apache.commons.lang.StringUtils;
+
public class LdapUserManager {
@Inject
@@ -181,6 +183,10 @@ public class LdapUserManager {
controls.setSearchScope(_ldapConfiguration.getScope());
controls.setReturningAttributes(_ldapConfiguration.getReturnAttributes());
- return context.search(_ldapConfiguration.getBaseDn(),
generateSearchFilter(username), controls);
+ String basedn = _ldapConfiguration.getBaseDn();
+ if (StringUtils.isBlank(basedn)) {
+ throw new IllegalArgumentException("ldap basedn is not
configured");
+ }
+ return context.search(basedn, generateSearchFilter(username),
controls);
}
-}
\ No newline at end of file
+}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c2c6ecf8/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 fa735d3..9fbc81f 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
@@ -21,10 +21,9 @@ import org.apache.cloudstack.ldap.LdapUserManager
import spock.lang.Shared
import javax.naming.NamingException
-import javax.naming.NamingEnumeration
import javax.naming.directory.Attribute
import javax.naming.directory.Attributes
-import javax.naming.directory.DirContext
+import javax.naming.directory.InitialDirContext
import javax.naming.directory.SearchControls
import javax.naming.directory.SearchResult
import javax.naming.ldap.LdapContext
@@ -51,83 +50,83 @@ class LdapUserManagerSpec extends spock.lang.Specification {
private def createGroupSearchContext() {
- def umSearchResult = Mock(SearchResult)
- umSearchResult.getName() >> principal;
- umSearchResult.getAttributes() >> principal
+ 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 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 groupSearchResult = Mock(SearchResult)
+ groupSearchResult.getName() >> principal;
+ groupSearchResult.getAttributes() >> attributes
- def searchGroupResults = new BasicNamingEnumerationImpl()
- searchGroupResults.add(groupSearchResult);
+ def searchGroupResults = new BasicNamingEnumerationImpl()
+ searchGroupResults.add(groupSearchResult);
- attributes = createUserAttributes(username, email, firstname, lastname)
- SearchResult userSearchResult = createSearchResult(attributes)
- def searchUsersResults = new BasicNamingEnumerationImpl()
- searchUsersResults.add(userSearchResult);
+ attributes = createUserAttributes(username, email, firstname, lastname)
+ SearchResult userSearchResult = createSearchResult(attributes)
+ def searchUsersResults = new BasicNamingEnumerationImpl()
+ searchUsersResults.add(userSearchResult);
- def context = Mock(LdapContext)
- context.search(_, _, _) >>> [searchGroupResults, searchUsersResults];
+ def context = Mock(LdapContext)
+ context.search(_, _, _) >>> [searchGroupResults, searchUsersResults];
- return context
+ return context
}
private def createContext() {
- Attributes attributes = createUserAttributes(username, email,
firstname, lastname)
- SearchResult searchResults = createSearchResult(attributes)
- def searchUsersResults = new BasicNamingEnumerationImpl()
- searchUsersResults.add(searchResults);
+ Attributes attributes = createUserAttributes(username, email,
firstname, lastname)
+ SearchResult searchResults = createSearchResult(attributes)
+ def searchUsersResults = new BasicNamingEnumerationImpl()
+ searchUsersResults.add(searchResults);
- def context = Mock(LdapContext)
- context.search(_, _, _) >> searchUsersResults;
+ def context = Mock(LdapContext)
+ context.search(_, _, _) >> searchUsersResults;
- return context
+ return context
}
private SearchResult createSearchResult(attributes) {
- def search = Mock(SearchResult)
+ def search = Mock(SearchResult)
- search.getName() >> "cn=" + attributes.getAt("uid").get();
+ search.getName() >> "cn=" + attributes.getAt("uid").get();
- search.getAttributes() >> attributes
- search.getNameInNamespace() >> principal
+ search.getAttributes() >> attributes
+ search.getNameInNamespace() >> principal
- return search
+ return search
}
private Attributes createUserAttributes(String username, String email,
String firstname, String lastname) {
- def attributes = Mock(Attributes)
+ def attributes = Mock(Attributes)
- def nameAttribute = Mock(Attribute)
- nameAttribute.getId() >> "uid"
- nameAttribute.get() >> username
- attributes.get("uid") >> nameAttribute
+ def nameAttribute = Mock(Attribute)
+ nameAttribute.getId() >> "uid"
+ nameAttribute.get() >> username
+ attributes.get("uid") >> nameAttribute
- def mailAttribute = Mock(Attribute)
- mailAttribute.getId() >> "mail"
- mailAttribute.get() >> email
- attributes.get("mail") >> mailAttribute
+ def mailAttribute = Mock(Attribute)
+ mailAttribute.getId() >> "mail"
+ mailAttribute.get() >> email
+ attributes.get("mail") >> mailAttribute
- def givennameAttribute = Mock(Attribute)
- givennameAttribute.getId() >> "givenname"
- givennameAttribute.get() >> firstname
- attributes.get("givenname") >> givennameAttribute
+ def givennameAttribute = Mock(Attribute)
+ givennameAttribute.getId() >> "givenname"
+ givennameAttribute.get() >> firstname
+ attributes.get("givenname") >> givennameAttribute
- def snAttribute = Mock(Attribute)
- snAttribute.getId() >> "sn"
- snAttribute.get() >> lastname
- attributes.get("sn") >> snAttribute
+ def snAttribute = Mock(Attribute)
+ snAttribute.getId() >> "sn"
+ snAttribute.get() >> lastname
+ attributes.get("sn") >> snAttribute
- return attributes
+ return attributes
}
def setupSpec() {
@@ -140,144 +139,158 @@ class LdapUserManagerSpec extends
spock.lang.Specification {
ldapConfiguration.getFirstnameAttribute() >> "givenname"
ldapConfiguration.getLastnameAttribute() >> "sn"
ldapConfiguration.getBaseDn() >> "dc=cloudstack,dc=org"
- ldapConfiguration.getCommonNameAttribute() >> "cn"
- ldapConfiguration.getGroupObject() >> "groupOfUniqueNames"
- ldapConfiguration.getGroupUniqueMemeberAttribute() >> "uniquemember"
+ ldapConfiguration.getCommonNameAttribute() >> "cn"
+ ldapConfiguration.getGroupObject() >> "groupOfUniqueNames"
+ ldapConfiguration.getGroupUniqueMemeberAttribute() >> "uniquemember"
username = "rmurphy"
email = "rmur...@test.com"
firstname = "Ryan"
lastname = "Murphy"
- principal = "cn=" + username + "," +
ldapConfiguration.getBaseDn()
+ principal = "cn=" + username + "," + ldapConfiguration.getBaseDn()
}
def "Test successfully creating an Ldap User from Search result"() {
- given: "We have attributes, a search and a user manager"
- def attributes = createUserAttributes(username, email,
firstname, lastname)
+ given: "We have attributes, a search and a user manager"
+ def attributes = createUserAttributes(username, email, firstname,
lastname)
def search = createSearchResult(attributes)
def userManager = new LdapUserManager(ldapConfiguration)
def result = userManager.createUser(search)
- expect: "The crated user the data supplied from LDAP"
+ expect: "The crated user the data supplied from LDAP"
result.username == username
result.email == email
result.firstname == firstname
result.lastname == lastname
- result.principal == principal
+ result.principal == principal
}
def "Test successfully returning a list from get users"() {
- given: "We have a LdapUserManager"
+ given: "We have a LdapUserManager"
def userManager = new LdapUserManager(ldapConfiguration)
- when: "A request for users is made"
+ when: "A request for users is made"
def result = userManager.getUsers(username, createContext())
- then: "A list of users is returned"
+ then: "A list of users is returned"
result.size() == 1
}
def "Test successfully returning a list from get users when no username is
given"() {
- given: "We have a LdapUserManager"
+ given: "We have a LdapUserManager"
def userManager = new LdapUserManager(ldapConfiguration)
- when: "Get users is called without a username"
+ when: "Get users is called without a username"
def result = userManager.getUsers(createContext())
- then: "All users are returned"
- result.size() == 1
+ then: "All users are returned"
+ result.size() == 1
}
def "Test successfully returning a NamingEnumeration from searchUsers"() {
- given: "We have a LdapUserManager"
- def userManager = new LdapUserManager(ldapConfiguration)
+ given: "We have a LdapUserManager"
+ def userManager = new LdapUserManager(ldapConfiguration)
- when: "We search for users"
+ when: "We search for users"
def result = userManager.searchUsers(createContext())
- then: "A list of users are returned."
+ then: "A list of users are returned."
result.next().getName() + "," + ldapConfiguration.getBaseDn() ==
principal
}
def "Test successfully returning an Ldap user from a get user request"() {
- given: "We have a LdapUserMaanger"
+ given: "We have a LdapUserMaanger"
- def userManager = new LdapUserManager(ldapConfiguration)
+ def userManager = new LdapUserManager(ldapConfiguration)
- when: "A request for a user is made"
- def result = userManager.getUser(username, createContext())
+ when: "A request for a user is made"
+ def result = userManager.getUser(username, createContext())
- then: "The user is returned"
- result.username == username
- result.email == email
- result.firstname == firstname
- result.lastname == lastname
- result.principal == principal
+ then: "The user is returned"
+ result.username == username
+ result.email == email
+ result.firstname == firstname
+ result.lastname == lastname
+ result.principal == principal
}
def "Test successfully throwing an exception when no users are found with
getUser"() {
- given: "We have a seachResult of users and a User Manager"
+ given: "We have a seachResult of users and a User Manager"
- def searchUsersResults = new BasicNamingEnumerationImpl()
+ def searchUsersResults = new BasicNamingEnumerationImpl()
- def context = Mock(LdapContext)
- context.search(_, _, _) >> searchUsersResults;
+ def context = Mock(LdapContext)
+ context.search(_, _, _) >> searchUsersResults;
- def userManager = new LdapUserManager(ldapConfiguration)
+ def userManager = new LdapUserManager(ldapConfiguration)
- when: "a get user request is made and no user is found"
- def result = userManager.getUser(username, context)
+ when: "a get user request is made and no user is found"
+ def result = userManager.getUser(username, context)
- then: "An exception is thrown."
- thrown NamingException
+ then: "An exception is thrown."
+ thrown NamingException
}
def "Test that a newly created Ldap User Manager is not null"() {
- given: "You have created a new Ldap user manager object"
- def result = new LdapUserManager();
- expect: "The result is not null"
- result != null
+ given: "You have created a new Ldap user manager object"
+ def result = new LdapUserManager();
+ expect: "The result is not null"
+ result != null
}
def "test successful generateGroupSearchFilter"() {
- given: "ldap user manager and ldap config"
- def ldapUserManager = new LdapUserManager(ldapConfiguration)
- def groupName = varGroupName == null ? "*" : varGroupName
- def expectedResult =
"(&(objectClass=groupOfUniqueNames)(cn="+groupName+"))";
-
- def result = ldapUserManager.generateGroupSearchFilter(varGroupName)
- expect:
- result == expectedResult
- where: "The group name passed is set to "
- varGroupName << ["", null, "Murphy"]
+ given: "ldap user manager and ldap config"
+ def ldapUserManager = new LdapUserManager(ldapConfiguration)
+ def groupName = varGroupName == null ? "*" : varGroupName
+ def expectedResult = "(&(objectClass=groupOfUniqueNames)(cn=" +
groupName + "))";
+
+ def result = ldapUserManager.generateGroupSearchFilter(varGroupName)
+ expect:
+ result == expectedResult
+ where: "The group name passed is set to "
+ varGroupName << ["", null, "Murphy"]
}
- def "test successful getUsersInGroup"(){
- given: "ldap user manager and ldap config"
- def ldapUserManager = new LdapUserManager(ldapConfiguration)
+ def "test successful getUsersInGroup"() {
+ 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())
+ then: "one user is returned"
+ result.size() == 1
+ }
+
+ def "test successful getUserForDn"() {
+ given: "ldap user manager and ldap config"
+ def ldapUserManager = new LdapUserManager(ldapConfiguration)
+
+ when: "A request for users is made"
+ def result = ldapUserManager.getUserForDn("cn=Ryan
Murphy,ou=engineering,dc=cloudstack,dc=org", createContext())
+ then: "A list of users is returned"
+ result != 1
+ result.username == username
+ result.email == email
+ result.firstname == firstname
+ result.lastname == lastname
+ result.principal == principal
- when: "A request for users is made"
- def result = ldapUserManager.getUsersInGroup("engineering",
createGroupSearchContext())
- then: "one user is returned"
- result.size() == 1
}
- def "test successful getUserForDn"(){
- given: "ldap user manager and ldap config"
- def ldapUserManager = new LdapUserManager(ldapConfiguration)
+ def "test searchUsers when ldap basedn in not set"() {
+ given: "ldap configuration where basedn is not set"
+ def ldapconfig = Mock(LdapConfiguration)
+ ldapconfig.getBaseDn() >> null
+ def ldapUserManager = new LdapUserManager(ldapconfig)
- when: "A request for users is made"
- def result = ldapUserManager.getUserForDn("cn=Ryan
Murphy,ou=engineering,dc=cloudstack,dc=org",createContext())
- then: "A list of users is returned"
- result != 1
- result.username == username
- result.email == email
- result.firstname == firstname
- result.lastname == lastname
- result.principal == principal
+ when: "A request for search users is made"
+ def result = ldapUserManager.searchUsers(new InitialDirContext())
+ then: "An exception with no basedn defined is returned"
+ def e = thrown(IllegalArgumentException)
+ e.message == "ldap basedn is not configured"
}
}
