diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java index 7599dadffba..24991c35587 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapAuthenticator.java @@ -66,7 +66,7 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { LdapTrustMapVO ldapTrustMapVO = _ldapManager.getDomainLinkedToLdap(domainId); if(ldapTrustMapVO != null) { try { - LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType(), ldapTrustMapVO.getName()); + LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType().toString(), ldapTrustMapVO.getName()); if(!ldapUser.isDisabled()) { result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password); if(result) { diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java index 97cee3d8cd6..6af2c4ebd95 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManager.java @@ -29,6 +29,8 @@ import org.apache.cloudstack.api.response.LinkDomainToLdapResponse; public interface LdapManager extends PluggableService { + enum LinkType { GROUP, OU;} + LdapConfigurationResponse addConfiguration(String hostname, int port) throws InvalidParameterValueException; boolean canAuthenticate(String principal, String password); diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java index 301fcc33653..aab6f8a5581 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java @@ -28,6 +28,7 @@ import javax.naming.ldap.LdapContext; import org.apache.cloudstack.api.command.LinkDomainToLdapCmd; import org.apache.cloudstack.api.response.LinkDomainToLdapResponse; import org.apache.cloudstack.ldap.dao.LdapTrustMapDao; +import org.apache.commons.lang.Validate; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -265,8 +266,14 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { @Override public LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, String name, short accountType) { - LdapTrustMapVO vo = _ldapTrustMapDao.persist(new LdapTrustMapVO(domainId, type, name, accountType)); - LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(vo.getDomainId(), vo.getType(), vo.getName(), vo.getAccountType()); + Validate.notNull(type, "type cannot be null. It should either be GROUP or OU"); + Validate.notNull(domainId, "domainId cannot be null."); + Validate.notEmpty(name, "GROUP or OU name cannot be empty"); + //Account type constants in com.cloud.user.Account + Validate.isTrue(accountType>=0 && accountType<=5, "accountype should be a number from 0-5"); + LinkType linkType = LdapManager.LinkType.valueOf(type.toUpperCase()); + LdapTrustMapVO vo = _ldapTrustMapDao.persist(new LdapTrustMapVO(domainId, linkType, name, accountType)); + LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(vo.getDomainId(), vo.getType().toString(), vo.getName(), vo.getAccountType()); return response; } diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapTrustMapVO.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapTrustMapVO.java index 1356df9b43c..8b1363816fa 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapTrustMapVO.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapTrustMapVO.java @@ -37,7 +37,7 @@ public class LdapTrustMapVO implements InternalIdentity { private Long id; @Column(name = "type") - private String type; + private LdapManager.LinkType type; @Column(name = "name") private String name; @@ -52,7 +52,7 @@ public class LdapTrustMapVO implements InternalIdentity { public LdapTrustMapVO() { } - public LdapTrustMapVO(long domainId, String type, String name, short accountType) { + public LdapTrustMapVO(long domainId, LdapManager.LinkType type, String name, short accountType) { this.domainId = domainId; this.type = type; this.name = name; @@ -64,7 +64,7 @@ public class LdapTrustMapVO implements InternalIdentity { return id; } - public String getType() { + public LdapManager.LinkType getType() { return type; } @@ -79,4 +79,37 @@ public class LdapTrustMapVO implements InternalIdentity { public short getAccountType() { return accountType; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + LdapTrustMapVO that = (LdapTrustMapVO) o; + + if (domainId != that.domainId) { + return false; + } + if (accountType != that.accountType) { + return false; + } + if (type != that.type) { + return false; + } + return name.equals(that.name); + + } + + @Override + public int hashCode() { + int result = type.hashCode(); + result = 31 * result + name.hashCode(); + result = 31 * result + (int) (domainId ^ (domainId >>> 32)); + result = 31 * result + (int) accountType; + return result; + } } diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy index b4a7267d480..6e38926d25c 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapManagerImplSpec.groovy @@ -25,6 +25,8 @@ import org.apache.cloudstack.api.command.LdapImportUsersCmd import org.apache.cloudstack.api.command.LdapListUsersCmd import org.apache.cloudstack.api.command.LdapUserSearchCmd import org.apache.cloudstack.api.command.LinkDomainToLdapCmd +import org.apache.cloudstack.api.response.LinkDomainToLdapResponse +import org.apache.cloudstack.ldap.dao.LdapTrustMapDao import javax.naming.NamingException import javax.naming.ldap.InitialLdapContext @@ -36,6 +38,8 @@ import org.apache.cloudstack.ldap.dao.LdapConfigurationDaoImpl import com.cloud.exception.InvalidParameterValueException import com.cloud.utils.Pair +import javax.naming.ldap.LdapContext + class LdapManagerImplSpec extends spock.lang.Specification { def "Test failing of getUser due to bind issue"() { given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager" @@ -428,4 +432,154 @@ class LdapManagerImplSpec extends spock.lang.Specification { then: "A list greater of size one is returned" result.size() == 1; } + + def "test linkDomainToLdap invalid ldap group type"() { + def ldapManager = new LdapManagerImpl() + LdapTrustMapDao ldapTrustMapDao = Mock(LdapTrustMapDao) + ldapManager._ldapTrustMapDao = ldapTrustMapDao + + def domainId = 1 + when: + println("using type: " + type) + LinkDomainToLdapResponse response = ldapManager.linkDomainToLdap(domainId, type, "CN=test,DC=CCP,DC=Citrix,DC=Com", (short)2) + then: + thrown(IllegalArgumentException) + where: + type << ["", null, "TEST", "TEST TEST"] + } + def "test linkDomainToLdap invalid domain"() { + def ldapManager = new LdapManagerImpl() + LdapTrustMapDao ldapTrustMapDao = Mock(LdapTrustMapDao) + ldapManager._ldapTrustMapDao = ldapTrustMapDao + + when: + LinkDomainToLdapResponse response = ldapManager.linkDomainToLdap(null, "GROUP", "CN=test,DC=CCP,DC=Citrix,DC=Com", (short)2) + then: + thrown(IllegalArgumentException) + } + def "test linkDomainToLdap invalid ldap name"() { + def ldapManager = new LdapManagerImpl() + LdapTrustMapDao ldapTrustMapDao = Mock(LdapTrustMapDao) + ldapManager._ldapTrustMapDao = ldapTrustMapDao + + def domainId = 1 + when: + println("using name: " + name) + LinkDomainToLdapResponse response = ldapManager.linkDomainToLdap(domainId, "GROUP", name, (short)2) + then: + thrown(IllegalArgumentException) + where: + name << ["", null] + } + def "test linkDomainToLdap invalid accountType"(){ + + def ldapManager = new LdapManagerImpl() + LdapTrustMapDao ldapTrustMapDao = Mock(LdapTrustMapDao) + ldapManager._ldapTrustMapDao = ldapTrustMapDao + + def domainId = 1 + when: + println("using accountType: " + accountType) + LinkDomainToLdapResponse response = ldapManager.linkDomainToLdap(domainId, "GROUP", "TEST", (short)accountType) + then: + thrown(IllegalArgumentException) + where: + accountType << [-1, 6, 20000, -500000] + } + def "test linkDomainToLdap when all is well"(){ + def ldapManager = new LdapManagerImpl() + LdapTrustMapDao ldapTrustMapDao = Mock(LdapTrustMapDao) + ldapManager._ldapTrustMapDao = ldapTrustMapDao + + def domainId=1 + def type=LdapManager.LinkType.GROUP + def name="CN=test,DC=CCP, DC=citrix,DC=com" + short accountType=2 + + 1 * ldapTrustMapDao.persist(new LdapTrustMapVO(domainId, type, name, accountType)) >> new LdapTrustMapVO(domainId, type, name, accountType) + + when: + LinkDomainToLdapResponse response = ldapManager.linkDomainToLdap(domainId, type.toString(), name, accountType) + then: + response.getDomainId() == domainId + response.getType() == type.toString() + response.getName() == name + response.getAccountType() == accountType + } + + def "test getUser(username,type,group) when username disabled in ldap"(){ + def ldapUserManager = Mock(LdapUserManager) + def ldapUserManagerFactory = Mock(LdapUserManagerFactory) + ldapUserManagerFactory.getInstance(_) >> ldapUserManager + def ldapContextFactory = Mock(LdapContextFactory) + ldapContextFactory.createBindContext() >> Mock(LdapContext) + def ldapConfiguration = Mock(LdapConfiguration) + + def ldapManager = new LdapManagerImpl() + ldapManager._ldapUserManagerFactory = ldapUserManagerFactory + ldapManager._ldapContextFactory = ldapContextFactory + ldapManager._ldapConfiguration = ldapConfiguration + + def username = "admin" + def type = "GROUP" + def name = "CN=test,DC=citrix,DC=com" + + ldapUserManager.getUser(username, type, name, _) >> new LdapUser(username, "email", "firstname", "lastname", "principal", "domain", true) + + when: + LdapUser user = ldapManager.getUser(username, type, name) + then: + user.getUsername() == username + user.isDisabled() == true + } + + def "test getUser(username,type,group) when username doesnt exist in ldap"(){ + def ldapUserManager = Mock(LdapUserManager) + def ldapUserManagerFactory = Mock(LdapUserManagerFactory) + ldapUserManagerFactory.getInstance(_) >> ldapUserManager + def ldapContextFactory = Mock(LdapContextFactory) + ldapContextFactory.createBindContext() >> Mock(LdapContext) + def ldapConfiguration = Mock(LdapConfiguration) + + def ldapManager = new LdapManagerImpl() + ldapManager._ldapUserManagerFactory = ldapUserManagerFactory + ldapManager._ldapContextFactory = ldapContextFactory + ldapManager._ldapConfiguration = ldapConfiguration + + def username = "admin" + def type = "GROUP" + def name = "CN=test,DC=citrix,DC=com" + + ldapUserManager.getUser(username, type, name, _) >> { throw new NamingException("Test naming exception") } + + when: + LdapUser user = ldapManager.getUser(username, type, name) + then: + thrown(NoLdapUserMatchingQueryException) + } + def "test getUser(username,type,group) when username is an active member of the group in ldap"(){ + def ldapUserManager = Mock(LdapUserManager) + def ldapUserManagerFactory = Mock(LdapUserManagerFactory) + ldapUserManagerFactory.getInstance(_) >> ldapUserManager + def ldapContextFactory = Mock(LdapContextFactory) + ldapContextFactory.createBindContext() >> Mock(LdapContext) + def ldapConfiguration = Mock(LdapConfiguration) + + def ldapManager = new LdapManagerImpl() + ldapManager._ldapUserManagerFactory = ldapUserManagerFactory + ldapManager._ldapContextFactory = ldapContextFactory + ldapManager._ldapConfiguration = ldapConfiguration + + def username = "admin" + def type = "GROUP" + def name = "CN=test,DC=citrix,DC=com" + + ldapUserManager.getUser(username, type, name, _) >> new LdapUser(username, "email", "firstname", "lastname", "principal", "domain", false) + + when: + LdapUser user = ldapManager.getUser(username, type, name) + then: + user.getUsername() == username + user.isDisabled() == false + } }