Repository: cloudstack
Updated Branches:
refs/heads/master 5efded3ae -> 736ff5f8e
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/736ff5f8
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/736ff5f8
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/736ff5f8
Branch: refs/heads/master
Commit: 736ff5f8e5b88061c488f7dae35df3d555eaa5df
Parents: 5efded3
Author: Rajani Karuturi <[email protected]>
Authored: Mon Aug 11 17:52:16 2014 +0530
Committer: Rajani Karuturi <[email protected]>
Committed: Mon Aug 11 17:54:31 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/736ff5f8/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/736ff5f8/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/736ff5f8/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..1533494 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/736ff5f8/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',
'[email protected]', null, 'ACCOUNT', 0, _) >> Mock(UserVO)
0 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy',
'[email protected]', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _)
+ 0 * accountService.updateUser(_,'Ryan', 'Murphy', '[email protected]',
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", "[email protected]", "Ryan",
"Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering"))
+ ldapManager.getUsers() >> users
+ LdapUserResponse response1 = new LdapUserResponse("rmurphy",
"[email protected]", "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',
'[email protected]', null, 'ACCOUNT', 0, _) >> Mock(UserVO)
+ 0 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy',
'[email protected]', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _)
+ 1 * accountService.updateUser(_,'Ryan', 'Murphy', '[email protected]',
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',
'[email protected]', null, 'ACCOUNT', 0, _)
1 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy',
'[email protected]', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _)
+ 0 * accountService.updateUser(_,'Ryan', 'Murphy', '[email protected]',
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/736ff5f8/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/736ff5f8/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;