Repository: cloudstack Updated Branches: refs/heads/hotfix/CLOUDSTACK-7303 [created] adadfe11f
Fixed CLOUDSTACK-7303 [LDAP] while importing ldap users, update the user info if it already exists in cloudstack Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/adadfe11 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/adadfe11 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/adadfe11 Branch: refs/heads/hotfix/CLOUDSTACK-7303 Commit: adadfe11f09b6da5fa5cc5f1eed792d48e879ddb Parents: 3547562 Author: Rajani Karuturi <rajanikarut...@gmail.com> Authored: Mon Aug 11 16:34:30 2014 +0530 Committer: Rajani Karuturi <rajanikarut...@gmail.com> Committed: Mon Aug 11 16:34:30 2014 +0530 ---------------------------------------------------------------------- api/src/com/cloud/user/AccountService.java | 4 ++ .../contrail/management/MockAccountManager.java | 13 ++++++ .../api/command/LdapImportUsersCmd.java | 14 +++++- .../ldap/LdapImportUsersCmdSpec.groovy | 33 ++++++++++++++ .../src/com/cloud/user/AccountManagerImpl.java | 48 ++++++++++++-------- .../com/cloud/user/MockAccountManagerImpl.java | 13 ++++++ 6 files changed, 104 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/adadfe11/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 eac8a76..2785264 100755 --- a/api/src/com/cloud/user/AccountService.java +++ b/api/src/com/cloud/user/AccountService.java @@ -79,6 +79,10 @@ public interface AccountService { Account getActiveAccountByName(String accountName, Long domainId); + UserAccount getActiveUserAccount(String username, Long domainId); + + UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, String timeZone); + Account getActiveAccountById(long accountId); Account getAccount(long accountId); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/adadfe11/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 4136b5c..2f97a0f 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 @@ -137,6 +137,19 @@ public class MockAccountManager extends ManagerBase implements AccountManager { } @Override + public UserAccount getActiveUserAccount(String username, Long domainId) { + // TODO Auto-generated method stub + return null; + } + + @Override + public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, + String timeZone) { + // TODO Auto-generated method stub + return null; + } + + @Override public User getActiveUser(long arg0) { return _systemUser; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/adadfe11/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 6f7be90..0095336 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 @@ -26,6 +26,7 @@ import java.util.UUID; import javax.inject.Inject; import com.cloud.user.Account; +import com.cloud.user.UserAccount; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -108,11 +109,20 @@ public class LdapImportUsersCmd extends BaseListCmd { private void createCloudstackUserAccount(LdapUser user, String accountName, Domain domain) { Account account = _accountService.getActiveAccountByName(accountName, domain.getId()); if (account == null) { + s_logger.debug("No account exists with name: " + accountName + "creating the account and an user with name: " + user.getUsername() + "in the account"); _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()); +// check if the user exists. if yes, call update + UserAccount csuser = _accountService.getActiveUserAccount(user.getUsername(), domain.getId()); + if(csuser == null) { + s_logger.debug("No user exists with name: " + user.getUsername() + "creating a user in the account: " + accountName); + _accountService.createUser(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domain.getId(), + UUID.randomUUID().toString()); + } else { + s_logger.debug("account with name: " + accountName + "exist and user with name: " + user.getUsername() + "exists in the account. Updating the account."); + _accountService.updateUser(csuser.getId(), user.getFirstname(), user.getLastname(), user.getEmail(), null, null, null, null, null); + } } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/adadfe11/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 79eba93..9266202 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 @@ -213,8 +213,10 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { 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, _, _) + 0 * accountService.updateUser(_,'Ryan', 'Murphy', 'rmur...@test.com', null, null, null, null, null); def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService) ldapImportUsersCmd.accountName = "ACCOUNT" @@ -226,6 +228,36 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { then: "expect 1 call on accountService createUser and 0 on account service create user account" } + + def "Test create ldap import account for an already existing cloudstack user"() { + 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.getActiveUserAccount('rmurphy',0) >> Mock(UserAccountVO) + 0 * 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, _, _) + 1 * accountService.updateUser(_,'Ryan', 'Murphy', 'rmur...@test.com', null, null, null, null, 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 updateUser and 0 on account service create user and 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) @@ -242,6 +274,7 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification { 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, _, _) + 0 * accountService.updateUser(_,'Ryan', 'Murphy', 'rmur...@test.com', null, null, null, null, null); def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService) ldapImportUsersCmd.accountName = "ACCOUNT" http://git-wip-us.apache.org/repos/asf/cloudstack/blob/adadfe11/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 11876fa..98efc79 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -1105,19 +1105,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override @ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User") - public UserAccount updateUser(UpdateUserCmd cmd) { - Long id = cmd.getId(); - String apiKey = cmd.getApiKey(); - String firstName = cmd.getFirstname(); - String email = cmd.getEmail(); - String lastName = cmd.getLastname(); - String password = cmd.getPassword(); - String secretKey = cmd.getSecretKey(); - String timeZone = cmd.getTimezone(); - String userName = cmd.getUsername(); - + public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, String timeZone) { // Input validation - UserVO user = _userDao.getUser(id); + UserVO user = _userDao.getUser(userId); if (user == null) { throw new InvalidParameterValueException("unable to find user by id"); @@ -1140,7 +1130,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M // don't allow updating system account if (account.getId() == Account.ACCOUNT_ID_SYSTEM) { - throw new PermissionDeniedException("user id : " + id + " is system account, update is not allowed"); + throw new PermissionDeniedException("user id : " + userId + " is system account, update is not allowed"); } checkAccess(CallContext.current().getCallingAccount(), AccessType.OperateEntry, true, account); @@ -1206,7 +1196,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } if (s_logger.isDebugEnabled()) { - s_logger.debug("updating user with id: " + id); + s_logger.debug("updating user with id: " + userId); } try { // check if the apiKey and secretKey are globally unique @@ -1215,23 +1205,38 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (apiKeyOwner != null) { User usr = apiKeyOwner.first(); - if (usr.getId() != id) { - throw new InvalidParameterValueException("The api key:" + apiKey + " exists in the system for user id:" + id + " ,please provide a unique key"); + if (usr.getId() != userId) { + throw new InvalidParameterValueException("The api key:" + apiKey + " exists in the system for user id:" + userId + " ,please provide a unique key"); } else { // allow the updation to take place } } } - _userDao.update(id, user); + _userDao.update(userId, user); } catch (Throwable th) { s_logger.error("error updating user", th); - throw new CloudRuntimeException("Unable to update user " + id); + throw new CloudRuntimeException("Unable to update user " + userId); } CallContext.current().putContextParameter(User.class, user.getUuid()); - return _userAccountDao.findById(id); + return _userAccountDao.findById(userId); + } + + @Override + public UserAccount updateUser(UpdateUserCmd cmd) { + Long id = cmd.getId(); + String apiKey = cmd.getApiKey(); + String firstName = cmd.getFirstname(); + String email = cmd.getEmail(); + String lastName = cmd.getLastname(); + String password = cmd.getPassword(); + String secretKey = cmd.getSecretKey(); + String timeZone = cmd.getTimezone(); + String userName = cmd.getUsername(); + + return updateUser(id, firstName, lastName, email, userName, password, apiKey, secretKey, timeZone); } @Override @@ -1806,6 +1811,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } @Override + public UserAccount getActiveUserAccount(String username, Long domainId) { + return _userAccountDao.getUserAccount(username, domainId); + } + + @Override public Account getActiveAccountById(long accountId) { return _accountDao.findById(accountId); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/adadfe11/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 746fa1b..8620031 100644 --- a/server/test/com/cloud/user/MockAccountManagerImpl.java +++ b/server/test/com/cloud/user/MockAccountManagerImpl.java @@ -138,6 +138,19 @@ public class MockAccountManagerImpl extends ManagerBase implements Manager, Acco } @Override + public UserAccount getActiveUserAccount(String username, Long domainId) { + // TODO Auto-generated method stub + return null; + } + + @Override + public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, + String timeZone) { + // TODO Auto-generated method stub + return null; + } + + @Override public Account getActiveAccountById(long accountId) { // TODO Auto-generated method stub return null;
