Skip to content

Commit 736ff5f

Browse files
karuturikaruturi
committed
Fixed CLOUDSTACK-7303 [LDAP] while importing ldap users, update the user info if it already exists in cloudstack
1 parent 5efded3 commit 736ff5f

6 files changed

Lines changed: 104 additions & 21 deletions

File tree

‎api/src/com/cloud/user/AccountService.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ UserAccount createUserAccount(String userName, String password, String firstName
7979

8080
Account getActiveAccountByName(String accountName, Long domainId);
8181

82+
UserAccount getActiveUserAccount(String username, Long domainId);
83+
84+
UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, String timeZone);
85+
8286
Account getActiveAccountById(long accountId);
8387

8488
Account getAccount(long accountId);

‎plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/MockAccountManager.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,19 @@ public Account getActiveAccountByName(String arg0, Long arg1) {
136136
return null;
137137
}
138138

139+
@Override
140+
public UserAccount getActiveUserAccount(String username, Long domainId) {
141+
// TODO Auto-generated method stub
142+
return null;
143+
}
144+
145+
@Override
146+
public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey,
147+
String timeZone) {
148+
// TODO Auto-generated method stub
149+
return null;
150+
}
151+
139152
@Override
140153
public User getActiveUser(long arg0) {
141154
return _systemUser;

‎plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/command/LdapImportUsersCmd.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import javax.inject.Inject;
2727

2828
import com.cloud.user.Account;
29+
import com.cloud.user.UserAccount;
2930
import org.apache.cloudstack.api.APICommand;
3031
import org.apache.cloudstack.api.ApiConstants;
3132
import org.apache.cloudstack.api.ApiErrorCode;
@@ -108,11 +109,20 @@ public LdapImportUsersCmd(final LdapManager ldapManager, final DomainService dom
108109
private void createCloudstackUserAccount(LdapUser user, String accountName, Domain domain) {
109110
Account account = _accountService.getActiveAccountByName(accountName, domain.getId());
110111
if (account == null) {
112+
s_logger.debug("No account exists with name: " + accountName + " creating the account and an user with name: " + user.getUsername() + " in the account");
111113
_accountService.createUserAccount(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, accountType,
112114
domain.getId(), domain.getNetworkDomain(), details, UUID.randomUUID().toString(), UUID.randomUUID().toString());
113115
} else {
114-
_accountService.createUser(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domain.getId(),
115-
UUID.randomUUID().toString());
116+
// check if the user exists. if yes, call update
117+
UserAccount csuser = _accountService.getActiveUserAccount(user.getUsername(), domain.getId());
118+
if(csuser == null) {
119+
s_logger.debug("No user exists with name: " + user.getUsername() + " creating a user in the account: " + accountName);
120+
_accountService.createUser(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domain.getId(),
121+
UUID.randomUUID().toString());
122+
} else {
123+
s_logger.debug("account with name: " + accountName + " exist and user with name: " + user.getUsername() + " exists in the account. Updating the account.");
124+
_accountService.updateUser(csuser.getId(), user.getFirstname(), user.getLastname(), user.getEmail(), null, null, null, null, null);
125+
}
116126
}
117127
}
118128

‎plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LdapImportUsersCmdSpec.groovy‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,10 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
213213

214214
def accountService = Mock(AccountService)
215215
1 * accountService.getActiveAccountByName('ACCOUNT', 0) >> Mock(AccountVO)
216+
216217
1 * accountService.createUser('rmurphy', _ , 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 0, _) >> Mock(UserVO)
217218
0 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _)
219+
0 * accountService.updateUser(_,'Ryan', 'Murphy', 'rmurphy@test.com', null, null, null, null, null);
218220

219221
def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
220222
ldapImportUsersCmd.accountName = "ACCOUNT"
@@ -226,6 +228,36 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
226228
then: "expect 1 call on accountService createUser and 0 on account service create user account"
227229
}
228230

231+
232+
def "Test create ldap import account for an already existing cloudstack user"() {
233+
given: "We have an LdapManager, DomainService, two users and a LdapImportUsersCmd"
234+
def ldapManager = Mock(LdapManager)
235+
List<LdapUser> users = new ArrayList()
236+
users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering"))
237+
ldapManager.getUsers() >> users
238+
LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")
239+
ldapManager.createLdapUserResponse(_) >>> response1
240+
241+
def domainService = Mock(DomainService)
242+
1 * domainService.getDomain(1L) >> new DomainVO("DOMAIN", 1L, 1L, "DOMAIN", UUID.randomUUID().toString());;
243+
244+
def accountService = Mock(AccountService)
245+
1 * accountService.getActiveAccountByName('ACCOUNT', 0) >> Mock(AccountVO)
246+
1 * accountService.getActiveUserAccount('rmurphy',0) >> Mock(UserAccountVO)
247+
0 * accountService.createUser('rmurphy', _ , 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 0, _) >> Mock(UserVO)
248+
0 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _)
249+
1 * accountService.updateUser(_,'Ryan', 'Murphy', 'rmurphy@test.com', null, null, null, null, null);
250+
251+
def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
252+
ldapImportUsersCmd.accountName = "ACCOUNT"
253+
ldapImportUsersCmd.accountType = 2;
254+
ldapImportUsersCmd.domainId = 1L;
255+
256+
when: "create account is called"
257+
ldapImportUsersCmd.execute()
258+
then: "expect 1 call on accountService updateUser and 0 on account service create user and create user account"
259+
}
260+
229261
def "Test create ldap import account for a new cloudstack account"() {
230262
given: "We have an LdapManager, DomainService, two users and a LdapImportUsersCmd"
231263
def ldapManager = Mock(LdapManager)
@@ -242,6 +274,7 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
242274
1 * accountService.getActiveAccountByName('ACCOUNT', 0) >> null
243275
0 * accountService.createUser('rmurphy', _ , 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 0, _)
244276
1 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _)
277+
0 * accountService.updateUser(_,'Ryan', 'Murphy', 'rmurphy@test.com', null, null, null, null, null);
245278

246279
def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
247280
ldapImportUsersCmd.accountName = "ACCOUNT"

‎server/src/com/cloud/user/AccountManagerImpl.java‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,19 +1105,9 @@ public UserVO createUser(String userName, String password, String firstName, Str
11051105

11061106
@Override
11071107
@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User")
1108-
public UserAccount updateUser(UpdateUserCmd cmd) {
1109-
Long id = cmd.getId();
1110-
String apiKey = cmd.getApiKey();
1111-
String firstName = cmd.getFirstname();
1112-
String email = cmd.getEmail();
1113-
String lastName = cmd.getLastname();
1114-
String password = cmd.getPassword();
1115-
String secretKey = cmd.getSecretKey();
1116-
String timeZone = cmd.getTimezone();
1117-
String userName = cmd.getUsername();
1118-
1108+
public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, String timeZone) {
11191109
// Input validation
1120-
UserVO user = _userDao.getUser(id);
1110+
UserVO user = _userDao.getUser(userId);
11211111

11221112
if (user == null) {
11231113
throw new InvalidParameterValueException("unable to find user by id");
@@ -1140,7 +1130,7 @@ public UserAccount updateUser(UpdateUserCmd cmd) {
11401130

11411131
// don't allow updating system account
11421132
if (account.getId() == Account.ACCOUNT_ID_SYSTEM) {
1143-
throw new PermissionDeniedException("user id : " + id + " is system account, update is not allowed");
1133+
throw new PermissionDeniedException("user id : " + userId + " is system account, update is not allowed");
11441134
}
11451135

11461136
checkAccess(CallContext.current().getCallingAccount(), AccessType.OperateEntry, true, account);
@@ -1206,7 +1196,7 @@ public UserAccount updateUser(UpdateUserCmd cmd) {
12061196
}
12071197

12081198
if (s_logger.isDebugEnabled()) {
1209-
s_logger.debug("updating user with id: " + id);
1199+
s_logger.debug("updating user with id: " + userId);
12101200
}
12111201
try {
12121202
// check if the apiKey and secretKey are globally unique
@@ -1215,23 +1205,38 @@ public UserAccount updateUser(UpdateUserCmd cmd) {
12151205

12161206
if (apiKeyOwner != null) {
12171207
User usr = apiKeyOwner.first();
1218-
if (usr.getId() != id) {
1219-
throw new InvalidParameterValueException("The api key:" + apiKey + " exists in the system for user id:" + id + " ,please provide a unique key");
1208+
if (usr.getId() != userId) {
1209+
throw new InvalidParameterValueException("The api key:" + apiKey + " exists in the system for user id:" + userId + " ,please provide a unique key");
12201210
} else {
12211211
// allow the updation to take place
12221212
}
12231213
}
12241214
}
12251215

1226-
_userDao.update(id, user);
1216+
_userDao.update(userId, user);
12271217
} catch (Throwable th) {
12281218
s_logger.error("error updating user", th);
1229-
throw new CloudRuntimeException("Unable to update user " + id);
1219+
throw new CloudRuntimeException("Unable to update user " + userId);
12301220
}
12311221

12321222
CallContext.current().putContextParameter(User.class, user.getUuid());
12331223

1234-
return _userAccountDao.findById(id);
1224+
return _userAccountDao.findById(userId);
1225+
}
1226+
1227+
@Override
1228+
public UserAccount updateUser(UpdateUserCmd cmd) {
1229+
Long id = cmd.getId();
1230+
String apiKey = cmd.getApiKey();
1231+
String firstName = cmd.getFirstname();
1232+
String email = cmd.getEmail();
1233+
String lastName = cmd.getLastname();
1234+
String password = cmd.getPassword();
1235+
String secretKey = cmd.getSecretKey();
1236+
String timeZone = cmd.getTimezone();
1237+
String userName = cmd.getUsername();
1238+
1239+
return updateUser(id, firstName, lastName, email, userName, password, apiKey, secretKey, timeZone);
12351240
}
12361241

12371242
@Override
@@ -1805,6 +1810,11 @@ public Account getActiveAccountByName(String accountName, Long domainId) {
18051810
}
18061811
}
18071812

1813+
@Override
1814+
public UserAccount getActiveUserAccount(String username, Long domainId) {
1815+
return _userAccountDao.getUserAccount(username, domainId);
1816+
}
1817+
18081818
@Override
18091819
public Account getActiveAccountById(long accountId) {
18101820
return _accountDao.findById(accountId);

‎server/test/com/cloud/user/MockAccountManagerImpl.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,19 @@ public Account getActiveAccountByName(String accountName, Long domainId) {
137137
return null;
138138
}
139139

140+
@Override
141+
public UserAccount getActiveUserAccount(String username, Long domainId) {
142+
// TODO Auto-generated method stub
143+
return null;
144+
}
145+
146+
@Override
147+
public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey,
148+
String timeZone) {
149+
// TODO Auto-generated method stub
150+
return null;
151+
}
152+
140153
@Override
141154
public Account getActiveAccountById(long accountId) {
142155
// TODO Auto-generated method stub

0 commit comments

Comments
 (0)