Return-Path: X-Original-To: apmail-cloudstack-commits-archive@www.apache.org Delivered-To: apmail-cloudstack-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 0EE7E107B5 for ; Tue, 29 Apr 2014 11:37:42 +0000 (UTC) Received: (qmail 962 invoked by uid 500); 29 Apr 2014 11:37:41 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 744 invoked by uid 500); 29 Apr 2014 11:37:40 -0000 Mailing-List: contact commits-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list commits@cloudstack.apache.org Received: (qmail 737 invoked by uid 99); 29 Apr 2014 11:37:39 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 29 Apr 2014 11:37:39 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 31CA8995150; Tue, 29 Apr 2014 11:37:39 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: dahn@apache.org To: commits@cloudstack.apache.org Message-Id: <6b22c4098d674876b85f8d1b4de9c8b0@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: updated refs/heads/4.4 to 69e550f Date: Tue, 29 Apr 2014 11:37:39 +0000 (UTC) Repository: cloudstack Updated Branches: refs/heads/4.4 8eb903ba4 -> 69e550f5e Fixed CLOUDSTACK-6509 Cannot import multiple LDAP/AD users into a cloudstack account Conflicts: api/src/com/cloud/user/AccountService.java plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java Signed-off-by: Koushik Das Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/69e550f5 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/69e550f5 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/69e550f5 Branch: refs/heads/4.4 Commit: 69e550f5ea0ea1b85adcce6366d97cffa0276e73 Parents: 8eb903b Author: Rajani Karuturi Authored: Fri Apr 25 16:42:39 2014 +0530 Committer: Daan Hoogland Committed: Tue Apr 29 13:37:28 2014 +0200 ---------------------------------------------------------------------- api/src/com/cloud/user/AccountService.java | 8 +++ .../contrail/management/MockAccountManager.java | 6 ++ .../api/command/LdapCreateAccountCmd.java | 63 +++++++++++++------- .../api/command/LdapImportUsersCmd.java | 26 +++++++- .../ldap/LdapImportUsersCmdSpec.groovy | 60 +++++++++++++++++++ .../src/com/cloud/user/AccountManagerImpl.java | 5 ++ .../com/cloud/user/MockAccountManagerImpl.java | 6 ++ ui/scripts/accountsWizard.js | 6 +- 8 files changed, 152 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/69e550f5/api/src/com/cloud/user/AccountService.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/user/AccountService.java b/api/src/com/cloud/user/AccountService.java index 71136bf..10be650 100755 --- a/api/src/com/cloud/user/AccountService.java +++ b/api/src/com/cloud/user/AccountService.java @@ -114,4 +114,12 @@ public interface AccountService { void checkAccess(Account caller, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException; Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly); + + /** + * returns the user account object for a given user id + * @param userId user id + * @return useraccount object if it exists else null + */ + UserAccount getUserAccountById(Long userId); + } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/69e550f5/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java ---------------------------------------------------------------------- diff --git a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index e9bbc8e..4763cb7 100644 --- a/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -96,6 +96,12 @@ public class MockAccountManager extends ManagerBase implements AccountManager { } @Override + public UserAccount getUserAccountById(Long userId) { + // TODO Auto-generated method stub + return null; + } + + @Override public String[] createApiKeyAndSecretKey(RegisterCmd arg0) { // TODO Auto-generated method stub return null; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/69e550f5/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java index 626bb8f..b753952 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapCreateAccountCmd.java @@ -23,9 +23,6 @@ import java.util.Map; import javax.inject.Inject; import javax.naming.NamingException; -import org.apache.log4j.Logger; -import org.bouncycastle.util.encoders.Base64; - import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -38,13 +35,16 @@ import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.ldap.LdapManager; import org.apache.cloudstack.ldap.LdapUser; +import org.apache.log4j.Logger; +import org.bouncycastle.util.encoders.Base64; +import com.cloud.domain.DomainVO; import com.cloud.user.Account; import com.cloud.user.AccountService; +import com.cloud.user.User; import com.cloud.user.UserAccount; -@APICommand(name = "ldapCreateAccount", description = "Creates an account from an LDAP user", responseObject = AccountResponse.class, since = "4.2.0", - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +@APICommand(name = "ldapCreateAccount", description = "Creates an account from an LDAP user", responseObject = AccountResponse.class, since = "4.2.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class LdapCreateAccountCmd extends BaseCmd { public static final Logger s_logger = Logger.getLogger(LdapCreateAccountCmd.class.getName()); private static final String s_name = "createaccountresponse"; @@ -52,23 +52,16 @@ public class LdapCreateAccountCmd extends BaseCmd { @Inject private LdapManager _ldapManager; - @Parameter(name = ApiConstants.ACCOUNT, - type = CommandType.STRING, - description = "Creates the user under the specified account. If no account is specified, the username will be used as the account name.") + @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, description = "Creates the user under the specified account. If no account is specified, the username will be used as the account name.") private String accountName; - @Parameter(name = ApiConstants.ACCOUNT_TYPE, - type = CommandType.SHORT, - required = true, - description = "Type of the account. 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. Specify 0 for user, 1 for root admin, and 2 for domain admin") private Short accountType; @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "Creates the user under the specified domain.") private Long domainId; - @Parameter(name = ApiConstants.TIMEZONE, - type = CommandType.STRING, - description = "Specifies a timezone for this command. For more information on the timezone parameter, see Time Zone Format.") + @Parameter(name = ApiConstants.TIMEZONE, type = CommandType.STRING, description = "Specifies a timezone for this command. For more information on the timezone parameter, see Time Zone Format.") private String timezone; @Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING, required = true, description = "Unique username.") @@ -96,22 +89,46 @@ public class LdapCreateAccountCmd extends BaseCmd { _accountService = accountService; } - UserAccount createCloudstackUserAccount(final LdapUser user) { - return _accountService.createUserAccount(username, generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, - accountType, domainId, networkDomain, details, accountUUID, userUUID); + UserAccount createCloudstackUserAccount(final LdapUser user, String accountName, Long domainId) { + Account account = _accountService.getActiveAccountByName(accountName, domainId); + if (account == null) { + return _accountService.createUserAccount(username, generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, accountType, + domainId, networkDomain, details, accountUUID, userUUID); + } else { + User newUser = _accountService.createUser(username, generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domainId, + userUUID); + return _accountService.getUserAccountById(newUser.getId()); + } + } + + private String getAccountName() { + String name = accountName; + if (accountName == null) { + name = username; + } + return name; + } + + private Long getDomainId() { + Long id = domainId; + if (id == null) { + id = DomainVO.ROOT_DOMAIN; + } + return id; } @Override public void execute() throws ServerApiException { final CallContext callContext = getCurrentContext(); - callContext.setEventDetails("Account Name: " + accountName + ", Domain Id:" + domainId); + String finalAccountName = getAccountName(); + Long finalDomainId = getDomainId(); + callContext.setEventDetails("Account Name: " + finalAccountName + ", Domain Id:" + finalDomainId); try { final LdapUser user = _ldapManager.getUser(username); validateUser(user); - final UserAccount userAccount = createCloudstackUserAccount(user); + final UserAccount userAccount = createCloudstackUserAccount(user, finalAccountName, finalDomainId); if (userAccount != null) { - final AccountResponse response = _responseGenerator - .createUserAccountResponse(ResponseView.Full, userAccount); + final AccountResponse response = _responseGenerator.createUserAccountResponse(ResponseView.Full, userAccount); response.setResponseName(getCommandName()); setResponseObject(response); } else { @@ -159,4 +176,4 @@ public class LdapCreateAccountCmd extends BaseCmd { } return true; } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/69e550f5/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java index 887ad00..6f7be90 100644 --- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java +++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java @@ -25,6 +25,7 @@ import java.util.UUID; import javax.inject.Inject; +import com.cloud.user.Account; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -87,6 +88,9 @@ public class LdapImportUsersCmd extends BaseListCmd { private Domain _domain; + @Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, description = "Creates the user under the specified account. If no account is specified, the username will be used as the account name.") + private String accountName; + @Inject private LdapManager _ldapManager; @@ -101,6 +105,17 @@ public class LdapImportUsersCmd extends BaseListCmd { _accountService = accountService; } + private void createCloudstackUserAccount(LdapUser user, String accountName, Domain domain) { + Account account = _accountService.getActiveAccountByName(accountName, domain.getId()); + if (account == null) { + _accountService.createUserAccount(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, accountType, + domain.getId(), domain.getNetworkDomain(), details, UUID.randomUUID().toString(), UUID.randomUUID().toString()); + } else { + _accountService.createUser(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domain.getId(), + UUID.randomUUID().toString()); + } + } + @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { @@ -122,8 +137,7 @@ public class LdapImportUsersCmd extends BaseListCmd { for (LdapUser user : users) { Domain domain = getDomain(user); try { - _accountService.createUserAccount(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, - user.getUsername(), accountType, domain.getId(), domain.getNetworkDomain(), details, UUID.randomUUID().toString(), UUID.randomUUID().toString()); + createCloudstackUserAccount(user, getAccountName(user), domain); addedUsers.add(user); } catch (InvalidParameterValueException ex) { s_logger.error("Failed to create user with username: " + user.getUsername() + " ::: " + ex.getMessage()); @@ -135,6 +149,14 @@ public class LdapImportUsersCmd extends BaseListCmd { setResponseObject(response); } + private String getAccountName(LdapUser user) { + String finalAccountName = accountName; + if(finalAccountName == null ) { + finalAccountName = user.getUsername(); + } + return finalAccountName; + } + private Domain getDomainForName(String name) { Domain domain = null; if (StringUtils.isNotBlank(name)) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/69e550f5/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapImportUsersCmdSpec.groovy ---------------------------------------------------------------------- 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 a66da1f..79eba93 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 @@ -19,9 +19,14 @@ package groovy.org.apache.cloudstack.ldap import com.cloud.domain.Domain import com.cloud.domain.DomainVO import com.cloud.user.AccountService +import com.cloud.user.AccountVO import com.cloud.user.DomainService +import com.cloud.user.UserAccountVO +import com.cloud.user.UserVO +import org.apache.cloudstack.api.command.LdapCreateAccountCmd import org.apache.cloudstack.api.command.LdapImportUsersCmd import org.apache.cloudstack.api.response.LdapUserResponse +import org.apache.cloudstack.context.CallContext import org.apache.cloudstack.ldap.LdapManager import org.apache.cloudstack.ldap.LdapUser @@ -193,4 +198,59 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { } + + def "Test create ldap import account for an already existing cloudstack account"() { + 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")) + 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 + + def domainService = Mock(DomainService) + 1 * domainService.getDomain(1L) >> new DomainVO("DOMAIN", 1L, 1L, "DOMAIN", UUID.randomUUID().toString());; + + def accountService = Mock(AccountService) + 1 * accountService.getActiveAccountByName('ACCOUNT', 0) >> Mock(AccountVO) + 1 * accountService.createUser('rmurphy', _ , 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 0, _) >> Mock(UserVO) + 0 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _) + + def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService) + ldapImportUsersCmd.accountName = "ACCOUNT" + ldapImportUsersCmd.accountType = 2; + ldapImportUsersCmd.domainId = 1L; + + when: "create account is called" + ldapImportUsersCmd.execute() + then: "expect 1 call on accountService createUser and 0 on account service create user account" + } + + def "Test create ldap import account for a new cloudstack account"() { + 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")) + 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 + + def domainService = Mock(DomainService) + 1 * domainService.getDomain(1L) >> new DomainVO("DOMAIN", 1L, 1L, "DOMAIN", UUID.randomUUID().toString());; + + def accountService = Mock(AccountService) + 1 * accountService.getActiveAccountByName('ACCOUNT', 0) >> null + 0 * accountService.createUser('rmurphy', _ , 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 0, _) + 1 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _) + + def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService) + ldapImportUsersCmd.accountName = "ACCOUNT" + ldapImportUsersCmd.accountType = 2; + ldapImportUsersCmd.domainId = 1L; + + when: "create account is called" + ldapImportUsersCmd.execute() + then: "expect 1 call on accountService createUser and 0 on account service create user account" + } + } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/69e550f5/server/src/com/cloud/user/AccountManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 227c611..d43cf97 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -2542,4 +2542,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } return null; } + + @Override + public UserAccount getUserAccountById(Long userId) { + return _userAccountDao.findById(userId); + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/69e550f5/server/test/com/cloud/user/MockAccountManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/user/MockAccountManagerImpl.java b/server/test/com/cloud/user/MockAccountManagerImpl.java index e53974a..f444c89 100644 --- a/server/test/com/cloud/user/MockAccountManagerImpl.java +++ b/server/test/com/cloud/user/MockAccountManagerImpl.java @@ -227,6 +227,12 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco } @Override + public UserAccount getUserAccountById(Long userId) { + // TODO Auto-generated method stub + return null; + } + + @Override public void logoutUser(long userId) { // TODO Auto-generated method stub } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/69e550f5/ui/scripts/accountsWizard.js ---------------------------------------------------------------------- diff --git a/ui/scripts/accountsWizard.js b/ui/scripts/accountsWizard.js index 6b4907c..d84fbfa 100644 --- a/ui/scripts/accountsWizard.js +++ b/ui/scripts/accountsWizard.js @@ -198,10 +198,10 @@ array1.push("&domainid=" + args.data.domainid); var account = args.data.account; - if (account === null || account.length === 0) { - account = args.username; + + if (account !== null && account.length > 0) { + array1.push("&account=" + account); } - array1.push("&account=" + account); var accountType = args.data.accounttype; if (args.data.accounttype == "1" && args.data.domainid != rootDomainId) { //if account type is admin, but domain is not Root domain