mirror of https://github.com/apache/cloudstack.git
Fixed 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 <ian@ianduffy.ie>
This commit is contained in:
Rajani Karuturi
Ian Duffy
parent
78b148d0fc
commit
4552ec6322
|
|
@ -29,6 +29,8 @@ import javax.naming.directory.DirContext;
|
|||
import javax.naming.directory.SearchControls;
|
||||
import javax.naming.directory.SearchResult;
|
||||
|
||||
import org.apache.commons.lang.StringUtils;
|
||||
|
||||
public class LdapUserManager {
|
||||
|
||||
@Inject
|
||||
|
|
@ -185,6 +187,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);
|
||||
}
|
||||
}
|
||||
|
|
@ -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 = "rmurphy@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+"))";
|
||||
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 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
|
||||
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 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.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
|
||||
|
||||
}
|
||||
|
||||
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 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"
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue