Skip to content

Commit f4779b4

Browse files
karuturikoushik-das
karuturi
authored and
koushik-das
committed
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 <koushik@apache.org>
1 parent 29b4fe6 commit f4779b4

8 files changed

Lines changed: 152 additions & 28 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,12 @@ UserAccount createUserAccount(String userName, String password, String firstName
114114
void checkAccess(Account caller, AccessType accessType, boolean sameOwner, PartOf... entities) throws PermissionDeniedException;
115115

116116
Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly);
117+
118+
/**
119+
* returns the user account object for a given user id
120+
* @param userId user id
121+
* @return useraccount object if it exists else null
122+
*/
123+
UserAccount getUserAccountById(Long userId);
124+
117125
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ public void checkAccess(Account arg0, Domain arg1) throws PermissionDeniedExcept
9595

9696
}
9797

98+
@Override
99+
public UserAccount getUserAccountById(Long userId) {
100+
// TODO Auto-generated method stub
101+
return null;
102+
}
103+
98104
@Override
99105
public String[] createApiKeyAndSecretKey(RegisterCmd arg0) {
100106
// TODO Auto-generated method stub

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

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@
2323
import javax.inject.Inject;
2424
import javax.naming.NamingException;
2525

26-
import org.apache.log4j.Logger;
27-
import org.bouncycastle.util.encoders.Base64;
28-
2926
import org.apache.cloudstack.api.APICommand;
3027
import org.apache.cloudstack.api.ApiConstants;
3128
import org.apache.cloudstack.api.ApiErrorCode;
@@ -38,37 +35,33 @@
3835
import org.apache.cloudstack.context.CallContext;
3936
import org.apache.cloudstack.ldap.LdapManager;
4037
import org.apache.cloudstack.ldap.LdapUser;
38+
import org.apache.log4j.Logger;
39+
import org.bouncycastle.util.encoders.Base64;
4140

41+
import com.cloud.domain.DomainVO;
4242
import com.cloud.user.Account;
4343
import com.cloud.user.AccountService;
44+
import com.cloud.user.User;
4445
import com.cloud.user.UserAccount;
4546

46-
@APICommand(name = "ldapCreateAccount", description = "Creates an account from an LDAP user", responseObject = AccountResponse.class, since = "4.2.0",
47-
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
47+
@APICommand(name = "ldapCreateAccount", description = "Creates an account from an LDAP user", responseObject = AccountResponse.class, since = "4.2.0", requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
4848
public class LdapCreateAccountCmd extends BaseCmd {
4949
public static final Logger s_logger = Logger.getLogger(LdapCreateAccountCmd.class.getName());
5050
private static final String s_name = "createaccountresponse";
5151

5252
@Inject
5353
private LdapManager _ldapManager;
5454

55-
@Parameter(name = ApiConstants.ACCOUNT,
56-
type = CommandType.STRING,
57-
description = "Creates the user under the specified account. If no account is specified, the username will be used as the account name.")
55+
@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.")
5856
private String accountName;
5957

60-
@Parameter(name = ApiConstants.ACCOUNT_TYPE,
61-
type = CommandType.SHORT,
62-
required = true,
63-
description = "Type of the account. Specify 0 for user, 1 for root admin, and 2 for domain admin")
58+
@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")
6459
private Short accountType;
6560

6661
@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "Creates the user under the specified domain.")
6762
private Long domainId;
6863

69-
@Parameter(name = ApiConstants.TIMEZONE,
70-
type = CommandType.STRING,
71-
description = "Specifies a timezone for this command. For more information on the timezone parameter, see Time Zone Format.")
64+
@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.")
7265
private String timezone;
7366

7467
@Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING, required = true, description = "Unique username.")
@@ -96,22 +89,46 @@ public LdapCreateAccountCmd(final LdapManager ldapManager, final AccountService
9689
_accountService = accountService;
9790
}
9891

99-
UserAccount createCloudstackUserAccount(final LdapUser user) {
100-
return _accountService.createUserAccount(username, generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName,
101-
accountType, domainId, networkDomain, details, accountUUID, userUUID);
92+
UserAccount createCloudstackUserAccount(final LdapUser user, String accountName, Long domainId) {
93+
Account account = _accountService.getActiveAccountByName(accountName, domainId);
94+
if (account == null) {
95+
return _accountService.createUserAccount(username, generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, accountType,
96+
domainId, networkDomain, details, accountUUID, userUUID);
97+
} else {
98+
User newUser = _accountService.createUser(username, generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domainId,
99+
userUUID);
100+
return _accountService.getUserAccountById(newUser.getId());
101+
}
102+
}
103+
104+
private String getAccountName() {
105+
String name = accountName;
106+
if (accountName == null) {
107+
name = username;
108+
}
109+
return name;
110+
}
111+
112+
private Long getDomainId() {
113+
Long id = domainId;
114+
if (id == null) {
115+
id = DomainVO.ROOT_DOMAIN;
116+
}
117+
return id;
102118
}
103119

104120
@Override
105121
public void execute() throws ServerApiException {
106122
final CallContext callContext = getCurrentContext();
107-
callContext.setEventDetails("Account Name: " + accountName + ", Domain Id:" + domainId);
123+
String finalAccountName = getAccountName();
124+
Long finalDomainId = getDomainId();
125+
callContext.setEventDetails("Account Name: " + finalAccountName + ", Domain Id:" + finalDomainId);
108126
try {
109127
final LdapUser user = _ldapManager.getUser(username);
110128
validateUser(user);
111-
final UserAccount userAccount = createCloudstackUserAccount(user);
129+
final UserAccount userAccount = createCloudstackUserAccount(user, finalAccountName, finalDomainId);
112130
if (userAccount != null) {
113-
final AccountResponse response = _responseGenerator
114-
.createUserAccountResponse(ResponseView.Full, userAccount);
131+
final AccountResponse response = _responseGenerator.createUserAccountResponse(ResponseView.Full, userAccount);
115132
response.setResponseName(getCommandName());
116133
setResponseObject(response);
117134
} else {
@@ -159,4 +176,4 @@ private boolean validateUser(final LdapUser user) throws ServerApiException {
159176
}
160177
return true;
161178
}
162-
}
179+
}

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import javax.inject.Inject;
2727

28+
import com.cloud.user.Account;
2829
import org.apache.cloudstack.api.APICommand;
2930
import org.apache.cloudstack.api.ApiConstants;
3031
import org.apache.cloudstack.api.ApiErrorCode;
@@ -87,6 +88,9 @@ public class LdapImportUsersCmd extends BaseListCmd {
8788

8889
private Domain _domain;
8990

91+
@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.")
92+
private String accountName;
93+
9094
@Inject
9195
private LdapManager _ldapManager;
9296

@@ -101,6 +105,17 @@ public LdapImportUsersCmd(final LdapManager ldapManager, final DomainService dom
101105
_accountService = accountService;
102106
}
103107

108+
private void createCloudstackUserAccount(LdapUser user, String accountName, Domain domain) {
109+
Account account = _accountService.getActiveAccountByName(accountName, domain.getId());
110+
if (account == null) {
111+
_accountService.createUserAccount(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, accountType,
112+
domain.getId(), domain.getNetworkDomain(), details, UUID.randomUUID().toString(), UUID.randomUUID().toString());
113+
} else {
114+
_accountService.createUser(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone, accountName, domain.getId(),
115+
UUID.randomUUID().toString());
116+
}
117+
}
118+
104119
@Override
105120
public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException,
106121
ResourceAllocationException, NetworkRuleConflictException {
@@ -122,8 +137,7 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE
122137
for (LdapUser user : users) {
123138
Domain domain = getDomain(user);
124139
try {
125-
_accountService.createUserAccount(user.getUsername(), generatePassword(), user.getFirstname(), user.getLastname(), user.getEmail(), timezone,
126-
user.getUsername(), accountType, domain.getId(), domain.getNetworkDomain(), details, UUID.randomUUID().toString(), UUID.randomUUID().toString());
140+
createCloudstackUserAccount(user, getAccountName(user), domain);
127141
addedUsers.add(user);
128142
} catch (InvalidParameterValueException ex) {
129143
s_logger.error("Failed to create user with username: " + user.getUsername() + " ::: " + ex.getMessage());
@@ -135,6 +149,14 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE
135149
setResponseObject(response);
136150
}
137151

152+
private String getAccountName(LdapUser user) {
153+
String finalAccountName = accountName;
154+
if(finalAccountName == null ) {
155+
finalAccountName = user.getUsername();
156+
}
157+
return finalAccountName;
158+
}
159+
138160
private Domain getDomainForName(String name) {
139161
Domain domain = null;
140162
if (StringUtils.isNotBlank(name)) {

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@ package groovy.org.apache.cloudstack.ldap
1919
import com.cloud.domain.Domain
2020
import com.cloud.domain.DomainVO
2121
import com.cloud.user.AccountService
22+
import com.cloud.user.AccountVO
2223
import com.cloud.user.DomainService
24+
import com.cloud.user.UserAccountVO
25+
import com.cloud.user.UserVO
26+
import org.apache.cloudstack.api.command.LdapCreateAccountCmd
2327
import org.apache.cloudstack.api.command.LdapImportUsersCmd
2428
import org.apache.cloudstack.api.response.LdapUserResponse
29+
import org.apache.cloudstack.context.CallContext
2530
import org.apache.cloudstack.ldap.LdapManager
2631
import org.apache.cloudstack.ldap.LdapUser
2732

@@ -193,4 +198,59 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
193198

194199
}
195200

201+
202+
def "Test create ldap import account for an already existing cloudstack account"() {
203+
given: "We have an LdapManager, DomainService, two users and a LdapImportUsersCmd"
204+
def ldapManager = Mock(LdapManager)
205+
List<LdapUser> users = new ArrayList()
206+
users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering"))
207+
ldapManager.getUsers() >> users
208+
LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")
209+
ldapManager.createLdapUserResponse(_) >>> response1
210+
211+
def domainService = Mock(DomainService)
212+
1 * domainService.getDomain(1L) >> new DomainVO("DOMAIN", 1L, 1L, "DOMAIN", UUID.randomUUID().toString());;
213+
214+
def accountService = Mock(AccountService)
215+
1 * accountService.getActiveAccountByName('ACCOUNT', 0) >> Mock(AccountVO)
216+
1 * accountService.createUser('rmurphy', _ , 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 0, _) >> Mock(UserVO)
217+
0 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _)
218+
219+
def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
220+
ldapImportUsersCmd.accountName = "ACCOUNT"
221+
ldapImportUsersCmd.accountType = 2;
222+
ldapImportUsersCmd.domainId = 1L;
223+
224+
when: "create account is called"
225+
ldapImportUsersCmd.execute()
226+
then: "expect 1 call on accountService createUser and 0 on account service create user account"
227+
}
228+
229+
def "Test create ldap import account for a new cloudstack account"() {
230+
given: "We have an LdapManager, DomainService, two users and a LdapImportUsersCmd"
231+
def ldapManager = Mock(LdapManager)
232+
List<LdapUser> users = new ArrayList()
233+
users.add(new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering"))
234+
ldapManager.getUsers() >> users
235+
LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")
236+
ldapManager.createLdapUserResponse(_) >>> response1
237+
238+
def domainService = Mock(DomainService)
239+
1 * domainService.getDomain(1L) >> new DomainVO("DOMAIN", 1L, 1L, "DOMAIN", UUID.randomUUID().toString());;
240+
241+
def accountService = Mock(AccountService)
242+
1 * accountService.getActiveAccountByName('ACCOUNT', 0) >> null
243+
0 * accountService.createUser('rmurphy', _ , 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 0, _)
244+
1 * accountService.createUserAccount('rmurphy', _, 'Ryan', 'Murphy', 'rmurphy@test.com', null, 'ACCOUNT', 2, 0, 'DOMAIN', null, _, _)
245+
246+
def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
247+
ldapImportUsersCmd.accountName = "ACCOUNT"
248+
ldapImportUsersCmd.accountType = 2;
249+
ldapImportUsersCmd.domainId = 1L;
250+
251+
when: "create account is called"
252+
ldapImportUsersCmd.execute()
253+
then: "expect 1 call on accountService createUser and 0 on account service create user account"
254+
}
255+
196256
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,4 +2623,9 @@ public Long finalyzeAccountId(final String accountName, final Long domainId, fin
26232623
}
26242624
return null;
26252625
}
2626+
2627+
@Override
2628+
public UserAccount getUserAccountById(Long userId) {
2629+
return _userAccountDao.findById(userId);
2630+
}
26262631
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,12 @@ public void checkAccess(Account account, AccessType accessType, boolean sameOwne
226226

227227
}
228228

229+
@Override
230+
public UserAccount getUserAccountById(Long userId) {
231+
// TODO Auto-generated method stub
232+
return null;
233+
}
234+
229235
@Override
230236
public void logoutUser(long userId) {
231237
// TODO Auto-generated method stub

‎ui/scripts/accountsWizard.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,10 @@
198198
array1.push("&domainid=" + args.data.domainid);
199199

200200
var account = args.data.account;
201-
if (account === null || account.length === 0) {
202-
account = args.username;
201+
202+
if (account !== null && account.length > 0) {
203+
array1.push("&account=" + account);
203204
}
204-
array1.push("&account=" + account);
205205

206206
var accountType = args.data.accounttype;
207207
if (args.data.accounttype == "1" && args.data.domainid != rootDomainId) { //if account type is admin, but domain is not Root domain

0 commit comments

Comments
 (0)