From 0680648036044e5d9166a0d973b2c9f714a41778 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Tue, 4 Aug 2015 16:44:58 +0530 Subject: [PATCH 01/16] CLOUDSTACK-8647: added cmd and response class for the new api --- .../apache/cloudstack/api/ApiConstants.java | 2 + .../api/command/LinkDomainToLdapCmd.java | 70 +++++++++++++++++++ .../response/LinkDomainToLdapResponse.java | 42 +++++++++++ 3 files changed, 114 insertions(+) create mode 100644 plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java create mode 100644 plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java index e86e2d4a995..2728bcc6fef 100644 --- a/api/src/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/org/apache/cloudstack/api/ApiConstants.java @@ -628,6 +628,8 @@ public class ApiConstants { public static final String OVM3_CLUSTER = "ovm3cluster"; public static final String OVM3_VIP = "ovm3vip"; + public static final String ADMIN = "admin"; + public enum HostDetails { all, capacity, events, stats, min; } diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java new file mode 100644 index 00000000000..8e5cd0c5176 --- /dev/null +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.api.command; + +import javax.inject.Inject; + +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.DomainResponse; +import org.apache.cloudstack.api.response.LinkDomainToLdapResponse; +import org.apache.cloudstack.ldap.LdapManager; +import org.apache.log4j.Logger; + +import com.cloud.user.Account; + +@APICommand(name = "linkDomainToLdap", description = "link an existing cloudstack domain to group or OU in ldap", responseObject = LinkDomainToLdapResponse.class, since = "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +public class LinkDomainToLdapCmd extends BaseCmd { + public static final Logger s_logger = Logger.getLogger(LinkDomainToLdapCmd.class.getName()); + private static final String s_name = "linkdomaintoldapresponse"; + + @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "The id of the domain which has to be linked to LDAP.") + private Long domainId; + + @Parameter(name = ApiConstants.TYPE, type = CommandType.STRING, required = true, description = "type of the ldap name. GROUP or OU") + private String type; + + @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "name of the group or OU in LDAP") + private String name; + + @Parameter(name = ApiConstants.ADMIN, type = CommandType.STRING, required = false, description = "domain admin username in LDAP ") + private String admin; + + @Inject + private LdapManager _ldapManager; + + @Override + public void execute() throws ServerApiException { + // TODO Auto-generated method stub + + } + + @Override + public String getCommandName() { + return s_name; + } + + @Override + public long getEntityOwnerId() { + return Account.ACCOUNT_ID_SYSTEM; + } +} diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java new file mode 100644 index 00000000000..e548fcea1ac --- /dev/null +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.api.response; + +import com.cloud.serializer.Param; +import com.google.gson.annotations.SerializedName; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseResponse; +import org.apache.log4j.Logger; + +public class LinkDomainToLdapResponse extends BaseResponse { + public static final Logger s_logger = Logger.getLogger(LinkDomainToLdapResponse.class.getName()); + + @SerializedName(ApiConstants.DOMAIN_ID) + @Param(description = "id of the Domain which is linked to LDAP") + private String domainId; + + @SerializedName(ApiConstants.NAME) + @Param(description = "name of the group or OU in LDAP which is linked to the domain") + private String name; + + @SerializedName(ApiConstants.TYPE) + @Param(description = "type of the name in LDAP which is linke to the domain") + private String type; + +} From e3ddde841ed4b4ee588927394a2121fefd2d684c Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Thu, 6 Aug 2015 17:12:24 +0530 Subject: [PATCH 02/16] CLOUDSTACK-8647 added new api linkLdapToDomain also added the required dao, table and vo --- client/tomcatconf/commands.properties.in | 1 + .../cloudstack/ldap/spring-ldap-context.xml | 1 + .../api/command/LinkDomainToLdapCmd.java | 14 +++- .../apache/cloudstack/ldap/LdapManager.java | 3 + .../cloudstack/ldap/LdapManagerImpl.java | 14 ++++ .../cloudstack/ldap/LdapTrustMapVO.java | 73 +++++++++++++++++++ .../cloudstack/ldap/dao/LdapTrustMapDao.java | 26 +++++++ .../ldap/dao/LdapTrustMapDaoImpl.java | 34 +++++++++ setup/db/db/schema-452to460.sql | 10 +++ 9 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapTrustMapVO.java create mode 100644 plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDao.java create mode 100644 plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDaoImpl.java diff --git a/client/tomcatconf/commands.properties.in b/client/tomcatconf/commands.properties.in index aec751658b3..473c6354a8f 100644 --- a/client/tomcatconf/commands.properties.in +++ b/client/tomcatconf/commands.properties.in @@ -771,6 +771,7 @@ deleteLdapConfiguration=3 listLdapUsers=3 ldapCreateAccount=3 importLdapUsers=3 +linkDomainToLdap=3 #### juniper-contrail commands diff --git a/plugins/user-authenticators/ldap/resources/META-INF/cloudstack/ldap/spring-ldap-context.xml b/plugins/user-authenticators/ldap/resources/META-INF/cloudstack/ldap/spring-ldap-context.xml index 8ae4009367f..07d6b381328 100644 --- a/plugins/user-authenticators/ldap/resources/META-INF/cloudstack/ldap/spring-ldap-context.xml +++ b/plugins/user-authenticators/ldap/resources/META-INF/cloudstack/ldap/spring-ldap-context.xml @@ -35,5 +35,6 @@ + diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java index 8e5cd0c5176..8601c2d2298 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java @@ -20,8 +20,10 @@ package org.apache.cloudstack.api.command; import javax.inject.Inject; +import com.cloud.exception.InvalidParameterValueException; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; @@ -32,7 +34,8 @@ import org.apache.log4j.Logger; import com.cloud.user.Account; -@APICommand(name = "linkDomainToLdap", description = "link an existing cloudstack domain to group or OU in ldap", responseObject = LinkDomainToLdapResponse.class, since = "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +@APICommand(name = "linkDomainToLdap", description = "link an existing cloudstack domain to group or OU in ldap", responseObject = LinkDomainToLdapResponse.class, since = "4.6.0", + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class LinkDomainToLdapCmd extends BaseCmd { public static final Logger s_logger = Logger.getLogger(LinkDomainToLdapCmd.class.getName()); private static final String s_name = "linkdomaintoldapresponse"; @@ -55,7 +58,14 @@ public class LinkDomainToLdapCmd extends BaseCmd { @Override public void execute() throws ServerApiException { // TODO Auto-generated method stub - + try { + LinkDomainToLdapResponse response = _ldapManager.linkDomainToLdap(domainId, type, name); + response.setObjectName("LinkDomainToLdap"); + response.setResponseName(getCommandName()); + setResponseObject(response); + } catch (final InvalidParameterValueException e) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.toString()); + } } @Override 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 31205c457ad..88f11ad12bb 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 @@ -25,6 +25,7 @@ import org.apache.cloudstack.api.response.LdapUserResponse; import com.cloud.exception.InvalidParameterValueException; import com.cloud.utils.Pair; import com.cloud.utils.component.PluggableService; +import org.apache.cloudstack.api.response.LinkDomainToLdapResponse; public interface LdapManager extends PluggableService { @@ -49,4 +50,6 @@ public interface LdapManager extends PluggableService { Pair, Integer> listConfigurations(LdapListConfigurationCmd cmd); List searchUsers(String query) throws NoLdapUserMatchingQueryException; + + LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, String name); } \ No newline at end of file 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 8e912b8b030..d0f5d9fddd7 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 @@ -25,6 +25,9 @@ import javax.inject.Inject; import javax.naming.NamingException; 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.log4j.Logger; import org.springframework.stereotype.Component; @@ -61,6 +64,9 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { @Inject LdapUserManagerFactory _ldapUserManagerFactory; + @Inject + LdapTrustMapDao _ldapTrustMapDao; + public LdapManagerImpl() { super(); @@ -168,6 +174,7 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { cmdList.add(LdapImportUsersCmd.class); cmdList.add(LDAPConfigCmd.class); cmdList.add(LDAPRemoveCmd.class); + cmdList.add(LinkDomainToLdapCmd.class); return cmdList; } @@ -243,4 +250,11 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { closeContext(context); } } + + @Override + public LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, String name) { + // TODO Auto-generated method stub + LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.persist(new LdapTrustMapVO(domainId, type, name)); + return null; + } } 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 new file mode 100644 index 00000000000..e4a9407ec6b --- /dev/null +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapTrustMapVO.java @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.ldap; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; + +import org.apache.cloudstack.api.InternalIdentity; + +@Entity +@Table(name = "ldap_trust_map") +public class LdapTrustMapVO implements InternalIdentity { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private Long id; + + @Column(name = "type") + private String type; + + @Column(name = "name") + private String name; + + @Column(name = "domain_id") + private long domainId; + + public LdapTrustMapVO(long domainId, String type, String name) { + this.domainId = domainId; + this.type = type; + this.name = name; + } + + @Override + public long getId() { + return id; + } + + public String getType() { + return type; + } + + public String getName() { + return name; + } + + public long getDomainId() { + return domainId; + } + + public LdapTrustMapVO() { + } +} diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDao.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDao.java new file mode 100644 index 00000000000..c4173fe1961 --- /dev/null +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDao.java @@ -0,0 +1,26 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.ldap.dao; + +import org.apache.cloudstack.ldap.LdapTrustMapVO; + +import com.cloud.utils.db.GenericDao; + +public interface LdapTrustMapDao extends GenericDao { +} diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDaoImpl.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDaoImpl.java new file mode 100644 index 00000000000..a6ce2b1053f --- /dev/null +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDaoImpl.java @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.ldap.dao; + +import javax.ejb.Local; + +import org.apache.cloudstack.ldap.LdapTrustMapVO; +import org.springframework.stereotype.Component; + +import com.cloud.utils.db.GenericDaoBase; + +@Component +@Local(value = {LdapTrustMapDao.class}) +public class LdapTrustMapDaoImpl extends GenericDaoBase implements LdapTrustMapDao { + public LdapTrustMapDaoImpl() { + super(); + } +} diff --git a/setup/db/db/schema-452to460.sql b/setup/db/db/schema-452to460.sql index e013b2bf85c..af313646bde 100644 --- a/setup/db/db/schema-452to460.sql +++ b/setup/db/db/schema-452to460.sql @@ -399,3 +399,13 @@ CREATE TABLE `cloud`.`external_bigswitch_bcf_devices` ( CONSTRAINT `fk_external_bigswitch_bcf_devices__physical_network_id` FOREIGN KEY (`physical_network_id`) REFERENCES `physical_network`(`id`) ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8; +CREATE TABLE `cloud`.`ldap_trust_map` ( + `id` int unsigned NOT NULL AUTO_INCREMENT, + `domain_id` bigint unsigned NOT NULL, + `type` varchar(10) NOT NULL, + `name` varchar(255) NOT NULL, + PRIMARY KEY (`id`), + UNIQUE KEY `uk_ldap_trust_map__domain_id` (`id`), + KEY `fk_ldap_trust_map__domain_id` (`domain_id`), + CONSTRAINT `fk_ldap_trust_map__domain_id` FOREIGN KEY (`domain_id`) REFERENCES `domain` (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; From 7109689fde9895d1e702544e4011de9eecc2c1a6 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Mon, 10 Aug 2015 17:31:34 +0530 Subject: [PATCH 03/16] CLOUDSTACK-8647 changed the authentication flow added check to see if domain is linked to ldap. If yes and the user is member of the group/OU, authenticate and import user. --- .../ldap/ADLdapUserManagerImpl.java | 13 ++++ .../cloudstack/ldap/LdapAuthenticator.java | 74 +++++++++++++++---- .../cloudstack/ldap/LdapConfiguration.java | 7 +- .../apache/cloudstack/ldap/LdapManager.java | 6 +- .../cloudstack/ldap/LdapManagerImpl.java | 33 +++++++-- .../org/apache/cloudstack/ldap/LdapUser.java | 9 ++- .../cloudstack/ldap/LdapUserManager.java | 2 + .../ldap/OpenLdapUserManagerImpl.java | 69 ++++++++++++++++- .../cloudstack/ldap/dao/LdapTrustMapDao.java | 1 + .../ldap/dao/LdapTrustMapDaoImpl.java | 14 ++++ .../com/cloud/user/AccountManagerImpl.java | 6 +- 11 files changed, 203 insertions(+), 31 deletions(-) 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 50f1fa00092..fc36267c0cf 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 @@ -79,4 +79,17 @@ public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl implements Ld s_logger.debug("group search filter = " + result); return result.toString(); } + + protected boolean isUserDisabled(SearchResult result) throws NamingException { + boolean isDisabledUser = false; + String userAccountControl = LdapUtils.getAttributeValue(result.getAttributes(), _ldapConfiguration.getUserAccountControlAttribute()); + if (userAccountControl != null) { + int control = Integer.valueOf(userAccountControl); + // second bit represents disabled user flag in AD + if ((control & 2) > 0) { + isDisabledUser = true; + } + } + return isDisabledUser; + } } 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 8c6820f8458..fb1b01e2aee 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 @@ -17,6 +17,9 @@ package org.apache.cloudstack.ldap; import com.cloud.server.auth.DefaultUserAuthenticator; +import com.cloud.user.Account; +import com.cloud.user.AccountService; +import com.cloud.user.User; import com.cloud.user.UserAccount; import com.cloud.user.dao.UserAccountDao; import com.cloud.utils.Pair; @@ -25,6 +28,7 @@ import org.apache.log4j.Logger; import javax.inject.Inject; import java.util.Map; +import java.util.UUID; public class LdapAuthenticator extends DefaultUserAuthenticator { private static final Logger s_logger = Logger.getLogger(LdapAuthenticator.class.getName()); @@ -33,6 +37,8 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { private LdapManager _ldapManager; @Inject private UserAccountDao _userAccountDao; + @Inject + public AccountService _accountService; public LdapAuthenticator() { super(); @@ -52,22 +58,64 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { return new Pair(false, null); } - final UserAccount user = _userAccountDao.getUserAccount(username, domainId); + boolean result = false; + ActionOnFailedAuthentication action = null; - if (user == null) { - s_logger.debug("Unable to find user with " + username + " in domain " + domainId); - return new Pair(false, null); - } else if (_ldapManager.isLdapEnabled()) { - boolean result = _ldapManager.canAuthenticate(username, password); - ActionOnFailedAuthentication action = null; - if (result == false) { - action = ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT; + if (_ldapManager.isLdapEnabled()) { + LdapTrustMapVO ldapTrustMapVO = _ldapManager.getDomainLinkedToLdap(domainId); + if(ldapTrustMapVO != null) { + try { + LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType(), ldapTrustMapVO.getName()); + if(!ldapUser.isDisabled()) { + result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password); + if(result) { + final UserAccount user = _userAccountDao.getUserAccount(username, domainId); + if (user == null) { + // import user to cloudstack + createCloudStackUserAccount(ldapUser, domainId); + } + } + } else { + //disable user in cloudstack + disableUserInCloudStack(ldapUser, domainId); + } + } catch (NoLdapUserMatchingQueryException e) { + s_logger.debug(e.getMessage()); + } + + } else { + //domain is not linked to ldap follow normal authentication + final UserAccount user = _userAccountDao.getUserAccount(username, domainId); + if(user != null ) { + try { + LdapUser ldapUser = _ldapManager.getUser(username); + if(!ldapUser.isDisabled()) { + result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password); + } else { + s_logger.debug("user with principal "+ ldapUser.getPrincipal() + " is disabled in ldap"); + } + } catch (NoLdapUserMatchingQueryException e) { + s_logger.debug(e.getMessage()); + } + } } - return new Pair(result, action); - - } else { - return new Pair(false, ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT); } + + if (!result) { + action = ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT; + } + return new Pair(result, action); + } + + private void createCloudStackUserAccount(LdapUser user, long domainId) { + String username = user.getUsername(); + _accountService.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), "GMT", username, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, domainId, + username, null, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); + } + + private void disableUserInCloudStack(LdapUser ldapUser, long domainId) { + final UserAccount user = _userAccountDao.getUserAccount(ldapUser.getUsername(), domainId); + _accountService.lockUser(user.getId()); } @Override 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 a64899af296..95019015442 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 @@ -108,7 +108,8 @@ public class LdapConfiguration implements Configurable{ } public String[] getReturnAttributes() { - return new String[] {getUsernameAttribute(), getEmailAttribute(), getFirstnameAttribute(), getLastnameAttribute(), getCommonNameAttribute()}; + return new String[] {getUsernameAttribute(), getEmailAttribute(), getFirstnameAttribute(), getLastnameAttribute(), getCommonNameAttribute(), + getUserAccountControlAttribute()}; } public int getScope() { @@ -159,6 +160,10 @@ public class LdapConfiguration implements Configurable{ return "cn"; } + public String getUserAccountControlAttribute() { + return "userAccountControl"; + } + public Long getReadTimeout() { return ldapReadTimeout.value(); } 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 88f11ad12bb..76d8ce05a48 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 @@ -31,7 +31,7 @@ public interface LdapManager extends PluggableService { LdapConfigurationResponse addConfiguration(String hostname, int port) throws InvalidParameterValueException; - boolean canAuthenticate(String username, String password); + boolean canAuthenticate(String principal, String password); LdapConfigurationResponse createLdapConfigurationResponse(LdapConfigurationVO configuration); @@ -41,6 +41,8 @@ public interface LdapManager extends PluggableService { LdapUser getUser(final String username) throws NoLdapUserMatchingQueryException; + LdapUser getUser(String username, String type, String name) throws NoLdapUserMatchingQueryException; + List getUsers() throws NoLdapUserMatchingQueryException; List getUsersInGroup(String groupName) throws NoLdapUserMatchingQueryException; @@ -52,4 +54,6 @@ public interface LdapManager extends PluggableService { List searchUsers(String query) throws NoLdapUserMatchingQueryException; LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, String name); + + public LdapTrustMapVO getDomainLinkedToLdap(long domainId); } \ No newline at end of file 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 d0f5d9fddd7..9d48956e592 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 @@ -105,17 +105,14 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { } @Override - public boolean canAuthenticate(final String username, final String password) { - final String escapedUsername = LdapUtils.escapeLDAPSearchFilter(username); + public boolean canAuthenticate(final String principal, final String password) { try { - final LdapUser user = getUser(escapedUsername); - final String principal = user.getPrincipal(); final LdapContext context = _ldapContextFactory.createUserContext(principal, password); closeContext(context); return true; - } catch (NamingException | IOException | NoLdapUserMatchingQueryException e) { - s_logger.debug("Exception while doing an LDAP bind for user "+" "+username, e); - s_logger.info("Failed to authenticate user: " + username + ". incorrect password."); + } catch (NamingException | IOException e) { + s_logger.debug("Exception while doing an LDAP bind for user "+" "+principal, e); + s_logger.info("Failed to authenticate user: " + principal + ". incorrect password."); return false; } } @@ -126,7 +123,7 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { context.close(); } } catch (final NamingException e) { - s_logger.warn(e.getMessage(),e); + s_logger.warn(e.getMessage(), e); } } @@ -195,6 +192,21 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { } } + @Override + public LdapUser getUser(final String username, final String type, final String name) throws NoLdapUserMatchingQueryException { + LdapContext context = null; + try { + context = _ldapContextFactory.createBindContext(); + final String escapedUsername = LdapUtils.escapeLDAPSearchFilter(username); + return _ldapUserManagerFactory.getInstance(_ldapConfiguration.getLdapProvider()).getUser(escapedUsername, type, name, context); + } catch (NamingException | IOException e) { + s_logger.debug("ldap Exception: ",e); + throw new NoLdapUserMatchingQueryException("No Ldap User found for username: "+username + "name: " + name + "of type" + type); + } finally { + closeContext(context); + } + } + @Override public List getUsers() throws NoLdapUserMatchingQueryException { LdapContext context = null; @@ -257,4 +269,9 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.persist(new LdapTrustMapVO(domainId, type, name)); return null; } + + @Override + public LdapTrustMapVO getDomainLinkedToLdap(long domainId){ + return _ldapTrustMapDao.findByDomainId(domainId); + } } diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUser.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUser.java index 0a998f2655a..c4c334b5b6e 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUser.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapUser.java @@ -23,14 +23,16 @@ public class LdapUser implements Comparable { private final String lastname; private final String username; private final String domain; + private final boolean disabled; - public LdapUser(final String username, final String email, final String firstname, final String lastname, final String principal, String domain) { + public LdapUser(final String username, final String email, final String firstname, final String lastname, final String principal, String domain, boolean disabled) { this.username = username; this.email = email; this.firstname = firstname; this.lastname = lastname; this.principal = principal; this.domain = domain; + this.disabled = disabled; } @Override @@ -74,6 +76,11 @@ public class LdapUser implements Comparable { return domain; } + public boolean isDisabled() { + return disabled; + } + + @Override public int hashCode() { return getUsername().hashCode(); 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 c1bfe742616..4e2bcf816b2 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 @@ -32,6 +32,8 @@ public interface LdapUserManager { public LdapUser getUser(final String username, final LdapContext context) throws NamingException, IOException; + public LdapUser getUser(final String username, final String type, final String name, final LdapContext context) throws NamingException, IOException; + public List getUsers(final LdapContext context) throws NamingException, IOException; public List getUsers(final String username, final LdapContext context) throws NamingException, IOException; diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java index 11e6bcfc92a..763f1b79ef9 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java @@ -63,7 +63,9 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { domain = domain.replace("," + _ldapConfiguration.getBaseDn(), ""); domain = domain.replace("ou=", ""); - return new LdapUser(username, email, firstname, lastname, principal, domain); + boolean disabled = isUserDisabled(result); + + return new LdapUser(username, email, firstname, lastname, principal, domain, disabled); } private String generateSearchFilter(final String username) { @@ -128,6 +130,43 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { } } + @Override + public LdapUser getUser(final String username, final String type, final String name, final LdapContext context) throws NamingException, IOException { + String basedn; + if("OU".equals(type)) { + basedn = name; + } else { + basedn = _ldapConfiguration.getBaseDn(); + } + + final StringBuilder userObjectFilter = new StringBuilder(); + userObjectFilter.append("(objectClass="); + userObjectFilter.append(_ldapConfiguration.getUserObject()); + userObjectFilter.append(")"); + + final StringBuilder usernameFilter = new StringBuilder(); + usernameFilter.append("("); + usernameFilter.append(_ldapConfiguration.getUsernameAttribute()); + usernameFilter.append("="); + usernameFilter.append((username == null ? "*" : username)); + usernameFilter.append(")"); + + final StringBuilder memberOfFilter = new StringBuilder(); + if ("GROUP".equals(type)) { + memberOfFilter.append("(memberof="); + memberOfFilter.append(name); + memberOfFilter.append(")"); + } + + final StringBuilder searchQuery = new StringBuilder(); + searchQuery.append("(&"); + searchQuery.append(userObjectFilter); + searchQuery.append(usernameFilter); + searchQuery.append(memberOfFilter); + searchQuery.append(")"); + + return searchUser(basedn, searchQuery.toString(), context); + } @Override public List getUsers(final LdapContext context) throws NamingException, IOException { return getUsers(null, context); @@ -191,6 +230,30 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { return searchUsers(null, context); } + protected boolean isUserDisabled(SearchResult result) throws NamingException { + return false; + } + + public LdapUser searchUser(final String basedn, final String searchString, final LdapContext context) throws NamingException, IOException { + final SearchControls searchControls = new SearchControls(); + + searchControls.setSearchScope(_ldapConfiguration.getScope()); + searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes()); + + NamingEnumeration results = context.search(basedn, searchString, searchControls); + final List users = new ArrayList(); + while (results.hasMoreElements()) { + final SearchResult result = results.nextElement(); + users.add(createUser(result)); + } + + if (users.size() == 1) { + return users.get(0); + } else { + throw new NamingException("No user found for basedn " + basedn + " and searchString " + searchString); + } + } + @Override public List searchUsers(final String username, final LdapContext context) throws NamingException, IOException { @@ -212,7 +275,9 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { results = context.search(basedn, generateSearchFilter(username), searchControls); while (results.hasMoreElements()) { final SearchResult result = results.nextElement(); - users.add(createUser(result)); + if (!isUserDisabled(result)) { + users.add(createUser(result)); + } } Control[] contextControls = context.getResponseControls(); if (contextControls != null) { diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDao.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDao.java index c4173fe1961..7ef3799b3d8 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDao.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDao.java @@ -23,4 +23,5 @@ import org.apache.cloudstack.ldap.LdapTrustMapVO; import com.cloud.utils.db.GenericDao; public interface LdapTrustMapDao extends GenericDao { + LdapTrustMapVO findByDomainId(long domainId); } diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDaoImpl.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDaoImpl.java index a6ce2b1053f..fb0d74d122d 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDaoImpl.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/dao/LdapTrustMapDaoImpl.java @@ -20,6 +20,8 @@ package org.apache.cloudstack.ldap.dao; import javax.ejb.Local; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; import org.apache.cloudstack.ldap.LdapTrustMapVO; import org.springframework.stereotype.Component; @@ -28,7 +30,19 @@ import com.cloud.utils.db.GenericDaoBase; @Component @Local(value = {LdapTrustMapDao.class}) public class LdapTrustMapDaoImpl extends GenericDaoBase implements LdapTrustMapDao { + private final SearchBuilder domainIdSearch; + public LdapTrustMapDaoImpl() { super(); + domainIdSearch = createSearchBuilder(); + domainIdSearch.and("domainId", domainIdSearch.entity().getDomainId(), SearchCriteria.Op.EQ); + domainIdSearch.done(); + } + + @Override + public LdapTrustMapVO findByDomainId(long domainId) { + final SearchCriteria sc = domainIdSearch.create(); + sc.setParameters("domainId", domainId); + return findOneBy(sc); } } diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 2d07e841601..a8745bb6e72 100644 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -2145,14 +2145,10 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M s_logger.debug("Attempting to log in user: " + username + " in domain " + domainId); } UserAccount userAccount = _userAccountDao.getUserAccount(username, domainId); - if (userAccount == null) { - s_logger.warn("Unable to find an user with username " + username + " in domain " + domainId); - return null; - } boolean authenticated = false; HashSet actionsOnFailedAuthenticaion = new HashSet(); - User.Source userSource = userAccount.getSource(); + User.Source userSource = userAccount != null ? userAccount.getSource(): User.Source.UNKNOWN; for (UserAuthenticator authenticator : _userAuthenticators) { if(userSource != User.Source.UNKNOWN) { if(!authenticator.getName().equalsIgnoreCase(userSource.name())){ From 0dc9ccd189682f82abd9ce1ab816213094b037db Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Tue, 11 Aug 2015 12:07:44 +0530 Subject: [PATCH 04/16] CLOUDSTACK-8647 added account_type to the linkDomainToLdap API --- .../api/command/LinkDomainToLdapCmd.java | 7 +++-- .../response/LinkDomainToLdapResponse.java | 30 +++++++++++++++++-- .../ldap/ADLdapUserManagerImpl.java | 8 +++-- .../cloudstack/ldap/LdapAuthenticator.java | 24 +++++++-------- .../apache/cloudstack/ldap/LdapManager.java | 2 +- .../cloudstack/ldap/LdapManagerImpl.java | 8 ++--- .../cloudstack/ldap/LdapTrustMapVO.java | 13 ++++++-- .../ldap/OpenLdapUserManagerImpl.java | 7 ++++- .../com/cloud/user/AccountManagerImpl.java | 8 +++++ setup/db/db/schema-452to460.sql | 1 + 10 files changed, 79 insertions(+), 29 deletions(-) diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java index 8601c2d2298..5a76e8ec20a 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java @@ -52,14 +52,17 @@ public class LinkDomainToLdapCmd extends BaseCmd { @Parameter(name = ApiConstants.ADMIN, type = CommandType.STRING, required = false, description = "domain admin username in LDAP ") private String admin; + @Parameter(name = ApiConstants.ACCOUNT_TYPE, type = CommandType.SHORT, required = true, description = "Type of the account to auto import. Specify 0 for user, 1 for root " + + "admin, and 2 for domain admin") + private short accountType; + @Inject private LdapManager _ldapManager; @Override public void execute() throws ServerApiException { - // TODO Auto-generated method stub try { - LinkDomainToLdapResponse response = _ldapManager.linkDomainToLdap(domainId, type, name); + LinkDomainToLdapResponse response = _ldapManager.linkDomainToLdap(domainId, type, name, accountType); response.setObjectName("LinkDomainToLdap"); response.setResponseName(getCommandName()); setResponseObject(response); diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java index e548fcea1ac..103fb25e540 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java @@ -22,14 +22,12 @@ import com.cloud.serializer.Param; import com.google.gson.annotations.SerializedName; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseResponse; -import org.apache.log4j.Logger; public class LinkDomainToLdapResponse extends BaseResponse { - public static final Logger s_logger = Logger.getLogger(LinkDomainToLdapResponse.class.getName()); @SerializedName(ApiConstants.DOMAIN_ID) @Param(description = "id of the Domain which is linked to LDAP") - private String domainId; + private long domainId; @SerializedName(ApiConstants.NAME) @Param(description = "name of the group or OU in LDAP which is linked to the domain") @@ -39,4 +37,30 @@ public class LinkDomainToLdapResponse extends BaseResponse { @Param(description = "type of the name in LDAP which is linke to the domain") private String type; + @SerializedName(ApiConstants.ACCOUNT_TYPE) + @Param(description = "Type of the account to auto import") + private short accountType; + + public LinkDomainToLdapResponse(long domainId, String type, String name, short accountType) { + this.domainId = domainId; + this.name = name; + this.type = type; + this.accountType = accountType; + } + + public long getDomainId() { + return domainId; + } + + public String getName() { + return name; + } + + public String getType() { + return type; + } + + public short getAccountType() { + return accountType; + } } 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 fc36267c0cf..89a278191a4 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,7 +32,7 @@ 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"; + private static final String MICROSOFT_AD_NESTED_MEMBERS_FILTER = "memberOf:1.2.840.113556.1.4.1941:"; @Override public List getUsersInGroup(String groupName, LdapContext context) throws NamingException { @@ -66,7 +66,7 @@ public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl implements Ld final StringBuilder memberOfFilter = new StringBuilder(); String groupCnName = _ldapConfiguration.getCommonNameAttribute() + "=" +groupName + "," + _ldapConfiguration.getBaseDn(); - memberOfFilter.append("(" + MICROSOFT_AD_NESTED_MEMBERS_FILTER + ":="); + memberOfFilter.append("(" + MICROSOFT_AD_NESTED_MEMBERS_FILTER + "="); memberOfFilter.append(groupCnName); memberOfFilter.append(")"); @@ -92,4 +92,8 @@ public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl implements Ld } return isDisabledUser; } + + protected String getMemberOfAttribute() { + return MICROSOFT_AD_NESTED_MEMBERS_FILTER; + } } 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 fb1b01e2aee..a04868eca01 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 @@ -17,7 +17,6 @@ package org.apache.cloudstack.ldap; import com.cloud.server.auth.DefaultUserAuthenticator; -import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.user.UserAccount; @@ -62,18 +61,16 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { ActionOnFailedAuthentication action = null; if (_ldapManager.isLdapEnabled()) { + final UserAccount user = _userAccountDao.getUserAccount(username, domainId); LdapTrustMapVO ldapTrustMapVO = _ldapManager.getDomainLinkedToLdap(domainId); if(ldapTrustMapVO != null) { try { LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType(), ldapTrustMapVO.getName()); if(!ldapUser.isDisabled()) { result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password); - if(result) { - final UserAccount user = _userAccountDao.getUserAccount(username, domainId); - if (user == null) { - // import user to cloudstack - createCloudStackUserAccount(ldapUser, domainId); - } + if(result && (user == null)) { + // import user to cloudstack + createCloudStackUserAccount(ldapUser, domainId, ldapTrustMapVO.getAccountType()); } } else { //disable user in cloudstack @@ -85,7 +82,6 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { } else { //domain is not linked to ldap follow normal authentication - final UserAccount user = _userAccountDao.getUserAccount(username, domainId); if(user != null ) { try { LdapUser ldapUser = _ldapManager.getUser(username); @@ -99,18 +95,18 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { } } } + if (!result && user != null) { + action = ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT; + } } - if (!result) { - action = ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT; - } return new Pair(result, action); } - private void createCloudStackUserAccount(LdapUser user, long domainId) { + private void createCloudStackUserAccount(LdapUser user, long domainId, short accountType) { String username = user.getUsername(); - _accountService.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), "GMT", username, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, domainId, - username, null, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); + _accountService.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), null, username, accountType, domainId, username, null, + UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); } private void disableUserInCloudStack(LdapUser ldapUser, long domainId) { 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 76d8ce05a48..97cee3d8cd6 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 @@ -53,7 +53,7 @@ public interface LdapManager extends PluggableService { List searchUsers(String query) throws NoLdapUserMatchingQueryException; - LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, String name); + LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, String name, short accountType); public LdapTrustMapVO getDomainLinkedToLdap(long domainId); } \ No newline at end of file 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 9d48956e592..301fcc33653 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 @@ -264,10 +264,10 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { } @Override - public LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, String name) { - // TODO Auto-generated method stub - LdapTrustMapVO ldapTrustMapVO = _ldapTrustMapDao.persist(new LdapTrustMapVO(domainId, type, name)); - return null; + 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()); + return response; } @Override 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 e4a9407ec6b..1356df9b43c 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 @@ -45,10 +45,18 @@ public class LdapTrustMapVO implements InternalIdentity { @Column(name = "domain_id") private long domainId; - public LdapTrustMapVO(long domainId, String type, String name) { + @Column(name = "account_type") + private short accountType; + + + public LdapTrustMapVO() { + } + + public LdapTrustMapVO(long domainId, String type, String name, short accountType) { this.domainId = domainId; this.type = type; this.name = name; + this.accountType = accountType; } @Override @@ -68,6 +76,7 @@ public class LdapTrustMapVO implements InternalIdentity { return domainId; } - public LdapTrustMapVO() { + public short getAccountType() { + return accountType; } } diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java index 763f1b79ef9..0c3e0d71705 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java @@ -153,7 +153,7 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { final StringBuilder memberOfFilter = new StringBuilder(); if ("GROUP".equals(type)) { - memberOfFilter.append("(memberof="); + memberOfFilter.append("(").append(getMemberOfAttribute()).append("="); memberOfFilter.append(name); memberOfFilter.append(")"); } @@ -167,6 +167,11 @@ public class OpenLdapUserManagerImpl implements LdapUserManager { return searchUser(basedn, searchQuery.toString(), context); } + + protected String getMemberOfAttribute() { + return "memberof"; + } + @Override public List getUsers(final LdapContext context) throws NamingException, IOException { return getUsers(null, context); diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index a8745bb6e72..634c2996143 100644 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -2173,6 +2173,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (domain != null) { domainName = domain.getName(); } + if (userAccount == null) { + _userAccountDao.getUserAccount(username, domainId); + } if (!userAccount.getState().equalsIgnoreCase(Account.State.enabled.toString()) || !userAccount.getAccountState().equalsIgnoreCase(Account.State.enabled.toString())) { @@ -2192,6 +2195,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M s_logger.debug("Unable to authenticate user with username " + username + " in domain " + domainId); } + if (userAccount == null) { + s_logger.warn("Unable to find an user with username " + username + " in domain " + domainId); + return null; + } + if (userAccount.getState().equalsIgnoreCase(Account.State.enabled.toString())) { if (!isInternalAccount(userAccount.getId())) { // Internal accounts are not disabled diff --git a/setup/db/db/schema-452to460.sql b/setup/db/db/schema-452to460.sql index af313646bde..bf0c5c5e3ae 100644 --- a/setup/db/db/schema-452to460.sql +++ b/setup/db/db/schema-452to460.sql @@ -404,6 +404,7 @@ CREATE TABLE `cloud`.`ldap_trust_map` ( `domain_id` bigint unsigned NOT NULL, `type` varchar(10) NOT NULL, `name` varchar(255) NOT NULL, + `account_type` int(1) unsigned NOT NULL, PRIMARY KEY (`id`), UNIQUE KEY `uk_ldap_trust_map__domain_id` (`id`), KEY `fk_ldap_trust_map__domain_id` (`domain_id`), From 59291864fc893935294fc9a8ac60c6c537a7caff Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Tue, 11 Aug 2015 14:27:55 +0530 Subject: [PATCH 05/16] CLOUDSTACK-8647 added nested group enabled config in ldap querying the nested groups only when nested groups are enabled --- .../ldap/ADLdapUserManagerImpl.java | 9 ++++-- .../cloudstack/ldap/LdapAuthenticator.java | 32 +++++++++++++------ .../cloudstack/ldap/LdapConfiguration.java | 9 +++++- .../com/cloud/user/AccountManagerImpl.java | 4 +-- 4 files changed, 38 insertions(+), 16 deletions(-) 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 89a278191a4..5570084c5d4 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 @@ -33,6 +33,7 @@ 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:"; + private static final String MICROSOFT_AD_MEMBERS_FILTER = "memberOf"; @Override public List getUsersInGroup(String groupName, LdapContext context) throws NamingException { @@ -66,7 +67,7 @@ public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl implements Ld final StringBuilder memberOfFilter = new StringBuilder(); String groupCnName = _ldapConfiguration.getCommonNameAttribute() + "=" +groupName + "," + _ldapConfiguration.getBaseDn(); - memberOfFilter.append("(" + MICROSOFT_AD_NESTED_MEMBERS_FILTER + "="); + memberOfFilter.append("(").append(getMemberOfAttribute()).append("="); memberOfFilter.append(groupCnName); memberOfFilter.append(")"); @@ -94,6 +95,10 @@ public class ADLdapUserManagerImpl extends OpenLdapUserManagerImpl implements Ld } protected String getMemberOfAttribute() { - return MICROSOFT_AD_NESTED_MEMBERS_FILTER; + if(_ldapConfiguration.isNestedGroupsEnabled()) { + return MICROSOFT_AD_NESTED_MEMBERS_FILTER; + } else { + return MICROSOFT_AD_MEMBERS_FILTER; + } } } 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 a04868eca01..7599dadffba 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 @@ -17,7 +17,8 @@ package org.apache.cloudstack.ldap; import com.cloud.server.auth.DefaultUserAuthenticator; -import com.cloud.user.AccountService; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; import com.cloud.user.User; import com.cloud.user.UserAccount; import com.cloud.user.dao.UserAccountDao; @@ -37,7 +38,7 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { @Inject private UserAccountDao _userAccountDao; @Inject - public AccountService _accountService; + private AccountManager _accountManager; public LdapAuthenticator() { super(); @@ -68,13 +69,17 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { LdapUser ldapUser = _ldapManager.getUser(username, ldapTrustMapVO.getType(), ldapTrustMapVO.getName()); if(!ldapUser.isDisabled()) { result = _ldapManager.canAuthenticate(ldapUser.getPrincipal(), password); - if(result && (user == null)) { - // import user to cloudstack - createCloudStackUserAccount(ldapUser, domainId, ldapTrustMapVO.getAccountType()); + if(result) { + if(user == null) { + // import user to cloudstack + createCloudStackUserAccount(ldapUser, domainId, ldapTrustMapVO.getAccountType()); + } else { + enableUserInCloudStack(user); + } } } else { //disable user in cloudstack - disableUserInCloudStack(ldapUser, domainId); + disableUserInCloudStack(user); } } catch (NoLdapUserMatchingQueryException e) { s_logger.debug(e.getMessage()); @@ -103,15 +108,22 @@ public class LdapAuthenticator extends DefaultUserAuthenticator { return new Pair(result, action); } + private void enableUserInCloudStack(UserAccount user) { + if(user != null && (user.getState().equalsIgnoreCase(Account.State.disabled.toString()))) { + _accountManager.enableUser(user.getId()); + } + } + private void createCloudStackUserAccount(LdapUser user, long domainId, short accountType) { String username = user.getUsername(); - _accountService.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), null, username, accountType, domainId, username, null, + _accountManager.createUserAccount(username, "", user.getFirstname(), user.getLastname(), user.getEmail(), null, username, accountType, domainId, username, null, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); } - private void disableUserInCloudStack(LdapUser ldapUser, long domainId) { - final UserAccount user = _userAccountDao.getUserAccount(ldapUser.getUsername(), domainId); - _accountService.lockUser(user.getId()); + private void disableUserInCloudStack(UserAccount user) { + if (user != null) { + _accountManager.disableUser(user.getId()); + } } @Override 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 95019015442..56b39a8b3d1 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 @@ -39,6 +39,9 @@ public class LdapConfiguration implements Configurable{ private static final ConfigKey ldapProvider = new ConfigKey(String.class, "ldap.provider", "Advanced", "openldap", "ldap provider ex:openldap, microsoftad", true, ConfigKey.Scope.Global, null); + private static final ConfigKey ldapEnableNestedGroups = new ConfigKey(Boolean.class, "ldap.nested.groups.enable", "Advanced", "true", + "if true, nested groups will also be queried", true, ConfigKey.Scope.Global, null); + private final static int scope = SearchControls.SUBTREE_SCOPE; @Inject @@ -183,6 +186,10 @@ public class LdapConfiguration implements Configurable{ return provider; } + public boolean isNestedGroupsEnabled() { + return ldapEnableNestedGroups.value(); + } + @Override public String getConfigComponentName() { return LdapConfiguration.class.getSimpleName(); @@ -190,6 +197,6 @@ public class LdapConfiguration implements Configurable{ @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {ldapReadTimeout, ldapPageSize, ldapProvider}; + return new ConfigKey[] {ldapReadTimeout, ldapPageSize, ldapProvider, ldapEnableNestedGroups}; } } \ No newline at end of file diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 634c2996143..edc8ad87b78 100644 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -2173,9 +2173,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (domain != null) { domainName = domain.getName(); } - if (userAccount == null) { - _userAccountDao.getUserAccount(username, domainId); - } + userAccount = _userAccountDao.getUserAccount(username, domainId); if (!userAccount.getState().equalsIgnoreCase(Account.State.enabled.toString()) || !userAccount.getAccountState().equalsIgnoreCase(Account.State.enabled.toString())) { From 2825c07b38795ff541d4e9dc648612ce84fd728f Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Tue, 11 Aug 2015 15:20:22 +0530 Subject: [PATCH 06/16] CLOUDSTACK-8647 support for assigning and admin to linked ldap domain if an admin username is given to the linkDomainToLdap, added support to import this user User will be imported only if the user is available in the group/ou in ldap and an account with the name doesnt exist in cloudstack. on successful import, accountid will be returned in response. --- .../api/command/LinkDomainToLdapCmd.java | 31 +++++++++++++++++++ .../response/LinkDomainToLdapResponse.java | 12 +++++++ 2 files changed, 43 insertions(+) diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java index 5a76e8ec20a..f5a0ef82a2f 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java @@ -21,6 +21,9 @@ package org.apache.cloudstack.api.command; import javax.inject.Inject; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.user.AccountService; +import com.cloud.user.User; +import com.cloud.user.UserAccount; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -30,10 +33,14 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.api.response.LinkDomainToLdapResponse; import org.apache.cloudstack.ldap.LdapManager; +import org.apache.cloudstack.ldap.LdapUser; +import org.apache.cloudstack.ldap.NoLdapUserMatchingQueryException; import org.apache.log4j.Logger; import com.cloud.user.Account; +import java.util.UUID; + @APICommand(name = "linkDomainToLdap", description = "link an existing cloudstack domain to group or OU in ldap", responseObject = LinkDomainToLdapResponse.class, since = "4.6.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class LinkDomainToLdapCmd extends BaseCmd { @@ -59,10 +66,34 @@ public class LinkDomainToLdapCmd extends BaseCmd { @Inject private LdapManager _ldapManager; + @Inject + public AccountService _accountService; + @Override public void execute() throws ServerApiException { try { LinkDomainToLdapResponse response = _ldapManager.linkDomainToLdap(domainId, type, name, accountType); + if(admin!=null) { + try { + LdapUser ldapUser = _ldapManager.getUser(admin, type, name); + if(!ldapUser.isDisabled()) { + Account account = _accountService.getActiveAccountByName(admin, domainId); + if (account == null) { + UserAccount userAccount = + _accountService.createUserAccount(admin, "", ldapUser.getFirstname(), ldapUser.getLastname(), ldapUser.getEmail(), null, admin, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, domainId, admin, null, UUID.randomUUID().toString(), + UUID.randomUUID().toString(), User.Source.LDAP); + response.setAdminId(String.valueOf(userAccount.getAccountId())); + s_logger.info("created an account with name " + admin + " in the given domain " + domainId); + } else { + s_logger.debug("an account with name " + admin + " already exists in the domain " + domainId); + } + } else { + s_logger.debug("ldap user with username "+admin+" is disabled in the given group/ou"); + } + } catch (NoLdapUserMatchingQueryException e) { + s_logger.debug("no ldap user matching username " + admin + " in the given group/ou"); + } + } response.setObjectName("LinkDomainToLdap"); response.setResponseName(getCommandName()); setResponseObject(response); diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java index 103fb25e540..b0032b04b4d 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java @@ -41,6 +41,10 @@ public class LinkDomainToLdapResponse extends BaseResponse { @Param(description = "Type of the account to auto import") private short accountType; + @SerializedName(ApiConstants.ACCOUNT_ID) + @Param(description = "Domain Admin accountId that is created") + private String adminId; + public LinkDomainToLdapResponse(long domainId, String type, String name, short accountType) { this.domainId = domainId; this.name = name; @@ -63,4 +67,12 @@ public class LinkDomainToLdapResponse extends BaseResponse { public short getAccountType() { return accountType; } + + public String getAdminId() { + return adminId; + } + + public void setAdminId(String adminId) { + this.adminId = adminId; + } } From 6cb5d3a3bc4c02071fb1b1005f9fd40b7468d6e3 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Wed, 12 Aug 2015 10:42:11 +0530 Subject: [PATCH 07/16] CLOUDSTACK-8647: fixed unittests --- .../ldap/ADLdapUserManagerImplSpec.groovy | 17 ++++++++++- .../ldap/LdapAuthenticatorSpec.groovy | 7 +++++ .../ldap/LdapConfigurationSpec.groovy | 2 +- .../ldap/LdapCreateAccountCmdSpec.groovy | 10 +++---- .../ldap/LdapImportUsersCmdSpec.groovy | 26 ++++++++--------- .../ldap/LdapListUsersCmdSpec.groovy | 6 ++-- .../ldap/LdapManagerImplSpec.groovy | 28 +++++-------------- .../ldap/LdapSearchUserCmdSpec.groovy | 2 +- .../cloudstack/ldap/LdapUserSpec.groovy | 22 +++++++-------- 9 files changed, 64 insertions(+), 56 deletions(-) diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/ADLdapUserManagerImplSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/ADLdapUserManagerImplSpec.groovy index 6fafa3edf22..93b1b17a460 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/ADLdapUserManagerImplSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/ADLdapUserManagerImplSpec.groovy @@ -39,10 +39,11 @@ class ADLdapUserManagerImplSpec extends spock.lang.Specification { adLdapUserManager._ldapConfiguration = ldapConfiguration; } - def "test generate AD search filter"() { + def "test generate AD search filter with nested groups enabled"() { ldapConfiguration.getUserObject() >> "user" ldapConfiguration.getCommonNameAttribute() >> "CN" ldapConfiguration.getBaseDn() >> "DC=cloud,DC=citrix,DC=com" + ldapConfiguration.isNestedGroupsEnabled() >> true def result = adLdapUserManager.generateADGroupSearchFilter(group); expect: @@ -52,6 +53,20 @@ class ADLdapUserManagerImplSpec extends spock.lang.Specification { group << ["dev", "dev-hyd"] } + def "test generate AD search filter with nested groups disabled"() { + ldapConfiguration.getUserObject() >> "user" + ldapConfiguration.getCommonNameAttribute() >> "CN" + ldapConfiguration.getBaseDn() >> "DC=cloud,DC=citrix,DC=com" + ldapConfiguration.isNestedGroupsEnabled() >> false + + def result = adLdapUserManager.generateADGroupSearchFilter(group); + expect: + assert result.contains("memberOf=") + result == "(&(objectClass=user)(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))" + where: + group << ["dev", "dev-hyd"] + } + def "test getUsersInGroup null group"() { ldapConfiguration.getScope() >> SearchControls.SUBTREE_SCOPE ldapConfiguration.getReturnAttributes() >> ["username", "firstname", "lastname", "email"] diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy index 51f8e84559a..e38a0310eea 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy @@ -22,6 +22,7 @@ import com.cloud.utils.Pair import org.apache.cloudstack.ldap.LdapAuthenticator import org.apache.cloudstack.ldap.LdapConfigurationVO import org.apache.cloudstack.ldap.LdapManager +import org.apache.cloudstack.ldap.LdapUser class LdapAuthenticatorSpec extends spock.lang.Specification { @@ -40,7 +41,10 @@ class LdapAuthenticatorSpec extends spock.lang.Specification { def "Test failed authentication due to ldap bind being unsuccessful"() { given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator" def ldapManager = Mock(LdapManager) + def ldapUser = Mock(LdapUser) + ldapUser.isDisabled() >> false ldapManager.isLdapEnabled() >> true + ldapManager.getUser("rmurphy") >> ldapUser ldapManager.canAuthenticate(_, _) >> false UserAccountDao userAccountDao = Mock(UserAccountDao) @@ -72,8 +76,11 @@ class LdapAuthenticatorSpec extends spock.lang.Specification { def "Test successful authentication"() { given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator" def ldapManager = Mock(LdapManager) + def ldapUser = Mock(LdapUser) + ldapUser.isDisabled() >> false ldapManager.isLdapEnabled() >> true ldapManager.canAuthenticate(_, _) >> true + ldapManager.getUser("rmurphy") >> ldapUser UserAccountDao userAccountDao = Mock(UserAccountDao) userAccountDao.getUserAccount(_, _) >> new UserAccountVO() 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 4f93adff435..6f967cc6d8b 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 @@ -127,7 +127,7 @@ class LdapConfigurationSpec extends spock.lang.Specification { when: "Get return attributes is called" String[] returnAttributes = ldapConfiguration.getReturnAttributes() then: "An array containing uid, mail, givenname, sn and cn is returned" - returnAttributes == ["uid", "mail", "givenname", "sn", "cn"] + returnAttributes == ["uid", "mail", "givenname", "sn", "cn", "userAccountControl"] } def "Test that getScope returns SearchControls.SUBTREE_SCOPE"() { diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy index 1f17e704f52..a0b20bbcb13 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapCreateAccountCmdSpec.groovy @@ -53,7 +53,7 @@ class LdapCreateAccountCmdSpec extends spock.lang.Specification { def "Test failed creation due to a null response from cloudstack account creater"() { given: "We have an LdapManager, AccountService and LdapCreateAccountCmd" LdapManager ldapManager = Mock(LdapManager) - ldapManager.getUser(_) >> new LdapUser("rmurphy", "rmurphy@cloudstack.org", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") + ldapManager.getUser(_) >> new LdapUser("rmurphy", "rmurphy@cloudstack.org", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false) AccountService accountService = Mock(AccountService) def ldapCreateAccountCmd = Spy(LdapCreateAccountCmd, constructorArgs: [ldapManager, accountService]) ldapCreateAccountCmd.getCurrentContext() >> Mock(CallContext) @@ -105,7 +105,7 @@ class LdapCreateAccountCmdSpec extends spock.lang.Specification { AccountService accountService = Mock(AccountService) def ldapCreateAccountCmd = new LdapCreateAccountCmd(ldapManager, accountService); when: "a user with an username, email, firstname and lastname is validated" - def result = ldapCreateAccountCmd.validateUser(new LdapUser("username", "email", "firstname", "lastname", "principal", "domain")) + def result = ldapCreateAccountCmd.validateUser(new LdapUser("username", "email", "firstname", "lastname", "principal", "domain", false)) then: "the result is true" result == true } @@ -116,7 +116,7 @@ class LdapCreateAccountCmdSpec extends spock.lang.Specification { AccountService accountService = Mock(AccountService) def ldapCreateAccountCmd = new LdapCreateAccountCmd(ldapManager, accountService) when: "A user with no email address attempts to validate" - ldapCreateAccountCmd.validateUser(new LdapUser("username", null, "firstname", "lastname", "principal", "domain")) + ldapCreateAccountCmd.validateUser(new LdapUser("username", null, "firstname", "lastname", "principal", "domain", false)) then: "An exception is thrown" thrown Exception } @@ -127,7 +127,7 @@ class LdapCreateAccountCmdSpec extends spock.lang.Specification { AccountService accountService = Mock(AccountService) def ldapCreateAccountCmd = new LdapCreateAccountCmd(ldapManager, accountService) when: "A user with no firstname attempts to validate" - ldapCreateAccountCmd.validateUser(new LdapUser("username", "email", null, "lastname", "principal")) + ldapCreateAccountCmd.validateUser(new LdapUser("username", "email", null, "lastname", "principal", false)) then: "An exception is thrown" thrown Exception } @@ -138,7 +138,7 @@ class LdapCreateAccountCmdSpec extends spock.lang.Specification { AccountService accountService = Mock(AccountService) def ldapCreateAccountCmd = new LdapCreateAccountCmd(ldapManager, accountService) when: "A user with no lastname attempts to validate" - ldapCreateAccountCmd.validateUser(new LdapUser("username", "email", "firstname", null, "principal", "domain")) + ldapCreateAccountCmd.validateUser(new LdapUser("username", "email", "firstname", null, "principal", "domain", false)) then: "An exception is thown" thrown Exception } diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapImportUsersCmdSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapImportUsersCmdSpec.groovy index 6e0759f85c1..514caeb17da 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapImportUsersCmdSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapImportUsersCmdSpec.groovy @@ -53,8 +53,8 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { def accountService = Mock(AccountService) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) - users.add(new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) + users.add(new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) ldapManager.getUsers() >> users LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") LdapUserResponse response2 = new LdapUserResponse("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering") @@ -81,8 +81,8 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { def accountService = Mock(AccountService) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) - users.add(new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) + users.add(new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) ldapManager.getUsersInGroup("TestGroup") >> users LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") LdapUserResponse response2 = new LdapUserResponse("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering") @@ -110,8 +110,8 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { def accountService = Mock(AccountService) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) - users.add(new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) + users.add(new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) ldapManager.getUsersInGroup("TestGroup") >> users LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") LdapUserResponse response2 = new LdapUserResponse("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering") @@ -139,8 +139,8 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { def accountService = Mock(AccountService) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) - users.add(new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) + users.add(new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) ldapManager.getUsers() >> users LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") LdapUserResponse response2 = new LdapUserResponse("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering") @@ -169,8 +169,8 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { ldapImportUsersCmd.domainId = varDomainId ldapImportUsersCmd.groupName = varGroupName - def ldapUser1 = new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") - def ldapUser2 = new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering"); + def ldapUser1 = new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false) + def ldapUser2 = new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering", false); Domain domain = new DomainVO(expectedDomainName, 1L, 1L, expectedDomainName, UUID.randomUUID().toString()); if (varDomainId != null) { @@ -204,7 +204,7 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { given: "We have an LdapManager, DomainService, two users and a LdapImportUsersCmd" def ldapManager = Mock(LdapManager) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) ldapManager.getUsers() >> users LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") ldapManager.createLdapUserResponse(_) >>> response1 @@ -234,7 +234,7 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { given: "We have an LdapManager, DomainService, two users and a LdapImportUsersCmd" def ldapManager = Mock(LdapManager) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) ldapManager.getUsers() >> users LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") ldapManager.createLdapUserResponse(_) >>> response1 @@ -263,7 +263,7 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { given: "We have an LdapManager, DomainService, two users and a LdapImportUsersCmd" def ldapManager = Mock(LdapManager) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) ldapManager.getUsers() >> users LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering") ldapManager.createLdapUserResponse(_) >>> response1 diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapListUsersCmdSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapListUsersCmdSpec.groovy index 4b32eb1ecd6..5247a1ec895 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapListUsersCmdSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapListUsersCmdSpec.groovy @@ -53,7 +53,7 @@ class LdapListUsersCmdSpec extends spock.lang.Specification { given: "We have an LdapManager, one user, QueryService and a LdapListUsersCmd" def ldapManager = Mock(LdapManager) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null)) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null, false)) ldapManager.getUsers() >> users LdapUserResponse response = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) ldapManager.createLdapUserResponse(_) >> response @@ -92,7 +92,7 @@ class LdapListUsersCmdSpec extends spock.lang.Specification { queryService.searchForUsers(_) >> queryServiceResponse - def ldapUser = new LdapUser("rmurphy", "rmurphy@cloudstack.org", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) + def ldapUser = new LdapUser("rmurphy", "rmurphy@cloudstack.org", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null, false) def ldapListUsersCmd = new LdapListUsersCmd(ldapManager,queryService) when: "isACloudstackUser is executed" @@ -109,7 +109,7 @@ class LdapListUsersCmdSpec extends spock.lang.Specification { queryService.searchForUsers(_) >> new ListResponse() - def ldapUser = new LdapUser("rmurphy", "rmurphy@cloudstack.org", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) + def ldapUser = new LdapUser("rmurphy", "rmurphy@cloudstack.org", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null, false) def ldapListUsersCmd = new LdapListUsersCmd(ldapManager,queryService) when: "isACloudstackUser is executed" 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 4c62a4eb67e..b4a7267d480 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 @@ -24,6 +24,7 @@ import org.apache.cloudstack.api.command.LdapDeleteConfigurationCmd 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 javax.naming.NamingException import javax.naming.ldap.InitialLdapContext @@ -111,7 +112,7 @@ class LdapManagerImplSpec extends spock.lang.Specification { def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManagerFactory, ldapConfiguration) when: "A ldap user response is generated" def result = ldapManager.createLdapUserResponse(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", - "engineering")) + "engineering", false)) then: "The result of the response should match the given ldap user" result.username == "rmurphy" result.email == "rmurphy@test.com" @@ -131,7 +132,7 @@ class LdapManagerImplSpec extends spock.lang.Specification { ldapUserManagerFactory.getInstance(_) >> ldapUserManager ldapContextFactory.createBindContext() >> null List users = new ArrayList<>(); - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null)) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null, false)) ldapUserManager.getUsers(_) >> users; def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManagerFactory, ldapConfiguration) when: "We search for a group of users" @@ -149,7 +150,7 @@ class LdapManagerImplSpec extends spock.lang.Specification { def ldapConfiguration = Mock(LdapConfiguration) ldapUserManagerFactory.getInstance(_) >> ldapUserManager ldapContextFactory.createBindContext() >> null - ldapUserManager.getUser(_, _) >> new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) + ldapUserManager.getUser(_, _) >> new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null, false) def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManagerFactory, ldapConfiguration) when: "We search for a user" def result = ldapManager.getUser("rmurphy") @@ -194,22 +195,6 @@ class LdapManagerImplSpec extends spock.lang.Specification { result == false } - def "Test successful failed result from canAuthenticate due to user not found"() { - given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager" - def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl) - def ldapContextFactory = Mock(LdapContextFactory) - def ldapUserManager = Mock(LdapUserManager) - def ldapUserManagerFactory = Mock(LdapUserManagerFactory) - ldapUserManagerFactory.getInstance(_) >> ldapUserManager - def ldapConfiguration = Mock(LdapConfiguration) - def ldapManager = Spy(LdapManagerImpl, constructorArgs: [ldapConfigurationDao, ldapContextFactory, ldapUserManagerFactory, ldapConfiguration]) - ldapManager.getUser(_) >> { throw new NamingException() } - when: "The user attempts to authenticate and the user is not found" - def result = ldapManager.canAuthenticate("rmurphy", "password") - then: "the authentication fails" - result == false - } - def "Test successful failed result from deleteConfiguration due to configuration not existing"() { given: "We have an LdapConfigurationDao, LdapContextFactory, LdapUserManager and LdapManager" def ldapConfigurationDao = Mock(LdapConfigurationDaoImpl) @@ -293,7 +278,7 @@ class LdapManagerImplSpec extends spock.lang.Specification { ldapContextFactory.createBindContext() >> null; List users = new ArrayList(); - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering", false)) ldapUserManager.getUsers(_, _) >> users; def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManagerFactory, ldapConfiguration) @@ -364,6 +349,7 @@ class LdapManagerImplSpec extends spock.lang.Specification { cmdList.add(LdapImportUsersCmd.class); cmdList.add(LDAPConfigCmd.class); cmdList.add(LDAPRemoveCmd.class); + cmdList.add(LinkDomainToLdapCmd.class) return cmdList } @@ -434,7 +420,7 @@ class LdapManagerImplSpec extends spock.lang.Specification { ldapUserManagerFactory.getInstance(_) >> ldapUserManager ldapContextFactory.createBindContext() >> null List users = new ArrayList<>(); - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", "engineering")) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", "engineering", false)) ldapUserManager.getUsersInGroup("engineering", _) >> users; def ldapManager = new LdapManagerImpl(ldapConfigurationDao, ldapContextFactory, ldapUserManagerFactory, ldapConfiguration) when: "We search for a group of users" diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapSearchUserCmdSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapSearchUserCmdSpec.groovy index 1411c29f7b4..55510875899 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapSearchUserCmdSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapSearchUserCmdSpec.groovy @@ -48,7 +48,7 @@ class LdapSearchUserCmdSpec extends spock.lang.Specification { given: "We have an Ldap manager and ldap user search cmd" def ldapManager = Mock(LdapManager) List users = new ArrayList() - users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null)) + users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null, false)) ldapManager.searchUsers(_) >> users LdapUserResponse response = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,dc=cloudstack,dc=org", null) ldapManager.createLdapUserResponse(_) >> response diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserSpec.groovy index 6df947be22a..8ddfc9a23b8 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapUserSpec.groovy @@ -22,7 +22,7 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing LdapUsers hashCode generation"() { given: - def userA = new LdapUser(usernameA, "", "", "", "", "") + def userA = new LdapUser(usernameA, "", "", "", "", "", false) expect: userA.hashCode() == usernameA.hashCode() where: @@ -31,8 +31,8 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing that LdapUser successfully gives the correct result for a compare to"() { given: "You have created two LDAP user objects" - def userA = new LdapUser(usernameA, "", "", "", "", "") - def userB = new LdapUser(usernameB, "", "", "", "", "") + def userA = new LdapUser(usernameA, "", "", "", "", "", false) + def userB = new LdapUser(usernameB, "", "", "", "", "", false) expect: "That when compared the result is less than or equal to 0" userA.compareTo(userB) <= 0 where: "The following values are used" @@ -43,8 +43,8 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing that LdapUsers equality"() { given: - def userA = new LdapUser(usernameA, "", "", "", "", "") - def userB = new LdapUser(usernameB, "", "", "", "", "") + def userA = new LdapUser(usernameA, "", "", "", "", "", false) + def userB = new LdapUser(usernameB, "", "", "", "", "", false) expect: userA.equals(userA) == true userA.equals(new Object()) == false @@ -56,7 +56,7 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing that the username is correctly set with the ldap object"() { given: "You have created a LDAP user object with a username" - def user = new LdapUser(username, "", "", "", "", "") + def user = new LdapUser(username, "", "", "", "", "", false) expect: "The username is equal to the given data source" user.getUsername() == username where: "The username is set to " @@ -65,7 +65,7 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing the email is correctly set with the ldap object"() { given: "You have created a LDAP user object with a email" - def user = new LdapUser("", email, "", "", "", "") + def user = new LdapUser("", email, "", "", "", "", false) expect: "The email is equal to the given data source" user.getEmail() == email where: "The email is set to " @@ -74,7 +74,7 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing the firstname is correctly set with the ldap object"() { given: "You have created a LDAP user object with a firstname" - def user = new LdapUser("", "", firstname, "", "", "") + def user = new LdapUser("", "", firstname, "", "", "", false) expect: "The firstname is equal to the given data source" user.getFirstname() == firstname where: "The firstname is set to " @@ -83,7 +83,7 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing the lastname is correctly set with the ldap object"() { given: "You have created a LDAP user object with a lastname" - def user = new LdapUser("", "", "", lastname, "", "") + def user = new LdapUser("", "", "", lastname, "", "", false) expect: "The lastname is equal to the given data source" user.getLastname() == lastname where: "The lastname is set to " @@ -92,7 +92,7 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing the principal is correctly set with the ldap object"() { given: "You have created a LDAP user object with a principal" - def user = new LdapUser("", "", "", "", principal, "") + def user = new LdapUser("", "", "", "", principal, "", false) expect: "The principal is equal to the given data source" user.getPrincipal() == principal where: "The principal is set to " @@ -101,7 +101,7 @@ class LdapUserSpec extends spock.lang.Specification { def "Testing the domain is correctly set with the ldap object"() { given: "You have created a LDAP user object with a principal" - def user = new LdapUser("", "", "", "", "", domain) + def user = new LdapUser("", "", "", "", "", domain, false) expect: "The principal is equal to the given data source" user.getDomain() == domain where: "The username is set to " From dd6d6d18a588e7884385c74b98c635b487ea4e93 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Mon, 24 Aug 2015 14:30:37 +0530 Subject: [PATCH 08/16] CLOUDSTACK-8647 unittests for LinkDomainToLdap api command --- .../ldap/LinkDomainToLdapCmdSpec.groovy | 165 ++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy new file mode 100644 index 00000000000..dbf92fbba89 --- /dev/null +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy @@ -0,0 +1,165 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package groovy.org.apache.cloudstack.ldap + +import com.cloud.exception.InvalidParameterValueException +import com.cloud.user.Account +import com.cloud.user.AccountService +import com.cloud.user.User +import com.cloud.user.UserAccount +import org.apache.cloudstack.api.ServerApiException +import org.apache.cloudstack.api.command.LinkDomainToLdapCmd +import org.apache.cloudstack.api.response.LinkDomainToLdapResponse +import org.apache.cloudstack.ldap.LdapManager +import org.apache.cloudstack.ldap.LdapUser +import spock.lang.Shared +import spock.lang.Specification + +class LinkDomainToLdapCmdSpec extends Specification { + + @Shared + private LdapManager _ldapManager; + + @Shared + public AccountService _accountService; + + @Shared + public LinkDomainToLdapCmd linkDomainToLdapCmd; + + def setup() { + _ldapManager = Mock(LdapManager) + _accountService = Mock(AccountService) + + linkDomainToLdapCmd = new LinkDomainToLdapCmd() + linkDomainToLdapCmd._accountService = _accountService + linkDomainToLdapCmd._ldapManager = _ldapManager + } + + def "test invalid params"() { + _ldapManager.linkDomainToLdap(_,_,_,_) >> {throw new InvalidParameterValueException("invalid param")} + when: + linkDomainToLdapCmd.execute(); + then: + thrown(ServerApiException) + } + def "test valid params without admin"(){ + LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(1, "GROUP", "CN=test,DC=ccp,DC=citrix,DC=com", (short)2) + _ldapManager.linkDomainToLdap(_,_,_,_) >> response + when: + linkDomainToLdapCmd.execute() + then: + LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject() + result.getObjectName() == "LinkDomainToLdap" + result.getResponseName() == linkDomainToLdapCmd.getCommandName() + } + + def "test with valid params and with disabled admin"() { + def domainId = 1; + def type = "GROUP"; + def name = "CN=test,DC=ccp,DC=Citrix,DC=com" + def accountType = 2; + def username = "admin" + + LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType) + _ldapManager.linkDomainToLdap(_,_,_,_) >> response + _ldapManager.getUser(username, type, name) >> new LdapUser(username, "admin@ccp.citrix.com", "Admin", "Admin", name, "ccp", true) + + linkDomainToLdapCmd.admin = username + linkDomainToLdapCmd.type = type + linkDomainToLdapCmd.name = name + linkDomainToLdapCmd.domainId = domainId + + when: + linkDomainToLdapCmd.execute() + then: + LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject() + result.getObjectName() == "LinkDomainToLdap" + result.getResponseName() == linkDomainToLdapCmd.getCommandName() + result.getDomainId() == domainId + result.getType() == type + result.getName() == name + result.getAdminId() == null + } + + def "test with valid params and with admin who exist in cloudstack already"() { + def domainId = 1; + def type = "GROUP"; + def name = "CN=test,DC=ccp,DC=Citrix,DC=com" + def accountType = 2; + def username = "admin" + + LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType) + _ldapManager.linkDomainToLdap(_,_,_,_) >> response + _ldapManager.getUser(username, type, name) >> new LdapUser(username, "admin@ccp.citrix.com", "Admin", "Admin", name, "ccp", false) + + _accountService.getActiveAccountByName(username, domainId) >> Mock(Account) + + linkDomainToLdapCmd.admin = username + linkDomainToLdapCmd.type = type + linkDomainToLdapCmd.name = name + linkDomainToLdapCmd.domainId = domainId + + when: + linkDomainToLdapCmd.execute() + then: + LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject() + result.getObjectName() == "LinkDomainToLdap" + result.getResponseName() == linkDomainToLdapCmd.getCommandName() + result.getDomainId() == domainId + result.getType() == type + result.getName() == name + result.getAdminId() == null + } + + def "test with valid params and with admin who doesnt exist in cloudstack"() { + def domainId = 1; + def type = "GROUP"; + def name = "CN=test,DC=ccp,DC=Citrix,DC=com" + def accountType = 2; + def username = "admin" + def accountId = 24 + + LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType) + _ldapManager.linkDomainToLdap(_,_,_,_) >> response + _ldapManager.getUser(username, type, name) >> new LdapUser(username, "admin@ccp.citrix.com", "Admin", "Admin", name, "ccp", false) + + _accountService.getActiveAccountByName(username, domainId) >> null + UserAccount userAccount = Mock(UserAccount) + userAccount.getAccountId() >> 24 + _accountService.createUserAccount(username, "", "Admin", "Admin", "admin@ccp.citrix.com", null, username, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, domainId, + username, null, _, _, User.Source.LDAP) >> userAccount + + linkDomainToLdapCmd.admin = username + linkDomainToLdapCmd.type = type + linkDomainToLdapCmd.name = name + linkDomainToLdapCmd.domainId = domainId + + when: + linkDomainToLdapCmd.execute() + then: + LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject() + result.getObjectName() == "LinkDomainToLdap" + result.getResponseName() == linkDomainToLdapCmd.getCommandName() + result.getDomainId() == domainId + result.getType() == type + result.getName() == name + result.getAdminId() == String.valueOf(accountId) + } + +} From 6572abc7b3095d07d003498e06c85bef1fe9dfd9 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Mon, 24 Aug 2015 16:51:36 +0530 Subject: [PATCH 09/16] CLOUDSTACK-8647 added unittests for new methods in ldapmanager --- .../cloudstack/ldap/LdapAuthenticator.java | 2 +- .../apache/cloudstack/ldap/LdapManager.java | 2 + .../cloudstack/ldap/LdapManagerImpl.java | 11 +- .../cloudstack/ldap/LdapTrustMapVO.java | 39 ++++- .../ldap/LdapManagerImplSpec.groovy | 154 ++++++++++++++++++ 5 files changed, 202 insertions(+), 6 deletions(-) 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 + } } From 36340d97bdecbb437dcbfd5ef30b49020ef681a5 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Thu, 27 Aug 2015 14:41:37 +0530 Subject: [PATCH 10/16] CLOUDSTACK-8647: UI for trust AD feature --- .../classes/resources/messages.properties | 4 + setup/db/db/schema-452to460.sql | 2 +- ui/css/cloudstack3.css | 8 ++ ui/dictionary2.jsp | 4 + ui/scripts/docs.js | 11 ++ ui/scripts/domains.js | 106 ++++++++++++++++++ 6 files changed, 134 insertions(+), 1 deletion(-) diff --git a/client/WEB-INF/classes/resources/messages.properties b/client/WEB-INF/classes/resources/messages.properties index 492b46ee8b4..49e8b50386e 100644 --- a/client/WEB-INF/classes/resources/messages.properties +++ b/client/WEB-INF/classes/resources/messages.properties @@ -2133,6 +2133,10 @@ label.every=Every label.day=Day label.of.month=of month label.add.private.gateway=Add Private Gateway +label.link.domain.to.ldap=Link Domain to LDAP +message.link.domain.to.ldap=Enable autosync for this domain in LDAP +label.ldap.link.type=Type +label.account.type=Account Type message.desc.created.ssh.key.pair=Created a SSH Key Pair. message.please.confirm.remove.ssh.key.pair=Please confirm that you want to remove this SSH Key Pair message.password.has.been.reset.to=Password has been reset to diff --git a/setup/db/db/schema-452to460.sql b/setup/db/db/schema-452to460.sql index bf0c5c5e3ae..3ca066015f4 100644 --- a/setup/db/db/schema-452to460.sql +++ b/setup/db/db/schema-452to460.sql @@ -406,7 +406,7 @@ CREATE TABLE `cloud`.`ldap_trust_map` ( `name` varchar(255) NOT NULL, `account_type` int(1) unsigned NOT NULL, PRIMARY KEY (`id`), - UNIQUE KEY `uk_ldap_trust_map__domain_id` (`id`), + UNIQUE KEY `uk_ldap_trust_map__domain_id` (`domain_id`), KEY `fk_ldap_trust_map__domain_id` (`domain_id`), CONSTRAINT `fk_ldap_trust_map__domain_id` FOREIGN KEY (`domain_id`) REFERENCES `domain` (`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/ui/css/cloudstack3.css b/ui/css/cloudstack3.css index 60ac9ca4d64..e7c93be3b6b 100644 --- a/ui/css/cloudstack3.css +++ b/ui/css/cloudstack3.css @@ -12851,6 +12851,14 @@ div.ui-dialog div.autoscaler div.field-group div.form-container form div.form-it background-position: -230px -677px; } +.linktoldap .icon { + background-position: -197px -65px; +} + +.linktoldap:hover .icon { + background-position: -197px -647px; +} + .label-hovered { cursor: pointer; color: #0000FF !important; diff --git a/ui/dictionary2.jsp b/ui/dictionary2.jsp index d415266bce6..66faa0d0619 100644 --- a/ui/dictionary2.jsp +++ b/ui/dictionary2.jsp @@ -1066,6 +1066,10 @@ under the License. 'label.ovm3.vip': '', 'label.local.file': '', 'label.local.storage.enabled.system.vms': '', +'label.link.domain.to.ldap': '', +'message.link.domain.to.ldap': '', +'label.ldap.link.type': '', +'label.account.type': '' 'label.create.ssh.key.pair': '', 'label.fingerprint': '', 'label.host.tag': '', diff --git a/ui/scripts/docs.js b/ui/scripts/docs.js index 809c398cd2f..ed6ab0c938c 100755 --- a/ui/scripts/docs.js +++ b/ui/scripts/docs.js @@ -1317,5 +1317,16 @@ cloudStack.docs = { helpOvm3Vip: { desc: 'The VIP used by the pool and cluster', externalLink: '' + }, + helpLdapGroupName: { + desc: 'Fully qualified name of OU/GROUP in LDAP', + externalLink: '' + }, + helpLdapGroupType: { + desc: 'Type of LDAP name provided. Can be either GROUP/OU', + externalLink: '' + }, + helpLdapLinkDomainAdmin: { + desc: 'domain admin of the linked domain. Specify a username in GROUP/OU of LDAP' } }; diff --git a/ui/scripts/domains.js b/ui/scripts/domains.js index 7f8220e7c0c..dcec93d53f4 100644 --- a/ui/scripts/domains.js +++ b/ui/scripts/domains.js @@ -313,6 +313,109 @@ } }, + linktoldap: { + label: 'label.link.domain.to.ldap', + + action: function(args) { + var data = { + domainid: args.context.domains[0].id, + type: args.data.type, + name: args.data.name, + accounttype: args.data.accounttype + }; + + if (args.data.admin != null && args.data.admin.length > 0) { + $.extend(data, { + admin: args.data.admin + }); + } + + $.ajax({ + url: createURL('linkDomainToLdap'), + data: data, + success: function(json) { + var item = json.linkdomaintoldapresponse.LinkDomainToLdap.domainid; + args.response.success({ + data: item + }); + }, + error: function(XMLHttpResponse) { + var errorMsg = parseXMLHttpResponse(XMLHttpResponse); + args.response.error(errorMsg); + } + }); + }, + + messages: { + notification: function(args) { + return 'label.link.domain.to.ldap'; + } + }, + + createForm: { + title: 'label.link.domain.to.ldap', + desc: 'message.link.domain.to.ldap', + fields: { + type: { + label: 'label.ldap.link.type', + docID: 'helpLdapGroupType', + validation: { + required: true + }, + select: function(args) { + var items = []; + items.push({ + id: "GROUP", + description: "GROUP" + }); //regular-user + items.push({ + id: "OU", + description: "OU" + }); //root-admin + args.response.success({ + data: items + }); + } + }, + name: { + label: 'label.name', + docID: 'helpLdapGroupName', + validation: { + required: true + } + }, + accounttype: { + label: 'label.account.type', + docID: 'helpAccountType', + validation: { + required: true + }, + select: function(args) { + var items = []; + items.push({ + id: 0, + description: "Normal User" + }); //regular-user + items.push({ + id: 2, + description: "Domain Admin" + }); //root-admin + args.response.success({ + data: items + }); + } + }, + admin: { + label: 'label.domain.admin', + docID: 'helpLdapLinkDomainAdmin', + validation: { + required: false + } + } + } + } + }, + updateResourceCount: { label: 'label.action.update.resource.count', messages: { @@ -652,6 +755,9 @@ if (jsonObj.level != 0) { //ROOT domain (whose level is 0) is not allowed to delete allowedActions.push("delete"); } + if(isLdapEnabled()) { + allowedActions.push("linktoldap") + } } else if (isDomainAdmin()) { if (args.context.domains[0].id != g_domainid) { allowedActions.push("edit"); //merge updateResourceLimit into edit From c2b36cb7059e91f6f5e9292cda4bb283017708cc Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Thu, 27 Aug 2015 16:18:47 +0530 Subject: [PATCH 11/16] CLOUDSTACK-8647: formatted LdapAuthenticatorSpec --- .../ldap/LdapAuthenticatorSpec.groovy | 104 +++++++++--------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy index e38a0310eea..435f9726f45 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy @@ -27,80 +27,80 @@ import org.apache.cloudstack.ldap.LdapUser class LdapAuthenticatorSpec extends spock.lang.Specification { def "Test a failed authentication due to user not being found within cloudstack"() { - given: "We have an LdapManager, userAccountDao and ldapAuthenticator and the user doesn't exist within cloudstack." + given: "We have an LdapManager, userAccountDao and ldapAuthenticator and the user doesn't exist within cloudstack." LdapManager ldapManager = Mock(LdapManager) UserAccountDao userAccountDao = Mock(UserAccountDao) userAccountDao.getUserAccount(_, _) >> null def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) - when: "A user authentications" + when: "A user authentications" def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null) - then: "their authentication fails" - result.first() == false + then: "their authentication fails" + result.first() == false } def "Test failed authentication due to ldap bind being unsuccessful"() { - given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator" - def ldapManager = Mock(LdapManager) - def ldapUser = Mock(LdapUser) - ldapUser.isDisabled() >> false - ldapManager.isLdapEnabled() >> true - ldapManager.getUser("rmurphy") >> ldapUser - ldapManager.canAuthenticate(_, _) >> false + given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator" + def ldapManager = Mock(LdapManager) + def ldapUser = Mock(LdapUser) + ldapUser.isDisabled() >> false + ldapManager.isLdapEnabled() >> true + ldapManager.getUser("rmurphy") >> ldapUser + ldapManager.canAuthenticate(_, _) >> false - UserAccountDao userAccountDao = Mock(UserAccountDao) - userAccountDao.getUserAccount(_, _) >> new UserAccountVO() - def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) + UserAccountDao userAccountDao = Mock(UserAccountDao) + userAccountDao.getUserAccount(_, _) >> new UserAccountVO() + def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) - when: "The user authenticates with an incorrect password" - def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null) + when: "The user authenticates with an incorrect password" + def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null) - then: "their authentication fails" - result.first() == false + then: "their authentication fails" + result.first() == false } def "Test failed authentication due to ldap not being configured"() { - given: "We have an LdapManager, A configured LDAP server, a userAccountDao and LdapAuthenticator" - def ldapManager = Mock(LdapManager) - ldapManager.isLdapEnabled() >> false + given: "We have an LdapManager, A configured LDAP server, a userAccountDao and LdapAuthenticator" + def ldapManager = Mock(LdapManager) + ldapManager.isLdapEnabled() >> false - UserAccountDao userAccountDao = Mock(UserAccountDao) - userAccountDao.getUserAccount(_, _) >> new UserAccountVO() + UserAccountDao userAccountDao = Mock(UserAccountDao) + userAccountDao.getUserAccount(_, _) >> new UserAccountVO() - def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) - when: "The user authenticates" - def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null) - then: "their authentication fails" - result.first() == false + def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) + when: "The user authenticates" + def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null) + then: "their authentication fails" + result.first() == false } - def "Test successful authentication"() { - given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator" - def ldapManager = Mock(LdapManager) - def ldapUser = Mock(LdapUser) - ldapUser.isDisabled() >> false - ldapManager.isLdapEnabled() >> true - ldapManager.canAuthenticate(_, _) >> true - ldapManager.getUser("rmurphy") >> ldapUser + def "Test successful authentication"() { + given: "We have an LdapManager, LdapConfiguration, userAccountDao and LdapAuthenticator" + def ldapManager = Mock(LdapManager) + def ldapUser = Mock(LdapUser) + ldapUser.isDisabled() >> false + ldapManager.isLdapEnabled() >> true + ldapManager.canAuthenticate(_, _) >> true + ldapManager.getUser("rmurphy") >> ldapUser - UserAccountDao userAccountDao = Mock(UserAccountDao) - userAccountDao.getUserAccount(_, _) >> new UserAccountVO() - def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) + UserAccountDao userAccountDao = Mock(UserAccountDao) + userAccountDao.getUserAccount(_, _) >> new UserAccountVO() + def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) - when: "The user authenticates with an incorrect password" - def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null) + when: "The user authenticates with an incorrect password" + def result = ldapAuthenticator.authenticate("rmurphy", "password", 0, null) - then: "their authentication passes" - result.first() == true - } + then: "their authentication passes" + result.first() == true + } def "Test that encode doesn't change the input"() { - given: "We have an LdapManager, userAccountDao and LdapAuthenticator" - LdapManager ldapManager = Mock(LdapManager) - UserAccountDao userAccountDao = Mock(UserAccountDao) - def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) - when: "a users password is encoded" - def result = ldapAuthenticator.encode("password") - then: "it doesn't change" - result == "password" + given: "We have an LdapManager, userAccountDao and LdapAuthenticator" + LdapManager ldapManager = Mock(LdapManager) + UserAccountDao userAccountDao = Mock(UserAccountDao) + def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) + when: "a users password is encoded" + def result = ldapAuthenticator.encode("password") + then: "it doesn't change" + result == "password" } } From 1c836a8999a28ef8d6161600a3a29586b30cb532 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Thu, 27 Aug 2015 17:24:40 +0530 Subject: [PATCH 12/16] CLOUDSTACK-8647: unittests for LdapAuthenticatorSpec --- .../ldap/LdapAuthenticatorSpec.groovy | 145 +++++++++++++++++- 1 file changed, 144 insertions(+), 1 deletion(-) diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy index 435f9726f45..ca19e8c633b 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapAuthenticatorSpec.groovy @@ -16,12 +16,17 @@ // under the License. package groovy.org.apache.cloudstack.ldap +import com.cloud.server.auth.UserAuthenticator +import com.cloud.user.Account +import com.cloud.user.AccountManager +import com.cloud.user.User +import com.cloud.user.UserAccount import com.cloud.user.UserAccountVO import com.cloud.user.dao.UserAccountDao import com.cloud.utils.Pair import org.apache.cloudstack.ldap.LdapAuthenticator -import org.apache.cloudstack.ldap.LdapConfigurationVO import org.apache.cloudstack.ldap.LdapManager +import org.apache.cloudstack.ldap.LdapTrustMapVO import org.apache.cloudstack.ldap.LdapUser class LdapAuthenticatorSpec extends spock.lang.Specification { @@ -103,4 +108,142 @@ class LdapAuthenticatorSpec extends spock.lang.Specification { then: "it doesn't change" result == "password" } + + def "test authentication when ldap is disabled"(){ + LdapManager ldapManager = Mock(LdapManager) + UserAccountDao userAccountDao = Mock(UserAccountDao) + def ldapAuthenticator = new LdapAuthenticator(ldapManager, userAccountDao) + ldapManager.isLdapEnabled() >> false + + when: + Pair result = ldapAuthenticator.authenticate("rajanik", "password", 1, null) + then: + result.first() == false + result.second() == null + + } + + // tests when domain is linked to LDAP + def "test authentication when domain is linked and user disabled in ldap"(){ + LdapManager ldapManager = Mock(LdapManager) + UserAccountDao userAccountDao = Mock(UserAccountDao) + AccountManager accountManager = Mock(AccountManager) + + def ldapAuthenticator = new LdapAuthenticator() + ldapAuthenticator._ldapManager = ldapManager + ldapAuthenticator._userAccountDao = userAccountDao + ldapAuthenticator._accountManager = accountManager + + long domainId = 1; + String username = "rajanik" + LdapManager.LinkType type = LdapManager.LinkType.GROUP + String name = "CN=test,DC=ccp,DC=citrix,DC=com" + + ldapManager.isLdapEnabled() >> true + UserAccount userAccount = Mock(UserAccount) + userAccountDao.getUserAccount(username, domainId) >> userAccount + userAccount.getId() >> 1 + ldapManager.getDomainLinkedToLdap(domainId) >> new LdapTrustMapVO(domainId, type, name, (short)2) + ldapManager.getUser(username, type.toString(), name) >> new LdapUser(username, "email", "firstname", "lastname", "principal", "domain", true) + //user should be disabled in cloudstack + accountManager.disableUser(1) >> userAccount + + when: + Pair result = ldapAuthenticator.authenticate(username, "password", domainId, null) + then: + result.first() == false + result.second() == UserAuthenticator.ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT + } + + def "test authentication when domain is linked and first time user can authenticate in ldap"(){ + LdapManager ldapManager = Mock(LdapManager) + UserAccountDao userAccountDao = Mock(UserAccountDao) + AccountManager accountManager = Mock(AccountManager) + + def ldapAuthenticator = new LdapAuthenticator() + ldapAuthenticator._ldapManager = ldapManager + ldapAuthenticator._userAccountDao = userAccountDao + ldapAuthenticator._accountManager = accountManager + + long domainId = 1; + String username = "rajanik" + LdapManager.LinkType type = LdapManager.LinkType.GROUP + String name = "CN=test,DC=ccp,DC=citrix,DC=com" + + ldapManager.isLdapEnabled() >> true + userAccountDao.getUserAccount(username, domainId) >> null + ldapManager.getDomainLinkedToLdap(domainId) >> new LdapTrustMapVO(domainId, type, name, (short)0) + ldapManager.getUser(username, type.toString(), name) >> new LdapUser(username, "email", "firstname", "lastname", "principal", "domain", false) + ldapManager.canAuthenticate(_,_) >> true + //user should be created in cloudstack + accountManager.createUserAccount(username, "", "firstname", "lastname", "email", null, username, (short) 2, domainId, username, null, _, _, User.Source.LDAP) >> Mock(UserAccount) + + when: + Pair result = ldapAuthenticator.authenticate(username, "password", domainId, null) + then: + result.first() == true + result.second() == null + } + + def "test authentication when domain is linked and existing user can authenticate in ldap"(){ + LdapManager ldapManager = Mock(LdapManager) + UserAccountDao userAccountDao = Mock(UserAccountDao) + AccountManager accountManager = Mock(AccountManager) + + def ldapAuthenticator = new LdapAuthenticator() + ldapAuthenticator._ldapManager = ldapManager + ldapAuthenticator._userAccountDao = userAccountDao + ldapAuthenticator._accountManager = accountManager + + long domainId = 1; + String username = "rajanik" + LdapManager.LinkType type = LdapManager.LinkType.GROUP + String name = "CN=test,DC=ccp,DC=citrix,DC=com" + + ldapManager.isLdapEnabled() >> true + UserAccount userAccount = Mock(UserAccount) + userAccountDao.getUserAccount(username, domainId) >> userAccount + userAccount.getId() >> 1 + userAccount.getState() >> Account.State.disabled.toString() + ldapManager.getDomainLinkedToLdap(domainId) >> new LdapTrustMapVO(domainId, type, name, (short)2) + ldapManager.getUser(username, type.toString(), name) >> new LdapUser(username, "email", "firstname", "lastname", "principal", "domain", false) + ldapManager.canAuthenticate(_,_) >> true + //user should be enabled in cloudstack if disabled + accountManager.enableUser(1) >> userAccount + + when: + Pair result = ldapAuthenticator.authenticate(username, "password", domainId, null) + then: + result.first() == true + result.second() == null + } + + def "test authentication when domain is linked and user cannot authenticate in ldap"(){ + LdapManager ldapManager = Mock(LdapManager) + UserAccountDao userAccountDao = Mock(UserAccountDao) + AccountManager accountManager = Mock(AccountManager) + + def ldapAuthenticator = new LdapAuthenticator() + ldapAuthenticator._ldapManager = ldapManager + ldapAuthenticator._userAccountDao = userAccountDao + ldapAuthenticator._accountManager = accountManager + + long domainId = 1; + String username = "rajanik" + LdapManager.LinkType type = LdapManager.LinkType.GROUP + String name = "CN=test,DC=ccp,DC=citrix,DC=com" + + ldapManager.isLdapEnabled() >> true + UserAccount userAccount = Mock(UserAccount) + userAccountDao.getUserAccount(username, domainId) >> userAccount + ldapManager.getDomainLinkedToLdap(domainId) >> new LdapTrustMapVO(domainId, type, name, (short)2) + ldapManager.getUser(username, type.toString(), name) >> new LdapUser(username, "email", "firstname", "lastname", "principal", "domain", false) + ldapManager.canAuthenticate(_,_) >> false + + when: + Pair result = ldapAuthenticator.authenticate(username, "password", domainId, null) + then: + result.first() == false + result.second() == UserAuthenticator.ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT + } } From ca8b37535a6ade064968013ebbee9c1915cba587 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Tue, 1 Sep 2015 10:44:30 +0530 Subject: [PATCH 13/16] CLOUDSTACK-8647: updated with review comments made domainId compulsory in api LinkDomainToLdapCmd used accountServive from BaseCmd in LinkDomainToLdapCmd changed the allowed account type values to 0 and 2 --- .../cloudstack/api/command/LinkDomainToLdapCmd.java | 11 ++++------- .../org/apache/cloudstack/ldap/LdapManagerImpl.java | 4 ++-- .../apache/cloudstack/ldap/LdapManagerImplSpec.groovy | 2 +- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java index f5a0ef82a2f..90fcaad596d 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java @@ -21,7 +21,6 @@ package org.apache.cloudstack.api.command; import javax.inject.Inject; import com.cloud.exception.InvalidParameterValueException; -import com.cloud.user.AccountService; import com.cloud.user.User; import com.cloud.user.UserAccount; import org.apache.cloudstack.api.APICommand; @@ -47,7 +46,8 @@ public class LinkDomainToLdapCmd extends BaseCmd { public static final Logger s_logger = Logger.getLogger(LinkDomainToLdapCmd.class.getName()); private static final String s_name = "linkdomaintoldapresponse"; - @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "The id of the domain which has to be linked to LDAP.") + @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = true, entityType = DomainResponse.class, description = "The id of the domain which has to be " + + "linked to LDAP.") private Long domainId; @Parameter(name = ApiConstants.TYPE, type = CommandType.STRING, required = true, description = "type of the ldap name. GROUP or OU") @@ -59,16 +59,13 @@ public class LinkDomainToLdapCmd extends BaseCmd { @Parameter(name = ApiConstants.ADMIN, type = CommandType.STRING, required = false, description = "domain admin username in LDAP ") private String admin; - @Parameter(name = ApiConstants.ACCOUNT_TYPE, type = CommandType.SHORT, required = true, description = "Type of the account to auto import. Specify 0 for user, 1 for root " + - "admin, and 2 for domain admin") + @Parameter(name = ApiConstants.ACCOUNT_TYPE, type = CommandType.SHORT, required = true, description = "Type of the account to auto import. Specify 0 for user and 2 for " + + "domain admin") private short accountType; @Inject private LdapManager _ldapManager; - @Inject - public AccountService _accountService; - @Override public void execute() throws ServerApiException { try { 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 aab6f8a5581..563c0535541 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 @@ -269,8 +269,8 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { 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"); + //Account type should be 0 or 2. check the constants in com.cloud.user.Account + Validate.isTrue(accountType==0 || accountType==2, "accountype should be either 0(normal user) or 2(domain admin)"); 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()); 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 6e38926d25c..c9af0020848 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 @@ -484,7 +484,7 @@ class LdapManagerImplSpec extends spock.lang.Specification { then: thrown(IllegalArgumentException) where: - accountType << [-1, 6, 20000, -500000] + accountType << [-1, 1, 3, 4, 5, 6, 20000, -500000] } def "test linkDomainToLdap when all is well"(){ def ldapManager = new LdapManagerImpl() From 26fea34d169403315cbd4d80d37b9fa948cc2868 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Tue, 1 Sep 2015 14:29:40 +0530 Subject: [PATCH 14/16] CLOUDSTACK-8647: string formatting --- .../ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 563c0535541..3fd928225fc 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 @@ -202,7 +202,7 @@ public class LdapManagerImpl implements LdapManager, LdapValidator { return _ldapUserManagerFactory.getInstance(_ldapConfiguration.getLdapProvider()).getUser(escapedUsername, type, name, context); } catch (NamingException | IOException e) { s_logger.debug("ldap Exception: ",e); - throw new NoLdapUserMatchingQueryException("No Ldap User found for username: "+username + "name: " + name + "of type" + type); + throw new NoLdapUserMatchingQueryException("No Ldap User found for username: "+username + "name: " + name + "of type: " + type); } finally { closeContext(context); } From 6177bae810a3f6c3677cb471e1dc12c5d55631d3 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Tue, 1 Sep 2015 14:49:42 +0530 Subject: [PATCH 15/16] CLOUDSTACK-8647 removed duplicate key in create sql of ldap_trust_map --- setup/db/db/schema-452to460.sql | 1 - 1 file changed, 1 deletion(-) diff --git a/setup/db/db/schema-452to460.sql b/setup/db/db/schema-452to460.sql index 3ca066015f4..1f236081a7e 100644 --- a/setup/db/db/schema-452to460.sql +++ b/setup/db/db/schema-452to460.sql @@ -407,6 +407,5 @@ CREATE TABLE `cloud`.`ldap_trust_map` ( `account_type` int(1) unsigned NOT NULL, PRIMARY KEY (`id`), UNIQUE KEY `uk_ldap_trust_map__domain_id` (`domain_id`), - KEY `fk_ldap_trust_map__domain_id` (`domain_id`), CONSTRAINT `fk_ldap_trust_map__domain_id` FOREIGN KEY (`domain_id`) REFERENCES `domain` (`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; From 53a441faf6d5c74f666a130f4b438977684c3800 Mon Sep 17 00:00:00 2001 From: Rajani Karuturi Date: Thu, 3 Sep 2015 18:05:50 +0530 Subject: [PATCH 16/16] CLOUDSTACK-8647: linkdomaintoldap shouldnt fail when createuseraccount fails Incase create useraccount fails with any runtime exception, linkdomaintoldap api shouldnt fail. It just will not return the admin id as it didnt create the account. added test cases to verify this as well. --- .../api/command/LinkDomainToLdapCmd.java | 28 ++++---- .../ldap/LinkDomainToLdapCmdSpec.groovy | 67 +++++++++++++++++++ 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java index 90fcaad596d..0ffa8408ce3 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LinkDomainToLdapCmd.java @@ -71,24 +71,28 @@ public class LinkDomainToLdapCmd extends BaseCmd { try { LinkDomainToLdapResponse response = _ldapManager.linkDomainToLdap(domainId, type, name, accountType); if(admin!=null) { + LdapUser ldapUser = null; try { - LdapUser ldapUser = _ldapManager.getUser(admin, type, name); - if(!ldapUser.isDisabled()) { - Account account = _accountService.getActiveAccountByName(admin, domainId); - if (account == null) { - UserAccount userAccount = - _accountService.createUserAccount(admin, "", ldapUser.getFirstname(), ldapUser.getLastname(), ldapUser.getEmail(), null, admin, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, domainId, admin, null, UUID.randomUUID().toString(), - UUID.randomUUID().toString(), User.Source.LDAP); + ldapUser = _ldapManager.getUser(admin, type, name); + } catch (NoLdapUserMatchingQueryException e) { + s_logger.debug("no ldap user matching username " + admin + " in the given group/ou", e); + } + if (ldapUser != null && !ldapUser.isDisabled()) { + Account account = _accountService.getActiveAccountByName(admin, domainId); + if (account == null) { + try { + UserAccount userAccount = _accountService.createUserAccount(admin, "", ldapUser.getFirstname(), ldapUser.getLastname(), ldapUser.getEmail(), null, + admin, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, domainId, admin, null, UUID.randomUUID().toString(), UUID.randomUUID().toString(), User.Source.LDAP); response.setAdminId(String.valueOf(userAccount.getAccountId())); s_logger.info("created an account with name " + admin + " in the given domain " + domainId); - } else { - s_logger.debug("an account with name " + admin + " already exists in the domain " + domainId); + } catch (Exception e) { + s_logger.info("an exception occurred while creating account with name " + admin +" in domain " + domainId, e); } } else { - s_logger.debug("ldap user with username "+admin+" is disabled in the given group/ou"); + s_logger.debug("an account with name " + admin + " already exists in the domain " + domainId); } - } catch (NoLdapUserMatchingQueryException e) { - s_logger.debug("no ldap user matching username " + admin + " in the given group/ou"); + } else { + s_logger.debug("ldap user with username "+admin+" is disabled in the given group/ou"); } } response.setObjectName("LinkDomainToLdap"); diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy index dbf92fbba89..9d667bf4cfb 100644 --- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy +++ b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy @@ -28,6 +28,7 @@ import org.apache.cloudstack.api.command.LinkDomainToLdapCmd import org.apache.cloudstack.api.response.LinkDomainToLdapResponse import org.apache.cloudstack.ldap.LdapManager import org.apache.cloudstack.ldap.LdapUser +import org.apache.cloudstack.ldap.NoLdapUserMatchingQueryException import spock.lang.Shared import spock.lang.Specification @@ -162,4 +163,70 @@ class LinkDomainToLdapCmdSpec extends Specification { result.getAdminId() == String.valueOf(accountId) } + def "test when admin doesnt exist in ldap"() { + def domainId = 1; + def type = "GROUP"; + def name = "CN=test,DC=ccp,DC=Citrix,DC=com" + def accountType = 2; + def username = "admin" + + LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType) + _ldapManager.linkDomainToLdap(_,_,_,_) >> response + _ldapManager.getUser(username, type, name) >> {throw new NoLdapUserMatchingQueryException("get ldap user failed from mock")} + + linkDomainToLdapCmd.admin = username + linkDomainToLdapCmd.type = type + linkDomainToLdapCmd.name = name + linkDomainToLdapCmd.domainId = domainId + + when: + linkDomainToLdapCmd.execute() + then: + LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject() + result.getObjectName() == "LinkDomainToLdap" + result.getResponseName() == linkDomainToLdapCmd.getCommandName() + result.getDomainId() == domainId + result.getType() == type + result.getName() == name + result.getAdminId() == null + } + + /** + * api should not fail in this case as link domain to ldap is successful + */ + def "test when create user account throws a run time exception"() { + def domainId = 1; + def type = "GROUP"; + def name = "CN=test,DC=ccp,DC=Citrix,DC=com" + def accountType = 2; + def username = "admin" + def accountId = 24 + + LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType) + _ldapManager.linkDomainToLdap(_,_,_,_) >> response + _ldapManager.getUser(username, type, name) >> new LdapUser(username, "admin@ccp.citrix.com", "Admin", "Admin", name, "ccp", false) + + _accountService.getActiveAccountByName(username, domainId) >> null + UserAccount userAccount = Mock(UserAccount) + userAccount.getAccountId() >> 24 + _accountService.createUserAccount(username, "", "Admin", "Admin", "admin@ccp.citrix.com", null, username, Account.ACCOUNT_TYPE_DOMAIN_ADMIN, domainId, + username, null, _, _, User.Source.LDAP) >> { throw new RuntimeException("created failed from mock") } + + linkDomainToLdapCmd.admin = username + linkDomainToLdapCmd.type = type + linkDomainToLdapCmd.name = name + linkDomainToLdapCmd.domainId = domainId + + when: + linkDomainToLdapCmd.execute() + then: + LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject() + result.getObjectName() == "LinkDomainToLdap" + result.getResponseName() == linkDomainToLdapCmd.getCommandName() + result.getDomainId() == domainId + result.getType() == type + result.getName() == name + result.getAdminId() == null + } + }