Repository: cloudstack Updated Branches: refs/heads/4.3 c37df38c8 -> 9303e7016
Fixed CLOUDSTACK-6509 Cannot import multiple LDAP/AD users into a cloudstack account Signed-off-by: Koushik Das <kous...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/9303e701 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/9303e701 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/9303e701 Branch: refs/heads/4.3 Commit: 9303e7016b8c2d20c8bf7d86dcde738983552d4e Parents: c37df38 Author: Rajani Karuturi <rajanikarut...@gmail.com> Authored: Fri Apr 25 16:42:39 2014 +0530 Committer: Koushik Das <kous...@apache.org> Committed: Tue Apr 29 12:11:08 2014 +0530 ---------------------------------------------------------------------- api/src/com/cloud/user/AccountService.java | 8 +++ .../contrail/management/MockAccountManager.java | 6 ++ .../api/command/LdapCreateAccountCmd.java | 44 +++++++++++--- .../api/command/LdapImportUsersCmd.java | 28 ++++++++- .../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, 149 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9303e701/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 8153a3f..33cb43d 100755 --- a/api/src/com/cloud/user/AccountService.java +++ b/api/src/com/cloud/user/AccountService.java @@ -102,4 +102,12 @@ public interface AccountService { void checkAccess(Account account, AccessType accessType, boolean sameOwner, ControlledEntity... entities) throws PermissionDeniedException; + + /** + * 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/9303e701/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 195e932..cb613ac 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 @@ -107,6 +107,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/9303e701/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 0878410..15d1ac3 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,6 +23,8 @@ import java.util.Map; import javax.inject.Inject; import javax.naming.NamingException; +import com.cloud.domain.DomainVO; +import com.cloud.user.User; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -88,22 +90,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(userAccount); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9303e701/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 5e724e8..99e9b87 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,9 @@ import java.util.UUID; import javax.inject.Inject; +import com.cloud.user.Account; +import com.cloud.user.User; +import com.cloud.user.UserAccount; import org.apache.cloudstack.api.*; import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.api.response.LdapUserResponse; @@ -68,6 +71,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; @@ -82,6 +88,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 { @@ -102,8 +119,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()); @@ -115,6 +131,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/9303e701/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<LdapUser> users = new ArrayList() + users.add(new LdapUser("rmurphy", "rmur...@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) + ldapManager.getUsers() >> users + LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmur...@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', 'rmur...@test.com', null, 'ACCOUNT', 0, _) >> Mock(UserVO) + 0 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmur...@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<LdapUser> users = new ArrayList() + users.add(new LdapUser("rmurphy", "rmur...@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")) + ldapManager.getUsers() >> users + LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmur...@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', 'rmur...@test.com', null, 'ACCOUNT', 0, _) + 1 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmur...@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/9303e701/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 d367653..00f6b1e 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -2321,4 +2321,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M public UserAccount getUserByApiKey(String apiKey) { return _userAccountDao.getUserByApiKey(apiKey); } + + @Override + public UserAccount getUserAccountById(Long userId) { + return _userAccountDao.findById(userId); + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/9303e701/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 38cc1a84..884302e 100644 --- a/server/test/com/cloud/user/MockAccountManagerImpl.java +++ b/server/test/com/cloud/user/MockAccountManagerImpl.java @@ -226,6 +226,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/9303e701/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
